Skip to content

Commit 6e976d0

Browse files
committed
feat(run-store): drop the unimplemented compare dial position, cover redis-only
`compare` was a name in the dial with no behaviour behind it: it wrote and read exactly as `dual-write` does, so turning it on would have looked like enabling divergence reporting and delivered plain dual-write. A dial value that silently does something other than its name is worse than a missing one. It returns with the ticket that implements the sampled dual-read and diff. `redis-only` was the thinnest tested position and the only one that cannot be rolled back, since the snapshots written while it is on exist nowhere else. It is also the only position that is a PAIR of settings, the decorator's mode and `snapshotWrites: false` on the store beneath it, and the previous single test used a store that still wrote snapshots. Every test in the new suite builds the pair, and covers the run mutation landing without its snapshot row, transitions, completions, the absent waitpoint join rows, and every read being Redis-served. One test characterises rather than endorses: a read shape the decorator does not recognise is delegated, and at this position Postgres holds nothing, so the caller gets an empty result rather than an error. Only the engine calls that method and it issues the recognised shape, so nothing is broken today. It is pinned so the terminal-cutover ticket decides deliberately whether a fall-through here should throw instead of answering empty.
1 parent 1bba0a0 commit 6e976d0

4 files changed

Lines changed: 329 additions & 5 deletions

