Skip to content

Commit 480105c

Browse files
d-csTrigger.dev RepoOps
authored andcommitted
feat(run-engine,run-store): make retried snapshot and waitpoint operations replay-safe
Internal groundwork for surviving brief database connection blips. No user-facing change on its own. Mono-RevId: b0eff85f8500b01f86dc17907da1b999589ebfcc
1 parent 5b6f9f1 commit 480105c

17 files changed

Lines changed: 1366 additions & 121 deletions

internal-packages/run-engine/src/engine/systems/executionSnapshotSystem.test.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,14 @@ function createEngineOptions(redisOptions: any, prisma: any, store?: PostgresRun
5656
class CountingPostgresRunStore extends PostgresRunStore {
5757
public creates = 0;
5858
public latestReads = 0;
59+
public snapshotCreateIds: (string | undefined)[] = [];
5960

6061
override async createExecutionSnapshot(
6162
input: CreateExecutionSnapshotInput,
6263
tx?: any
6364
): ReturnType<PostgresRunStore["createExecutionSnapshot"]> {
6465
this.creates++;
66+
this.snapshotCreateIds.push(input.id);
6567
return super.createExecutionSnapshot(input, tx);
6668
}
6769

@@ -130,6 +132,98 @@ describe("executionSnapshotSystem store routing (single-DB passthrough)", () =>
130132
}
131133
});
132134

