Skip to content

Commit 9ee22b9

Browse files
d-csTrigger.dev RepoOps
authored andcommitted
fix(run-store): only report a probe duplicate when shards return the same id
With more than two run stores configured, a lookup with no id in its where clause queries every store in parallel. Each store can legitimately return a different matching row, but the router treated any result from more than one store as one id returned by two shards and reported it under the first row's id. The router now groups the returned rows by id and reports a duplicate only for an id that came back from two or more stores. Rows without an id cannot be compared and are not reported. Which row the lookup returns is unchanged. Mono-RevId: cb8c11e08a67f81c25e45bf9cf2efc0fa3126203
1 parent 2696426 commit 9ee22b9

2 files changed

Lines changed: 185 additions & 5 deletions

File tree

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
// An id-less lookup legitimately returns a DIFFERENT row from each store. Above two stores the
2+
// probe runs every leg, and only one id seen on two stores is a duplicate.
3+
4+
import { makeNShardRunOpsPostgresTest } from "@internal/testcontainers";
5+
import type { PrismaClient } from "@trigger.dev/database";
6+
import type { RunOpsPrismaClient } from "@internal/run-ops-database";
7+
import { resolveShard } from "@trigger.dev/core/v3/isomorphic";
8+
import { describe, expect } from "vitest";
9+
import { PostgresRunStore } from "./PostgresRunStore.js";
10+
import { RoutingRunStore } from "./runOpsStore.js";
11+
import type { RoutingStoreMetrics } from "./routingStoreMetrics.js";
12+
13+
type AnyClient = PrismaClient | RunOpsPrismaClient;
14+
15+
const threeStoreTest = makeNShardRunOpsPostgresTest(1);
16+
17+
function gen2(shardChar: string, seed: string): string {
18+
const core = (seed.replace(/[^0-9a-v]/g, "0") + "k".repeat(24)).slice(0, 24);
19+
return `${core}${shardChar}2`;
20+
}
21+
const cuid = (seed: string) => (seed + "c".repeat(25)).slice(0, 25);
22+
const runOpsNew = (seed: string) =>
23+
(seed.replace(/[^0-9a-v]/g, "0") + "k".repeat(24)).slice(0, 24) + "01";
24+
25+
function makeStore(prisma: AnyClient, variant: "legacy" | "dedicated") {
26+
return new PostgresRunStore({
27+
prisma: prisma as never,
28+
readOnlyPrisma: prisma as never,
29+
schemaVariant: variant,
30+
});
31+
}
32+
33+
function recordingMetrics() {
34+
const duplicates: string[][] = [];
35+
const metrics: RoutingStoreMetrics = {
36+
recordDuplicateId: (shardKeys) => duplicates.push(shardKeys),
37+
recordWaitpointProbeFallback() {},
38+
recordShardRouted() {},
39+
};
40+
return { metrics, duplicates };
41+
}
42+
43+
function makeRouter(
44+
legacyPrisma: PrismaClient,
45+
newPrisma: RunOpsPrismaClient,
46+
shardPrismas: RunOpsPrismaClient[],
47+
metrics: RoutingStoreMetrics
48+
) {
49+
return new RoutingRunStore({
50+
new: makeStore(newPrisma, "dedicated"),
51+
legacy: makeStore(legacyPrisma, "legacy"),
52+
shards: [{ key: "a", store: makeStore(shardPrismas[0]!, "dedicated") }],
53+
resolveShard,
54+
metrics,
55+
});
56+
}
57+
58+
async function seedLegacyEnv(prisma: PrismaClient, suffix: string) {
59+
const organization = await prisma.organization.create({
60+
data: { title: `Org ${suffix}`, slug: `org-${suffix}` },
61+
});
62+
const project = await prisma.project.create({
63+
data: {
64+
name: `Project ${suffix}`,
65+
slug: `project-${suffix}`,
66+
externalRef: `proj_${suffix}`,
67+
organizationId: organization.id,
68+
},
69+
});
70+
const environment = await prisma.runtimeEnvironment.create({
71+
data: {
72+
type: "DEVELOPMENT",
73+
slug: "dev",
74+
projectId: project.id,
75+
organizationId: organization.id,
76+
apiKey: `tr_dev_${suffix}`,
77+
pkApiKey: `pk_dev_${suffix}`,
78+
shortcode: `short_${suffix}`,
79+
},
80+
});
81+
return { projectId: project.id, environmentId: environment.id };
82+
}
83+
84+
async function seedManualWaitpoint(
85+
prisma: AnyClient,
86+
params: { id: string; projectId: string; environmentId: string }
87+
) {
88+
await (prisma as PrismaClient).waitpoint.create({
89+
data: {
90+
id: params.id,
91+
friendlyId: `wp_${params.id}`,
92+
type: "MANUAL",
93+
status: "PENDING",
94+
idempotencyKey: `idem_${params.id}`,
95+
userProvidedIdempotencyKey: false,
96+
projectId: params.projectId,
97+
environmentId: params.environmentId,
98+
},
99+
});
100+
}
101+
102+
describe("RoutingRunStore id-less probe — duplicate alarm across three stores", () => {
103+
threeStoreTest(
104+
"a different MANUAL waitpoint on each store is not a duplicate",
105+
async ({ legacyPrisma, newPrisma, shardPrismas }) => {
106+
const env = await seedLegacyEnv(legacyPrisma, "probe_distinct");
107+
const { metrics, duplicates } = recordingMetrics();
108+
const router = makeRouter(legacyPrisma, newPrisma, shardPrismas, metrics);
109+
110+
const onLegacy = cuid("pdleg");
111+
const onNew = runOpsNew("pdnew");
112+
const onA = gen2("a", "pda");
113+
await seedManualWaitpoint(legacyPrisma, { id: onLegacy, ...env });
114+
await seedManualWaitpoint(newPrisma, { id: onNew, ...env });
115+
await seedManualWaitpoint(shardPrismas[0]!, { id: onA, ...env });
116+
117+
// WaitpointListPresenter's "any token in this environment" probe.
118+
const found = await router.findWaitpoint({
119+
where: { environmentId: env.environmentId, type: "MANUAL" },
120+
});
121+
122+
// The highest-precedence hit (gen-2 shard a) still wins.
123+
expect(found?.id).toBe(onA);
124+
expect(duplicates).toEqual([]);
125+
}
126+
);
127+
128+
threeStoreTest(
129+
"one id on new and on shard a alarms once, for those two stores",
130+
async ({ legacyPrisma, newPrisma, shardPrismas }) => {
131+
const env = await seedLegacyEnv(legacyPrisma, "probe_dup");
132+
const { metrics, duplicates } = recordingMetrics();
133+
const router = makeRouter(legacyPrisma, newPrisma, shardPrismas, metrics);
134+
135+
const dup = gen2("a", "pddup");
136+
await seedManualWaitpoint(legacyPrisma, { id: cuid("pddupleg"), ...env });
137+
await seedManualWaitpoint(newPrisma, { id: dup, ...env });
138+
await seedManualWaitpoint(shardPrismas[0]!, { id: dup, ...env });
139+
140+
const found = await router.findWaitpoint({
141+
where: { environmentId: env.environmentId, type: "MANUAL" },
142+
});
143+
144+
expect(found?.id).toBe(dup);
145+
expect(duplicates).toEqual([["new", "a"]]);
146+
}
147+
);
148+
149+
threeStoreTest(
150+
"a hit whose projection has no id is not compared and does not alarm",
151+
async ({ legacyPrisma, newPrisma, shardPrismas }) => {
152+
const env = await seedLegacyEnv(legacyPrisma, "probe_noid");
153+
const { metrics, duplicates } = recordingMetrics();
154+
const router = makeRouter(legacyPrisma, newPrisma, shardPrismas, metrics);
155+
156+
const onA = gen2("a", "pna");
157+
await seedManualWaitpoint(legacyPrisma, { id: cuid("pnleg"), ...env });
158+
await seedManualWaitpoint(shardPrismas[0]!, { id: onA, ...env });
159+
160+
// Legacy honours the projection and returns no id; the dedicated store still hydrates one.
161+
const found = await router.findWaitpoint({
162+
where: { environmentId: env.environmentId, type: "MANUAL" },
163+
select: { type: true },
164+
});
165+
166+
// `id` is forced into the narrowed select by PostgresRunStore's stripDedicatedRelations.
167+
expect(found).toEqual({ id: onA, type: "MANUAL" });
168+
expect(duplicates).toEqual([]);
169+
}
170+
);
171+
});