internal-packages/run-store/src/taskRunExecutionSnapshotStore.off.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ describe("TaskRunExecutionSnapshotStore at mode off", () => {
8787

8888
it("reports every other dial position as one that writes Redis", () => {
8989
const { store } = forwardingProbe();
90-
const modes = ["dual-write", "compare", "redis-read", "redis-only"] as const;
90+
const modes = ["dual-write", "redis-read", "redis-only"] as const;
9191

9292
for (const mode of modes) {
9393
const decorated = new TaskRunExecutionSnapshotStore(store, {

internal-packages/run-store/src/taskRunExecutionSnapshotStore.readCohort.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const ids = Array.from({ length: 500 }, (_, n) => `run_cohort_${n}_${n * 7919}`)
2828

2929
describe("the read cohort", () => {
3030
it("reads nothing from Redis before the read positions", () => {
31-
for (const mode of ["off", "dual-write", "compare"] as const) {
31+
for (const mode of ["off", "dual-write"] as const) {
3232
const store = probe(mode, 100);
3333
expect(ids.every((id) => !store.readsFromRedis(id))).toBe(true);
3434
}
Lines changed: 321 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,321 @@
1+
// `redis-only` is the terminal cutover, and it is the only dial position where Postgres stops being
2+
// authoritative: it cannot be rolled back by turning the dial down, because the snapshots written
3+
// while it was on exist nowhere else. It is also the only position that is a PAIR of settings, not
4+
// one — the decorator's mode AND `snapshotWrites: false` on the store underneath it — and the two
5+
// are set by different tickets. Every test here builds the pair, because testing the mode against a
6+
// store that still writes snapshots would exercise a configuration that never ships.
7+
import { describe, expect } from "vitest";
8+
import { containerTest } from "@internal/testcontainers";
9+
import { generateInternalId } from "@trigger.dev/core/v3/isomorphic";
10+
import { PostgresRunStore } from "./PostgresRunStore.js";
11+
import { RedisSnapshotStore } from "./redisSnapshotStore.js";
12+
import { TaskRunExecutionSnapshotStore } from "./taskRunExecutionSnapshotStore.js";
13+
import type { RunStore } from "./types.js";
14+
import {
15+
buildCreateRunData,
16+
seedSnapshotEnvironment,
17+
seedSnapshotWaitpoints,
18+
type SnapshotFixtureEnv,
19+
} from "./testFixtures/snapshotIdFixture.js";
20+
21+
const COMPLETED_TTL_MS = 72 * 60 * 60 * 1000;
22+
23+
/** The shipping pair: decorator at `redis-only` over a store that writes no snapshot rows. */
24+
function build(prisma: never, redisOptions: never) {
25+
const redis = new RedisSnapshotStore({ redisOptions, completedTtlMs: COMPLETED_TTL_MS });
26+
const reads: { method: string; source: string }[] = [];
27+
28+
const decorated = new TaskRunExecutionSnapshotStore(
29+
new PostgresRunStore({
30+
prisma,
31+
readOnlyPrisma: prisma,
32+
snapshotWrites: false,
33+
}) as unknown as RunStore,
34+
{
35+
store: redis,
36+
mode: "redis-only",
37+
metrics: {
38+
recordWrite: () => {},
39+
recordAppendFailed: () => {},
40+
recordRead: (method, source) => reads.push({ method, source }),
41+
},
42+
}
43+
);
44+
45+
return { decorated, redis, reads };
46+
}
47+
48+
function birth(env: SnapshotFixtureEnv, id: string) {
49+
return {
50+
id,
51+
engine: "V2" as const,
52+
executionStatus: "RUN_CREATED" as const,
53+
description: "Run was created",
54+
runStatus: "PENDING" as const,
55+
environmentId: env.id,
56+
environmentType: env.type,
57+
projectId: env.projectId,
58+
organizationId: env.organizationId,
59+
};
60+
}
61+
62+
async function seedRun(decorated: TaskRunExecutionSnapshotStore, env: SnapshotFixtureEnv) {
63+
const runId = generateInternalId();
64+
const snapshotId = generateInternalId();
65+
await decorated.createRun({
66+
data: buildCreateRunData(runId, env),
67+
snapshot: birth(env, snapshotId),
68+
});
69+
return { runId, snapshotId };
70+
}
71+
72+
function transition(runId: string, env: SnapshotFixtureEnv, description: string) {
73+
return {
74+
run: { id: runId, status: "EXECUTING" as const, attemptNumber: 1 },
75+
snapshot: { executionStatus: "EXECUTING" as const, description },
76+
environmentId: env.id,
77+
environmentType: env.type,
78+
projectId: env.projectId,
79+
organizationId: env.organizationId,
80+
};
81+
}
82+
83+
describe("redis-only: Postgres stops holding snapshots", () => {
84+
containerTest("the run row lands but no snapshot row does", async ({ prisma, redisOptions }) => {
85+
const { decorated, redis } = build(prisma as never, redisOptions as never);
86+
try {
87+
const env = await seedSnapshotEnvironment(prisma);
88+
const { runId, snapshotId } = await seedRun(decorated, env);
89+
90+
// The run itself is still Postgres-authoritative at this position. Only its snapshots move.
91+
expect(await prisma.taskRun.count({ where: { id: runId } })).toBe(1);
92+
expect(await prisma.taskRunExecutionSnapshot.count({ where: { runId } })).toBe(0);
93+
94+
// And the snapshot is genuinely in Redis under the id the caller minted.
95+
const head = await redis.getLatest(runId);
96+
expect(head?.id).toBe(snapshotId);
97+
} finally {
98+
await redis.quit();
99+
}
100+
});
101+
102+
containerTest("transitions write no snapshot row either", async ({ prisma, redisOptions }) => {
103+
const { decorated, redis } = build(prisma as never, redisOptions as never);
104+
try {
105+
const env = await seedSnapshotEnvironment(prisma);
106+
const { runId } = await seedRun(decorated, env);
107+
108+
await decorated.createExecutionSnapshot(transition(runId, env, "Run started"));
109+
await decorated.createExecutionSnapshot(transition(runId, env, "Run continued"));
110+
111+
expect(await prisma.taskRunExecutionSnapshot.count({ where: { runId } })).toBe(0);
112+
const since = await redis.getSinceCreatedAt(runId, new Date(Date.now() - 60_000), {
113+
limit: 50,
114+
});
115+
expect(since.kind).toBe("hit");
116+
expect(since.kind === "hit" ? since.entries.length : 0).toBeGreaterThanOrEqual(2);
117+
} finally {
118+
await redis.quit();
119+
}
120+
});
121+
122+
containerTest(
123+
"a completion still updates the run row while writing no snapshot",
124+
async ({ prisma, redisOptions }) => {
125+
const { decorated, redis } = build(prisma as never, redisOptions as never);
126+
try {
127+
const env = await seedSnapshotEnvironment(prisma);
128+
const { runId } = await seedRun(decorated, env);
129+
130+
await decorated.completeAttemptSuccess(
131+
runId,
132+
{
133+
completedAt: new Date(),
134+
outputType: "application/json",
135+
usageDurationMs: 1,
136+
costInCents: 0,
137+
snapshot: {
138+
id: generateInternalId(),
139+
executionStatus: "FINISHED",
140+
description: "Run completed",
141+
runStatus: "COMPLETED_SUCCESSFULLY",
142+
attemptNumber: 1,
143+
environmentId: env.id,
144+
environmentType: env.type,
145+
projectId: env.projectId,
146+
organizationId: env.organizationId,
147+
},
148+
},
149+
{ select: { id: true } }
150+
);
151+
152+
// The mutation half of a nested write must still land, or the run never finishes.
153+
const run = await prisma.taskRun.findFirstOrThrow({ where: { id: runId } });
154+
expect(run.status).toBe("COMPLETED_SUCCESSFULLY");
155+
expect(await prisma.taskRunExecutionSnapshot.count({ where: { runId } })).toBe(0);
156+
} finally {
157+
await redis.quit();
158+
}
159+
}
160+
);
161+
162+
containerTest(
163+
"no completed-waitpoint join rows are written for a snapshot Postgres does not have",
164+
async ({ prisma, redisOptions }) => {
165+
// The join rows point at a snapshot row. With snapshot writes off there is no such row, so
166+
// inserting them would leave links dangling at a snapshot only Redis holds.
167+
const { decorated, redis } = build(prisma as never, redisOptions as never);
168+
try {
169+
const env = await seedSnapshotEnvironment(prisma);
170+
const { runId } = await seedRun(decorated, env);
171+
const [wpA, wpB] = await seedSnapshotWaitpoints(prisma, env, 2);
172+
173+
await decorated.createExecutionSnapshot({
174+
...transition(runId, env, "Run resumed"),
175+
completedWaitpoints: [
176+
{ id: wpA, index: 0 },
177+
{ id: wpB, index: 1 },
178+
],
179+
});
180+
181+
expect(await prisma.taskRunExecutionSnapshot.count({ where: { runId } })).toBe(0);
182+
const joins = await prisma.$queryRawUnsafe<{ n: bigint }[]>(
183+
`SELECT count(*) AS n FROM "_completedWaitpoints" WHERE "B" = ANY($1::text[])`,
184+
[wpA, wpB]
185+
);
186+
expect(Number(joins[0]!.n)).toBe(0);
187+
} finally {
188+
await redis.quit();
189+
}
190+
}
191+
);
192+
});
193+
194+
describe("redis-only: every read is served from Redis", () => {
195+
containerTest(
196+
"the hot read, the since window and the waitpoint lookups all come from Redis",
197+
async ({ prisma, redisOptions }) => {
198+
// At every earlier position a Redis miss falls back to Postgres and the caller never notices.
199+
// Here Postgres holds nothing, so a read that fell back would answer empty rather than wrong,
200+
// and a run would silently lose its state. Each read is asserted to be Redis-sourced.
201+
const { decorated, redis, reads } = build(prisma as never, redisOptions as never);
202+
try {
203+
const env = await seedSnapshotEnvironment(prisma);
204+
const { runId } = await seedRun(decorated, env);
205+
const [wpA] = await seedSnapshotWaitpoints(prisma, env, 1);
206+
const created = await decorated.createExecutionSnapshot({
207+
...transition(runId, env, "Run resumed"),
208+
completedWaitpoints: [{ id: wpA, index: 0 }],
209+
});
210+
211+
const latest = await decorated.findLatestExecutionSnapshot(runId);
212+
expect(latest!.id).toBe(created.id);
213+
expect(latest!.completedWaitpointOrder).toEqual([wpA]);
214+
215+
const window = await decorated.findManyExecutionSnapshots({
216+
where: { runId, isValid: true, createdAt: { gt: new Date(Date.now() - 60_000) } },
217+
include: { checkpoint: true },
218+
orderBy: { createdAt: "desc" },
219+
take: 50,
220+
});
221+
expect(window.length).toBeGreaterThan(0);
222+
223+
const withPresence = await decorated.findSnapshotCompletedWaitpointIdsWithPresence(
224+
created.id,
225+
undefined,
226+
runId
227+
);
228+
expect(withPresence.ids).toEqual([wpA]);
229+
230+
expect(reads.length).toBeGreaterThan(0);
231+
expect(reads.every((r) => r.source === "redis")).toBe(true);
232+
} finally {
233+
await redis.quit();
234+
}
235+
}
236+
);
237+
238+
containerTest(
239+
"an unrecognised read shape falls through to a Postgres that holds nothing",
240+
async ({ prisma, redisOptions }) => {
241+
// CHARACTERISATION, NOT AN ENDORSEMENT. `findManyExecutionSnapshots` serves from Redis only
242+
// for the since-window shape `matchSinceWindow` recognises; anything else delegates. At every
243+
// dial position before this one that is harmless, because Postgres holds the same rows. Here
244+
// it holds none, so the caller gets an EMPTY result rather than an error, and empty is a
245+
// valid answer to this query. The same is true of the `miss` and `danglingCycle` fallbacks in
246+
// that method: all three are safe everywhere except the one position that cannot fall back.
247+
//
248+
// Only the engine's own call shapes reach this method today, and it issues the since-window
249+
// one, so nothing is broken. It is pinned here so the terminal-cutover ticket decides
250+
// deliberately whether a fall-through at `redis-only` should throw instead of answering
251+
// empty, rather than discovering this shape in production.
252+
const { decorated, redis } = build(prisma as never, redisOptions as never);
253+
try {
254+
const env = await seedSnapshotEnvironment(prisma);
255+
const { runId } = await seedRun(decorated, env);
256+
await decorated.createExecutionSnapshot(transition(runId, env, "Run started"));
257+
258+
// No `createdAt` cursor, so the shape does not match and the read is delegated.
259+
const unmatched = await decorated.findManyExecutionSnapshots({
260+
where: { runId, isValid: true },
261+
include: { checkpoint: true },
262+
orderBy: { createdAt: "desc" },
263+
take: 50,
264+
});
265+
266+
expect(unmatched).toEqual([]);
267+
268+
// The same run, asked the shape the engine actually issues, answers in full from Redis.
269+
const matched = await decorated.findManyExecutionSnapshots({
270+
where: { runId, isValid: true, createdAt: { gt: new Date(Date.now() - 60_000) } },
271+
include: { checkpoint: true },
272+
orderBy: { createdAt: "desc" },
273+
take: 50,
274+
});
275+
expect(matched.length).toBeGreaterThan(0);
276+
} finally {
277+
await redis.quit();
278+
}
279+
}
280+
);
281+
282+
containerTest(
283+
"the read cohort dial cannot route a run away from Redis",
284+
async ({ prisma, redisOptions }) => {
285+
// readPercent is a ramp control for `redis-read`. At `redis-only` a run routed to Postgres
286+
// would read a database that holds no snapshots at all, so the dial must be ignored here
287+
// whatever it is set to.
288+
const redis = new RedisSnapshotStore({ redisOptions, completedTtlMs: COMPLETED_TTL_MS });
289+
const reads: string[] = [];
290+
const decorated = new TaskRunExecutionSnapshotStore(
291+
new PostgresRunStore({
292+
prisma: prisma as never,
293+
readOnlyPrisma: prisma as never,
294+
snapshotWrites: false,
295+
}) as unknown as RunStore,
296+
{
297+
store: redis,
298+
mode: "redis-only",
299+
readPercent: 0,
300+
metrics: {
301+
recordWrite: () => {},
302+
recordAppendFailed: () => {},
303+
recordRead: (_m, source) => reads.push(source),
304+
},
305+
}
306+
);
307+
308+
try {
309+
const env = await seedSnapshotEnvironment(prisma);
310+
const { runId, snapshotId } = await seedRun(decorated, env);
311+
312+
const latest = await decorated.findLatestExecutionSnapshot(runId);
313+
314+
expect(latest!.id).toBe(snapshotId);
315+
expect(reads).not.toContain("postgres");
316+
} finally {
317+
await redis.quit();
318+
}
319+
}
320+
);
321+
});

internal-packages/run-store/src/taskRunExecutionSnapshotStore.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,13 @@ const WAITPOINT_CHUNK_SIZE = 100;
6060
* The rollout dial. Postgres stays fully written and authoritative in every position before
6161
* `redis-only`, so every earlier position rolls back losslessly by turning the dial down.
6262
*
63-
* `compare` writes exactly as `dual-write` does. Its sampled dual-read and diff are a later ticket;
64-
* the position is named here so the dial does not have to widen once that lands.
63+
* A `compare` position was named here before its behaviour existed, and it read from this type as a
64+
* real dial position while behaving in every respect exactly like `dual-write`. A dial value that
65+
* silently does something other than its name is worse than a missing one: turning it on would have
66+
* looked like enabling divergence reporting and delivered plain dual-write. It is added back by the
67+
* ticket that implements the sampled dual-read and diff, at which point the name will be true.
6568
*/
66-
export type SnapshotStoreMode = "off" | "dual-write" | "compare" | "redis-read" | "redis-only";
69+
export type SnapshotStoreMode = "off" | "dual-write" | "redis-read" | "redis-only";
6770

6871
/**
6972
* Enqueues the existing `repairSnapshot` job for a run whose append was lost. The decorator lives in

0 commit comments

Comments
 (0)