135+
// A transition routed through the engine hands the store a stable id, so a blip-retry inside the
136+
// store replays the same transition idempotently instead of duplicating it.
137+
containerTest(
138+
"snapshot transitions carry a store-supplied id",
139+
async ({ prisma, redisOptions }) => {
140+
const countingStore = new CountingPostgresRunStore({ prisma, readOnlyPrisma: prisma });
141+
const engine = new RunEngine(createEngineOptions(redisOptions, prisma, countingStore));
142+
143+
try {
144+
const run = await triggerRun(engine, prisma, "run_snapid1");
145+
const latest = await getLatestExecutionSnapshot(prisma, run.id, countingStore);
146+
countingStore.snapshotCreateIds.length = 0;
147+
148+
const created = await engine.executionSnapshotSystem.createExecutionSnapshot(prisma, {
149+
run: { id: run.id, status: latest.runStatus, attemptNumber: latest.attemptNumber },
150+
snapshot: { executionStatus: latest.executionStatus, description: "test transition" },
151+
previousSnapshotId: latest.id,
152+
environmentId: latest.environmentId,
153+
environmentType: latest.environmentType,
154+
projectId: latest.projectId,
155+
organizationId: latest.organizationId,
156+
});
157+
158+
expect(created.id).toBeDefined();
159+
expect(SnapshotId.fromFriendlyId(SnapshotId.toFriendlyId(created.id))).toBe(created.id);
160+
expect(countingStore.snapshotCreateIds).toEqual([created.id]);
161+
} finally {
162+
await engine.quit();
163+
}
164+
}
165+
);
166+
167+
// The transition-id invariant: each engine transition mints its OWN id, so two transitions never
168+
// collide; but replaying ONE transition (the same supplied id, as a blip-retry does) stays a single
169+
// row. This is what makes the store's conflict-ignoring insert safe: ids are per-transition, so a
170+
// replay is only ever the same transition, never different data reusing an id.
171+
containerTest(
172+
"distinct engine transitions mint distinct ids; a replayed store write retains one id",
173+
async ({ prisma, redisOptions }) => {
174+
const countingStore = new CountingPostgresRunStore({ prisma, readOnlyPrisma: prisma });
175+
const engine = new RunEngine(createEngineOptions(redisOptions, prisma, countingStore));
176+
177+
try {
178+
const run = await triggerRun(engine, prisma, "run_txids1");
179+
const latest = await getLatestExecutionSnapshot(prisma, run.id, countingStore);
180+
181+
const base = {
182+
run: { id: run.id, status: latest.runStatus, attemptNumber: latest.attemptNumber },
183+
environmentId: latest.environmentId,
184+
environmentType: latest.environmentType,
185+
projectId: latest.projectId,
186+
organizationId: latest.organizationId,
187+
};
188+
189+
// Two separate engine transitions receive two different ids (fresh mint per transition).
190+
const t1 = await engine.executionSnapshotSystem.createExecutionSnapshot(prisma, {
191+
...base,
192+
snapshot: { executionStatus: latest.executionStatus, description: "transition 1" },
193+
previousSnapshotId: latest.id,
194+
});
195+
const t2 = await engine.executionSnapshotSystem.createExecutionSnapshot(prisma, {
196+
...base,
197+
snapshot: { executionStatus: latest.executionStatus, description: "transition 2" },
198+
previousSnapshotId: t1.id,
199+
});
200+
expect(t1.id).not.toBe(t2.id);
201+
202+
// Replaying ONE store operation (the same supplied transition id) retains one id and one row.
203+
const transitionId = SnapshotId.generate().id;
204+
const input = {
205+
id: transitionId,
206+
run: { id: run.id, status: latest.runStatus, attemptNumber: latest.attemptNumber ?? 1 },
207+
snapshot: { executionStatus: latest.executionStatus, description: "replayed transition" },
208+
environmentId: latest.environmentId,
209+
environmentType: latest.environmentType,
210+
projectId: latest.projectId,
211+
organizationId: latest.organizationId,
212+
};
213+
const first = await countingStore.createExecutionSnapshot(input);
214+
const second = await countingStore.createExecutionSnapshot(input);
215+
216+
expect(first.id).toBe(transitionId);
217+
expect(second.id).toBe(transitionId);
218+
expect(await prisma.taskRunExecutionSnapshot.count({ where: { id: transitionId } })).toBe(
219+
1
220+
);
221+
} finally {
222+
await engine.quit();
223+
}
224+
}
225+
);
226+
133227
// getLatestExecutionSnapshot reads through the store, routed by run id.
134228
containerTest(
135229
"getLatestExecutionSnapshot reads through the store routed by run id",

internal-packages/run-engine/src/engine/systems/executionSnapshotSystem.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,7 @@ export class ExecutionSnapshotSystem {
437437
public async createExecutionSnapshot(
438438
prisma: PrismaClientOrTransaction,
439439
{
440+
snapshotId,
440441
run,
441442
snapshot,
442443
previousSnapshotId,
@@ -451,6 +452,12 @@ export class ExecutionSnapshotSystem {
451452
completedWaitpoints,
452453
error,
453454
}: {
455+
/**
456+
* Caller-supplied TRANSITION id: minted once per logical transition (e.g. so a publish guard can
457+
* arm keyed by it before the write) and NEVER reused for different snapshot data. The store insert
458+
* ignores conflicts, so a reused id keeps the first committed row and silently drops the new data.
459+
*/
460+
snapshotId?: string;
454461
run: { id: string; status: TaskRunStatus; attemptNumber?: number | null };
455462
snapshot: {
456463
executionStatus: TaskRunExecutionStatus;
@@ -478,8 +485,16 @@ export class ExecutionSnapshotSystem {
478485
// The heartbeat/eventBus side effects below are unchanged.
479486
store?: RunStore
480487
) {
488+
// Mint the snapshot id here (above the store) unless the caller supplied one, so a connection-blip
489+
// retry inside the store replays the SAME transition idempotently (the store's conflict-ignoring
490+
// insert keyed by the id, then a read) instead of duplicating it. The id is a transition id: minted
491+
// once per transition, never reused for different data. A caller supplies it only to arm a publish
492+
// guard keyed by the id before the write.
493+
const id = snapshotId ?? SnapshotId.generate().id;
494+
481495
const newSnapshot = await (store ?? this.$.runStore).createExecutionSnapshot(
482496
{
497+
id,
483498
run,
484499
snapshot,
485500
previousSnapshotId,

internal-packages/run-engine/src/engine/systems/waitpointSystem.test.ts

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,15 @@ class CountingRunStore extends PostgresRunStore {
8282
this.calls.push("updateManyWaitpoints");
8383
return super.updateManyWaitpoints(args, tx);
8484
}
85+
// The completion write goes through markWaitpointCompleted (the replay-safe completion method), so
86+
// it is the DB step the guard must precede on every completion route.
87+
override async markWaitpointCompleted(
88+
waitpointId: string,
89+
completion: Parameters<PostgresRunStore["markWaitpointCompleted"]>[1]
90+
): Promise<Prisma.BatchPayload> {
91+
this.calls.push("markWaitpointCompleted");
92+
return super.markWaitpointCompleted(waitpointId, completion);
93+
}
8594
override async findManyTaskRunWaitpoints<T extends Prisma.TaskRunWaitpointFindManyArgs>(
8695
args: Prisma.SelectSubset<T, Prisma.TaskRunWaitpointFindManyArgs>,
8796
client?: any
@@ -753,10 +762,15 @@ describe("WaitpointSystem completion fan-out + residency store-selection guard",
753762
// than) the first completion DB write on the same store.
754763
function expectGuardFiredBeforeUpdate(calls: string[]) {
755764
const guardIdx = calls.indexOf("forWaitpointCompletion");
756-
const updateIdx = calls.indexOf("updateManyWaitpoints");
765+
// The completion DB write is markWaitpointCompleted (some routes may also touch
766+
// updateManyWaitpoints); take whichever completion write lands first.
767+
const writeIdx = ["markWaitpointCompleted", "updateManyWaitpoints"]
768+
.map((m) => calls.indexOf(m))
769+
.filter((i) => i >= 0)
770+
.sort((a, b) => a - b)[0];
757771
expect(guardIdx).toBeGreaterThanOrEqual(0);
758-
expect(updateIdx).toBeGreaterThanOrEqual(0);
759-
expect(guardIdx).toBeLessThanOrEqual(updateIdx);
772+
expect(writeIdx).toBeGreaterThanOrEqual(0);
773+
expect(guardIdx).toBeLessThanOrEqual(writeIdx);
760774
}
761775

762776
// ----- Group 1: EXHAUSTIVE route enumeration -----
@@ -781,10 +795,10 @@ describe("WaitpointSystem completion fan-out + residency store-selection guard",
781795

782796
expect(store.calls).toContain("forWaitpointCompletion");
783797
const guardIdx = store.calls.indexOf("forWaitpointCompletion");
784-
const updateIdx = store.calls.indexOf("updateManyWaitpoints");
785-
// Synchronous route: guard is strictly the first DB step.
798+
const writeIdx = store.calls.indexOf("markWaitpointCompleted");
799+
// Synchronous route: guard is strictly the first DB step, before the completion write.
786800
expect(guardIdx).toBe(0);
787-
expect(updateIdx).toBeGreaterThan(guardIdx);
801+
expect(writeIdx).toBeGreaterThan(guardIdx);
788802
} finally {
789803
await engine.quit();
790804
}

internal-packages/run-engine/src/engine/waitpointCoordinator/legacyPostgresCoordinator.ts

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -147,17 +147,10 @@ export class LegacyPostgresWaitpointCoordinator implements WaitpointCoordinator
147147
}
148148

149149
// 1. Complete the Waitpoint (if not completed)
150+
// Replay-safe completion: the store builds the PENDING guard and COMPLETED values internally and
151+
// retries this (and only this) waitpoint-update path on a connection blip.
150152
const [updateError, updateResult] = await tryCatch(
151-
store.updateManyWaitpoints({
152-
where: { id: waitpointId, status: "PENDING" },
153-
data: {
154-
status: "COMPLETED",
155-
completedAt: new Date(),
156-
output: output?.value,
157-
outputType: output?.type,
158-
outputIsError: output?.isError,
159-
},
160-
})
153+
store.markWaitpointCompleted(waitpointId, { output })
161154
);
162155

163156
if (updateError) {
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
// #maybeInfraRetry must charge a blip-retry to the correct budget: replica reads to the replica
2+
// budget (so a replica retry storm can't drain the writer's), writer ops to the writer budget, and a
3+
// caller-transaction client is never retried. Selection is by the stored read-only object OR the
4+
// replica brand, so a routing-layer-forwarded replica wrapper (a fresh object, not the stored one)
5+
// still bills the replica budget. Deterministic one-shot fault injection, real testcontainer DB.
6+
import { postgresTest } from "@internal/testcontainers";
7+
import type { PrismaClient } from "@trigger.dev/database";
8+
import { generateInternalId } from "@trigger.dev/core/v3/isomorphic";
9+
import { expect } from "vitest";
10+
import { PostgresRunStore } from "./PostgresRunStore.js";
11+
import { setupSnapshotIdFixture } from "./testFixtures/snapshotIdFixture.js";
12+
import { withOneShotBlip } from "./testFixtures/oneShotBlip.js";
13+
import { markReadReplicaClient } from "./readReplicaClient.js";
14+
15+
const options = { enabled: true, maxAttempts: 12, backoffMinMs: 20, backoffMaxMs: 120 };
16+
17+
async function seedWaitpoint(prisma: PrismaClient, env: { id: string; projectId: string }) {
18+
const id = generateInternalId();
19+
await prisma.waitpoint.create({
20+
data: {
21+
id,
22+
friendlyId: `wp_${id}`,
23+
type: "MANUAL",
24+
status: "PENDING",
25+
idempotencyKey: `idem_${id}`,
26+
userProvidedIdempotencyKey: false,
27+
projectId: env.projectId,
28+
environmentId: env.id,
29+
},
30+
});
31+
return id;
32+
}
33+
34+
postgresTest(
35+
"a branded replica wrapper (not the stored one) charges its retry to the replica budget",
36+
async ({ prisma }) => {
37+
const { env } = await setupSnapshotIdFixture(prisma);
38+
const wpId = await seedWaitpoint(prisma as PrismaClient, env);
39+
40+
let writer = 0;
41+
let replica = 0;
42+
const store = new PostgresRunStore({
43+
prisma: prisma as never,
44+
readOnlyPrisma: prisma as never,
45+
infraRetry: { options, onRetry: () => writer++ },
46+
readInfraRetry: { options, onRetry: () => replica++ },
47+
});
48+
49+
// A freshly wrapped, branded replica handle: a different object than the stored readOnlyPrisma, so
50+
// the old identity-only check would have billed the writer. The brand routes it to the replica.
51+
const branded = markReadReplicaClient(withOneShotBlip(prisma, "waitpoint", "findFirst"));
52+
const wp = await store.findWaitpoint({ where: { id: wpId } }, branded as never);
53+
54+
expect(wp?.id).toBe(wpId);
55+
expect(replica).toBe(1);
56+
expect(writer).toBe(0);
57+
}
58+
);
59+
60+
postgresTest(
61+
"an op through the stored writer charges its retry to the writer budget",
62+
async ({ prisma }) => {
63+
const { env } = await setupSnapshotIdFixture(prisma);
64+
const wpId = await seedWaitpoint(prisma as PrismaClient, env);
65+
66+
let writer = 0;
67+
let replica = 0;
68+
const faultingWriter = withOneShotBlip(prisma, "waitpoint", "updateMany");
69+
const store = new PostgresRunStore({
70+
prisma: faultingWriter as never,
71+
readOnlyPrisma: prisma as never, // distinct stored replica so identity selection is meaningful
72+
infraRetry: { options, onRetry: () => writer++ },
73+
readInfraRetry: { options, onRetry: () => replica++ },
74+
});
75+
76+
// markWaitpointCompleted is the retried writer op (updateManyWaitpoints is not retried); it runs
77+
// waitpoint.updateMany on the writer, which the one-shot blip faults once, charging the writer budget.
78+
await store.markWaitpointCompleted(wpId, {
79+
output: { value: "{}", type: "application/json", isError: false },
80+
});
81+
82+
expect(writer).toBe(1);
83+
expect(replica).toBe(0);
84+
}
85+
);
86+
87+
postgresTest(
88+
"a read through the stored replica charges its retry to the replica budget",
89+
async ({ prisma }) => {
90+
const { env } = await setupSnapshotIdFixture(prisma);
91+
const wpId = await seedWaitpoint(prisma as PrismaClient, env);
92+
93+
let writer = 0;
94+
let replica = 0;
95+
const faultingReplica = withOneShotBlip(prisma, "waitpoint", "findFirst");
96+
const store = new PostgresRunStore({
97+
prisma: prisma as never,
98+
readOnlyPrisma: faultingReplica as never,
99+
infraRetry: { options, onRetry: () => writer++ },
100+
readInfraRetry: { options, onRetry: () => replica++ },
101+
});
102+
103+
const wp = await store.findWaitpoint({ where: { id: wpId } }); // no client -> stored readOnlyPrisma
104+
105+
expect(wp?.id).toBe(wpId);
106+
expect(replica).toBe(1);
107+
expect(writer).toBe(0);
108+
}
109+
);
110+
111+
postgresTest(
112+
"a caller-transaction client is never retried (boundary preserved for both budgets)",
113+
async ({ prisma }) => {
114+
const { env } = await setupSnapshotIdFixture(prisma);
115+
const wpId = await seedWaitpoint(prisma as PrismaClient, env);
116+
117+
let writer = 0;
118+
let replica = 0;
119+
const faulting = withOneShotBlip(prisma, "waitpoint", "findFirst");
120+
const store = new PostgresRunStore({
121+
prisma: prisma as never,
122+
readOnlyPrisma: prisma as never,
123+
infraRetry: { options, onRetry: () => writer++ },
124+
readInfraRetry: { options, onRetry: () => replica++ },
125+
});
126+
127+
// A tx client has no `$transaction`, so the op runs exactly once: the blip surfaces and neither
128+
// budget's onRetry fires.
129+
await expect(
130+
(faulting as any).$transaction((tx: any) => store.findWaitpoint({ where: { id: wpId } }, tx))
131+
).rejects.toThrow();
132+
133+
expect(writer).toBe(0);
134+
expect(replica).toBe(0);
135+
}
136+
);

0 commit comments

Comments
 (0)