Skip to content

Commit 0c143ec

Browse files
test(metadata): pin default memory cluster driver's cross-process isolation (#13609) (#13883)
Measurement for #13609: a positive control shows unregister() DOES fan out via CLUSTER_CHANNEL/notifyWatchers and a peer sharing the transport evicts immediately (upholds the source counter-evidence). A second test gives each replica its own MemoryPubSub instance -- the shipped default (`driver: 'memory'`) across two real OS processes -- and shows the deleted row is still served past 10 list-cache TTL windows, because the stale entry lives in the in-memory registry (no TTL) and readListUncached() never re-checks a registry hit against the loader. Measurement-only; no production behavior changed. Co-authored-by: Claude <noreply@anthropic.com>
1 parent 73c8466 commit 0c143ec

1 file changed

Lines changed: 112 additions & 0 deletions

File tree

packages/metadata/src/metadata-manager-cluster.test.ts

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,3 +399,115 @@ describe('MetadataManager — a cluster peer write invalidates local caches (#51
399399
expect(cachedTypes(a)).toEqual(['view']);
400400
});
401401
});
402+
403+
/**
404+
* #13609 — the seam behind "delete keeps being served cluster-wide, longer
405+
* than any TTL", measured statically (no live multi-node deployment
406+
* available; see the issue for the full write-up).
407+
*
408+
* Every test above in this file shares ONE `TestPubSub` bus between A and
409+
* B — the correct model of a WORKING cross-node transport (a real `redis` /
410+
* `postgres` cluster driver), and it is why `unregister()`'s cluster fan-out
411+
* (`notifyWatchers` → `CLUSTER_CHANNEL` → the peer's `invalidateForForeignWrite`)
412+
* checks out as correct: register() and unregister() call it identically, so
413+
* the #13405 reading ("create broadcasts, delete does not mirror") is false
414+
* as a claim about this mechanism.
415+
*
416+
* But `Runtime`'s shipped DEFAULT (`cluster` option omitted) is
417+
* `defineCluster({})` → `driver: 'memory'`, and `MemoryPubSub`'s own
418+
* doc-comment is explicit: "No cross-process delivery — use the redis /
419+
* postgres / nats driver for real multi-node setups." Each of N replica
420+
* PROCESSES constructs its OWN `MemoryPubSub` instance; nothing wires them
421+
* together. The split-brain guard (`assertClusterDriverSafeForTopology`,
422+
* `split-brain-guard.ts`) only fires when the operator has *declared*
423+
* multi-node via `OS_EXPECT_MULTI_NODE` / `OS_CLUSTER_REPLICAS>1` — silent
424+
* otherwise, by design (ADR-0010, path A).
425+
*
426+
* So the below gives A and B *separate* `TestPubSub` instances instead of
427+
* `makeCluster()`'s shared one — the faithful in-process stand-in for two
428+
* real OS processes each on the shipped default driver. No live cluster is
429+
* needed to observe the isolation: it is a property of the transport object
430+
* (no socket, no IPC — see `MemoryPubSub`'s full implementation), identical
431+
* whether the two instances live in one test process or two real ones.
432+
*
433+
* Both replicas locally `register()` the row before the delete, matching
434+
* what `restoreRuntimeDatasources` does on every replica's own boot: each
435+
* reads the same durable `sys_metadata` row and calls `metadata.register()`
436+
* LOCALLY — not via a broadcast. That is also the reconciling condition for
437+
* the three-way tension: `readListUncached()` merges the in-memory registry
438+
* BEFORE the loader and never overwrites a registry hit with a loader
439+
* answer (see `metadata-manager.ts`). A **new** name a peer's registry has
440+
* never seen falls through to the (shared, authoritative) loader on every
441+
* read, so a create looks like it "reaches" peers within one `list()` call —
442+
* no propagation required. A name a peer's registry already holds POSITIVE
443+
* is never re-checked against the loader at all, so a stale positive
444+
* survives every list-cache TTL window forever, not for ~30s. That asymmetry
445+
* — not "delete doesn't broadcast" — is what #13405 actually observed.
446+
*/
447+
describe('MetadataManager — default `memory` cluster driver does not cross OS processes (#13609)', () => {
448+
const dsRow = (name: string) => ({ name, driver: 'postgres', origin: 'runtime' as const });
449+
450+
it('control: WITH a working cross-node transport, unregister() on A evicts B immediately', async () => {
451+
// `makeCluster()` shares one bus — models a real redis/postgres driver.
452+
const { store, a, b } = makeCluster();
453+
await store.save('datasource', 'ds_doomed', dsRow('ds_doomed'));
454+
// Both replicas locally register at "boot", exactly like
455+
// `restoreRuntimeDatasources` reading the same `sys_metadata` row.
456+
await a.register('datasource', 'ds_doomed', dsRow('ds_doomed'));
457+
await b.register('datasource', 'ds_doomed', dsRow('ds_doomed'));
458+
expect(await b.get('datasource', 'ds_doomed')).toBeTruthy();
459+
460+
await a.unregister('datasource', 'ds_doomed');
461+
462+
// Positive control: the probe below (get + list, same door
463+
// `/api/v1/meta/datasource` reads through) WOULD have seen this.
464+
expect(await b.get('datasource', 'ds_doomed')).toBeUndefined();
465+
expect(viewNames(await b.list('datasource'))).toEqual([]);
466+
});
467+
468+
it('WITHOUT it (the shipped default across real replicas): B keeps serving the deleted row past 10 list-cache TTL windows', async () => {
469+
vi.useFakeTimers();
470+
try {
471+
const store = new SharedStoreLoader();
472+
const a = new MetadataManager({ formats: ['json'], loaders: [store] });
473+
const b = new MetadataManager({ formats: ['json'], loaders: [store] });
474+
// Two INDEPENDENT pubsub instances, not shared — the isolation
475+
// `defineCluster({ driver: 'memory' })` (the omitted-config
476+
// default) actually ships with across two real processes.
477+
a.attachClusterPubSub(new TestPubSub(), 'node-A');
478+
b.attachClusterPubSub(new TestPubSub(), 'node-B');
479+
480+
await store.save('datasource', 'ds_doomed', dsRow('ds_doomed'));
481+
await a.register('datasource', 'ds_doomed', dsRow('ds_doomed'));
482+
await b.register('datasource', 'ds_doomed', dsRow('ds_doomed'));
483+
expect(await b.get('datasource', 'ds_doomed')).toBeTruthy();
484+
485+
// DELETE lands on A (e.g. the admin REST door). Storage-first,
486+
// in-memory second (#5259): A is correct immediately, and the
487+
// shared DB row is gone too.
488+
await a.unregister('datasource', 'ds_doomed');
489+
expect(await a.get('datasource', 'ds_doomed')).toBeUndefined();
490+
expect(store.storage.get('datasource')?.has('ds_doomed')).toBe(false);
491+
492+
// B's broadcast never arrives (separate pubsub instance). Advance
493+
// WAY past the list-cache TTL this manager applies to `list()` —
494+
// if this were TTL-bound (the #5109 / A2.3 shape) it would have
495+
// cleared by now. It has not: the stale entry lives in the
496+
// REGISTRY, which carries no TTL at all, and every fresh
497+
// `readListUncached()` re-derives the same answer because the
498+
// registry hit is never checked against the (correct) loader.
499+
const ttl = (MetadataManager as unknown as { LIST_CACHE_TTL_MS: number })
500+
.LIST_CACHE_TTL_MS;
501+
vi.advanceTimersByTime(ttl * 10);
502+
503+
// This is exactly what `/api/v1/meta/datasource` reads through
504+
// (`MetadataProtocol` → `metadataService.list('datasource')`) and
505+
// what the admin `listDatasourceRecords` reads for the
506+
// detail/admin registry (`metadataOf()?.list('datasource')`).
507+
expect(await b.get('datasource', 'ds_doomed')).toBeTruthy();
508+
expect(viewNames(await b.list('datasource'))).toEqual(['ds_doomed']);
509+
} finally {
510+
vi.useRealTimers();
511+
}
512+
});
513+
});

0 commit comments

Comments
 (0)