‎internal-packages/run-store/src/runOpsStore.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -381,12 +381,21 @@ export class RoutingRunStore implements RunStore {
381381
// store (runs, waitpoints, attempts, snapshots). Batches are the exception: `batchTriggerV3`
382382
// writes raw to the control plane while runEngine routes by id, so a batch is legitimately
383383
// dual-resident. Those callers pass `alarmOnDuplicate: false` so a batch on legacy + a gen-2
384-
// shard is not mistaken for a violation.
384+
// shard is not mistaken for a violation. A lookup with no id in its where returns a different
385+
// row from each store, so only an id seen on two stores counts; a hit with no id can't be
386+
// compared.
385387
if (hits.length > 1 && opts?.alarmOnDuplicate !== false) {
386-
this.#reportDuplicateId(
387-
String((hits[0]!.value as { id?: unknown })?.id ?? "unknown"),
388-
hits.map((h) => h.key)
389-
);
388+
const keysById = new Map<string, ShardKey[]>();
389+
for (const { key, value } of hits) {
390+
const id = (value as { id?: unknown }).id;
391+
if (typeof id !== "string") continue;
392+
const keys = keysById.get(id);
393+
if (keys) keys.push(key);
394+
else keysById.set(id, [key]);
395+
}
396+
for (const [id, keys] of keysById) {
397+
if (keys.length > 1) this.#reportDuplicateId(id, keys);
398+
}
390399
}
391400
if (hits.length > 0) {
392401
// Highest authority wins: #precedence ascends, so take the last hit in that order.

0 commit comments

Comments
 (0)