Skip to content

Commit b6f3886

Browse files
committed
test(webapp): pin the batches-list merge precedence
The merge was only ever exercised as a union, with each id on exactly one store, so nothing caught a leg-order regression: a union is insensitive to the order its legs are applied in. Seed one id on two stores and assert the higher-authority copy is the one the page shows. Verified by mutation: making the lower-authority insert unconditional, and applying the legs in the wrong order, both fail it. Also corrects a comment that justified the leg order with a claim about duplicate ids being impossible. A single writer routes creates by id shape, but a row can still sit on two stores while data is moved between them, which is why the routing store dedupes batches without alarming.
1 parent 895d554 commit b6f3886

2 files changed

Lines changed: 59 additions & 2 deletions

File tree

apps/webapp/app/presenters/v3/BatchListPresenter.server.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,13 @@ export class BatchListPresenter extends BasePresenter {
142142
// The gen-1 pair keeps its original conditional form so its result stays byte-identical; the
143143
// shard legs then overwrite. Do NOT "simplify" the conditional insert below into an
144144
// unconditional one without also reordering the legs, or legacy would start winning over new.
145-
// A gen-1 id and a gen-2 id cannot collide in any case: a batch is created on exactly one store,
146-
// routed by its own id shape.
145+
//
146+
// Precedence is load-bearing, not decoration. Today a batch row has exactly one writer
147+
// (PostgresRunStore.createBatchTaskRun), which routes by id shape, so a duplicate id is not
148+
// produced by ordinary creates. But a row can still exist on two stores while data is being
149+
// moved between them, which is why the routing store dedupes batches WITHOUT alarming
150+
// (alarmOnDuplicate: false). When that happens the page must show the higher-authority copy,
151+
// and that is what the leg order below decides.
147152
const byId = new Map<string, BatchRow>();
148153
for (const row of newRows) {
149154
byId.set(row.id, row);

apps/webapp/test/batchListPresenter.readroute.test.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -852,6 +852,58 @@ describe("BatchListPresenter gen-2 shard legs (legacy PG14 + new PG17 + 2 shard
852852
}
853853
);
854854

855+
// The merge is not just a union: when one id exists on more than one store, precedence decides
856+
// which copy the page shows. Ascending legacy -> new -> shards, last write wins, mirroring the
857+
// routing store's #mergeById. Without this case every leg-order regression still passes, because
858+
// a union is order-insensitive.
859+
twoShardTest(
860+
"a duplicated id resolves by precedence: a shard copy outranks new, and new outranks legacy",
861+
async ({ legacyPrisma, newPrisma, shardPrismas }) => {
862+
const shardA = shardPrismas[0]!;
863+
const ctx = await seedParents(legacyPrisma, "gen2-precedence");
864+
865+
// Same id on legacy AND new. New is the higher authority, so its copy must win.
866+
const onBothGenOne = "cmm000000000000000000prec";
867+
await createBatch(legacyPrisma, ctx, {
868+
id: onBothGenOne,
869+
friendlyId: "fr_prec_gen1",
870+
status: "PENDING",
871+
createdAt: new Date(Date.now() - 60_000),
872+
});
873+
await createBatch(newPrisma, ctx, {
874+
id: onBothGenOne,
875+
friendlyId: "fr_prec_gen1",
876+
status: "COMPLETED",
877+
createdAt: new Date(Date.now() - 60_000),
878+
});
879+
880+
// Same id on new AND a shard. The shard is the higher authority, so its copy must win.
881+
const onNewAndShard = generateRunOpsIdV2("a");
882+
await createBatch(newPrisma, ctx, {
883+
id: onNewAndShard,
884+
friendlyId: "fr_prec_shard",
885+
status: "PENDING",
886+
createdAt: new Date(Date.now() - 30_000),
887+
});
888+
await createBatch(shardA, ctx, {
889+
id: onNewAndShard,
890+
friendlyId: "fr_prec_shard",
891+
status: "COMPLETED",
892+
createdAt: new Date(Date.now() - 30_000),
893+
});
894+
895+
const page = await shardPresenter(legacyPrisma, newPrisma, [
896+
{ key: "a", replica: shardA },
897+
]).call(baseCall(ctx, { pageSize: 10 }));
898+
899+
// Each id appears exactly once, and each carries the higher-authority store's status.
900+
expect(page.batches.map((b) => b.id).filter((id) => id === onBothGenOne)).toHaveLength(1);
901+
expect(page.batches.map((b) => b.id).filter((id) => id === onNewAndShard)).toHaveLength(1);
902+
expect(page.batches.find((b) => b.id === onBothGenOne)?.status).toBe("COMPLETED");
903+
expect(page.batches.find((b) => b.id === onNewAndShard)?.status).toBe("COMPLETED");
904+
}
905+
);
906+
855907
// Composition proof for the alias rule: a descriptor that declares `aliasOf` shares its target's
856908
// client BY REFERENCE, so `nonAliasedShardReplicas` drops it and the target's own leg returns the
857909
// rows. This is the soak topology — gen-2 ids living on the gen-1 new database.

0 commit comments

Comments
 (0)