Skip to content

Commit cc72e87

Browse files
d-csclaude
andcommitted
fix(run-store): keep caller replica reads on a replica; escalate only writer/tx
Review follow-up to the read-your-writes routing fix. The presence-only signal (`client != null`) escalated EVERY caller-passed read client to the owning store's primary, including an explicit read replica (e.g. `$replica`) passed by span/trace/session lookups and the run-ops read-through — defeating replica scaling. Writer and replica Prisma clients are structurally identical at runtime (a replica is a `new PrismaClient(...)` too, so it also exposes `$transaction`), so shape can't tell them apart. The client builder now brands replica handles (`markReadReplicaClient`) and the routing store reads the brand: a writer/tx still escalates to the owning primary (read-your-writes, incl. the dequeue hot path), while a branded replica or no client keeps the owning store's replica. Also probe the PRIMARY in `forWaitpointCompletion`'s store-resolution (it selects the store a subsequent write lands on), mirroring `#resolveWaitpointStore` — a replica-lagged probe could mis-resolve the owner and strand the run. Tests: branded-replica-stays-on-replica vs writer-escalates (routed reads); and forWaitpointCompletion resolving the owner under replica lag. Seed the snapshot↔ waitpoint link via the Prisma relation API instead of a raw join-table insert. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 40fd064 commit cc72e87

6 files changed

Lines changed: 144 additions & 19 deletions

File tree

apps/webapp/app/db.server.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
type PrismaTransactionOptions,
99
} from "@trigger.dev/database";
1010
import { RunOpsPrismaClient } from "@internal/run-ops-database";
11+
import { markReadReplicaClient } from "@internal/run-store";
1112
import invariant from "tiny-invariant";
1213
import { z } from "zod";
1314
import { env } from "./env.server";
@@ -170,7 +171,11 @@ export const prisma = singleton("prisma", () =>
170171

171172
export const $replica: PrismaReplicaClient = singleton("replica", () => {
172173
const replica = getReplicaClient();
173-
return replica ? captureInfrastructureErrors(tagDatasource("replica", replica)) : prisma;
174+
// Brand ONLY a real replica so the run-store routing layer keeps replica reads off the primary.
175+
// No replica configured → fall back to the writer `prisma`, which must stay UNBRANDED.
176+
return replica
177+
? markReadReplicaClient(captureInfrastructureErrors(tagDatasource("replica", replica)))
178+
: prisma;
174179
});
175180

176181
export type RunOpsClients = { writer: PrismaClient; replica: PrismaReplicaClient };
@@ -254,9 +259,14 @@ const runOpsTopology: RunOpsTopology = singleton("runOpsTopology", () => {
254259
captureInfraErrorsRunOps(
255260
tagDatasourceRunOps("writer", buildRunOpsWriterClient({ url, clientType }))
256261
),
262+
// Brand the run-ops replica (only built for a real replica URL) so routed replica reads stay
263+
// off the primary. When no replica URL is set, selectRunOpsTopology reuses the writer here —
264+
// which this callback never touches, so the writer stays unbranded.
257265
buildNewReplica: (url, clientType) =>
258-
captureInfraErrorsRunOps(
259-
tagDatasourceRunOps("replica", buildRunOpsReplicaClient({ url, clientType }))
266+
markReadReplicaClient(
267+
captureInfraErrorsRunOps(
268+
tagDatasourceRunOps("replica", buildRunOpsReplicaClient({ url, clientType }))
269+
)
260270
),
261271
}
262272
);
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
export * from "./types.js";
22
export * from "./PostgresRunStore.js";
33
export * from "./runOpsStore.js";
4+
export * from "./readReplicaClient.js";
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// A writer and a read-replica Prisma client are structurally identical at runtime (a replica is a
2+
// `new PrismaClient(...)` too, so it also exposes `$transaction`). The routing layer therefore
3+
// cannot tell a caller-passed replica from a writer by shape, yet it must — a writer/tx read has to
4+
// reach the owning store's PRIMARY (read-your-writes) while a replica read stays on a replica (read
5+
// scaling). So the client builder brands replica handles and the routing store reads the brand.
6+
export const READ_REPLICA_BRAND: unique symbol = Symbol.for("trigger.dev/run-store/read-replica");
7+
8+
// Brand a replica client (returns the same object). MUST only be called on a genuine replica: the
9+
// routing layer trusts the brand to mean "do not escalate this read to the primary". An unbranded
10+
// replica just escalates as before — a scaling miss, never a correctness bug.
11+
export function markReadReplicaClient<T extends object>(client: T): T {
12+
try {
13+
(client as Record<symbol, unknown>)[READ_REPLICA_BRAND] = true;
14+
} catch {
15+
// Frozen/exotic clients may reject the assignment; only costs the optimization, not correctness.
16+
}
17+
return client;
18+
}
19+
20+
export function isReadReplicaClient(client: unknown): boolean {
21+
return (
22+
!!client &&
23+
typeof client === "object" &&
24+
(client as Record<symbol, unknown>)[READ_REPLICA_BRAND] === true
25+
);
26+
}

internal-packages/run-store/src/runOpsStore.routedReadPrimary.test.ts

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import type { PrismaClient } from "@trigger.dev/database";
1616
import { describe, expect } from "vitest";
1717
import { PostgresRunStore } from "./PostgresRunStore.js";
1818
import { RoutingRunStore } from "./runOpsStore.js";
19+
import { markReadReplicaClient } from "./readReplicaClient.js";
1920
import type { CreateRunInput } from "./types.js";
2021

2122
// ownerEngine classifies by internal-id LENGTH: 25 chars → cuid → LEGACY, 27 → ksuid → NEW.
@@ -175,9 +176,12 @@ describe("run-ops split — routed reads honor a caller-passed client via the ow
175176
environmentId: seed.environment.id,
176177
},
177178
});
178-
await prisma14.$executeRaw`
179-
INSERT INTO "_completedWaitpoints" ("A", "B") VALUES (${snapshotId}, ${waitpoint.id})
180-
`;
179+
// Link via the Prisma relation API (not a raw insert into the implicit join table) so a
180+
// relation rename fails at compile time rather than silently seeding nothing.
181+
await prisma14.taskRunExecutionSnapshot.update({
182+
where: { id: snapshotId },
183+
data: { completedWaitpoints: { connect: { id: waitpoint.id } } },
184+
});
181185
expect(await router.findSnapshotCompletedWaitpointIds(snapshotId, prisma14)).toEqual([
182186
waitpoint.id,
183187
]);
@@ -263,6 +267,51 @@ describe("run-ops split — routed reads honor a caller-passed client via the ow
263267
}
264268
);
265269

270+
// Read scaling: a caller that passes an explicit READ REPLICA (e.g. `$replica`) must NOT be
271+
// escalated to the primary — only true read-your-writes (a writer/tx) should. A replica is a
272+
// full PrismaClient at runtime (it has `$transaction` too), so shape can't distinguish it; the
273+
// client builder brands it and the router honors the brand. Proven here by branding a client and
274+
// showing the read stays on the owning store's (empty) replica — same as passing no client —
275+
// while an unbranded writer escalates and finds the fresh row.
276+
heteroPostgresTest(
277+
"a branded read-replica client stays on the replica; a writer escalates to the primary",
278+
async ({ prisma14, prisma17 }) => {
279+
const { router } = splitTopology("LEGACY", prisma14, prisma17);
280+
const seed = await seedEnvironment(prisma14, "replica_leg");
281+
const runId = `run_${CUID_25}`;
282+
await router.createRun(
283+
buildCreateRunInput({
284+
runId,
285+
friendlyId: "run_replica_leg",
286+
organizationId: seed.organization.id,
287+
projectId: seed.project.id,
288+
runtimeEnvironmentId: seed.environment.id,
289+
})
290+
);
291+
292+
// The router discards the caller's client object (it can't cross DBs) and reads the brand
293+
// only, so a branded marker faithfully stands in for a passed `$replica`.
294+
const replicaClient = markReadReplicaClient({} as unknown as PrismaClient);
295+
296+
// findRun (readYourWrites path): branded replica → owning replica (empty) → miss.
297+
expect(
298+
await router.findRun({ id: runId }, { select: { id: true } }, replicaClient)
299+
).toBeNull();
300+
// Control: an unbranded writer escalates to the owning primary → finds the fresh row.
301+
expect((await router.findRun({ id: runId }, { select: { id: true } }, prisma14))?.id).toBe(
302+
runId
303+
);
304+
305+
// findLatestExecutionSnapshot (#ownPrimary path): same replica-stays-on-replica invariant.
306+
expect(await router.findLatestExecutionSnapshot(runId, replicaClient)).toBeNull();
307+
expect((await router.findLatestExecutionSnapshot(runId, prisma14))?.executionStatus).toBe(
308+
"RUN_CREATED"
309+
);
310+
// No client behaves identically to the branded replica.
311+
expect(await router.findLatestExecutionSnapshot(runId)).toBeNull();
312+
}
313+
);
314+
266315
heteroPostgresTest(
267316
"LEGACY cuid: waitpoint reads honor a caller client",
268317
async ({ prisma14, prisma17 }) => {

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1257,6 +1257,39 @@ describe("RoutingRunStore.findRuns split-mode fan-out + drain", () => {
12571257
}
12581258
);
12591259

1260+
// forWaitpointCompletion selects the store a subsequent WRITE (updateManyWaitpoints) lands on,
1261+
// so its resolution probe must read each store's PRIMARY — not the replica. Here the owning
1262+
// (NEW) store's replica lags (points at an empty DB), so a replica probe would miss the fresh
1263+
// waitpoint and mis-resolve to the id-shape's default (LEGACY), stranding the run.
1264+
heteroPostgresTest(
1265+
"forWaitpointCompletion probes the primary, resolving the owner even under replica lag",
1266+
async ({ prisma14, prisma17 }) => {
1267+
const legacyStore = new PostgresRunStore({ prisma: prisma14, readOnlyPrisma: prisma14 });
1268+
// NEW store writes to prisma17 but its replica is the (empty, w.r.t. this waitpoint) prisma14.
1269+
const newStore = new PostgresRunStore({ prisma: prisma17, readOnlyPrisma: prisma14 });
1270+
const router = new RoutingRunStore({ new: newStore, legacy: legacyStore });
1271+
const seed17 = await seedEnvironment(prisma17, "wpc_lag17");
1272+
1273+
const cuidWaitpointId = `waitpoint_${"c".repeat(25)}`; // id-shape → LEGACY (the wrong owner)
1274+
await prisma17.waitpoint.create({
1275+
data: {
1276+
id: cuidWaitpointId,
1277+
friendlyId: "waitpoint_wpc_lag",
1278+
type: "MANUAL",
1279+
idempotencyKey: "wpc-lag-key",
1280+
userProvidedIdempotencyKey: false,
1281+
projectId: seed17.project.id,
1282+
environmentId: seed17.environment.id,
1283+
},
1284+
});
1285+
1286+
// Only the NEW store's PRIMARY (prisma17) has the row; a replica probe (prisma14) misses it.
1287+
expect(await router.forWaitpointCompletion(cuidWaitpointId, { routeKind: "MANUAL" })).toBe(
1288+
newStore
1289+
);
1290+
}
1291+
);
1292+
12601293
// A waitpoint must be born on the same DB as its run (cuid → LEGACY, ksuid → NEW) so that
12611294
// completion and the blocking edge — which already routes by run id — line up. A cuid
12621295
// waitpoint landing on NEW is the regression that strands a non-opted org's wait forever.

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

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import type {
2626
TaskRunWithWaitpoint,
2727
WaitpointColocationOptions,
2828
} from "./types.js";
29+
import { isReadReplicaClient } from "./readReplicaClient.js";
2930

3031
/**
3132
* Run-ops routing substrate for the TaskRun-core method group. Implements {@link RunStore}
@@ -57,11 +58,11 @@ export class RoutingRunStore implements RunStore {
5758

5859
// Map a caller-passed read client onto a routed store. The caller's client is bound to the
5960
// control-plane connection — the wrong database for a NEW-resident row — so it is never forwarded
60-
// verbatim. Its PRESENCE is the read-your-writes signal (mirroring `client ?? readOnlyPrisma` in
61-
// PostgresRunStore): the routed read runs on the owning store's OWN primary; no client keeps the
62-
// store's default.
61+
// verbatim. A WRITER/tx signals read-your-writes (the just-written row must beat replica lag), so
62+
// the routed read runs on the owning store's OWN primary. A caller-passed REPLICA (branded) or no
63+
// client keeps the owning store's replica, preserving read scaling.
6364
static #ownPrimary(store: RunStore, client: ReadClient | undefined): ReadClient | undefined {
64-
return client === undefined ? undefined : store.primaryReadClient;
65+
return client != null && !isReadReplicaClient(client) ? store.primaryReadClient : undefined;
6566
}
6667

6768
// An unclassifiable id is treated as LEGACY (probe the control-plane DB rather than drop a
@@ -1174,12 +1175,17 @@ export class RoutingRunStore implements RunStore {
11741175
: this.#legacy;
11751176
// Resolve to where the waitpoint ACTUALLY lives: a migrated run's waitpoint can be on NEW
11761177
// with a LEGACY-classified id (or vice versa), so verify and fall back rather than route
1177-
// by id-shape alone and miss it (which leaves the blocked run stuck forever).
1178-
if (await preferred.findWaitpoint({ where: { id: waitpointId } })) {
1178+
// by id-shape alone and miss it (which leaves the blocked run stuck forever). This guard
1179+
// selects the store a WRITE (updateManyWaitpoints) then lands on, so it must probe each
1180+
// store's PRIMARY (mirroring #resolveWaitpointStore's onPrimary): a just-created waitpoint the
1181+
// replica hasn't caught up on would otherwise mis-resolve the owner and strand the run.
1182+
if (
1183+
await preferred.findWaitpoint({ where: { id: waitpointId } }, preferred.primaryReadClient)
1184+
) {
11791185
return preferred;
11801186
}
11811187
const other = preferred === this.#new ? this.#legacy : this.#new;
1182-
if (await other.findWaitpoint({ where: { id: waitpointId } })) {
1188+
if (await other.findWaitpoint({ where: { id: waitpointId } }, other.primaryReadClient)) {
11831189
return other;
11841190
}
11851191
return preferred;
@@ -1524,18 +1530,18 @@ function selectOrIncludeArgs(
15241530
return undefined;
15251531
}
15261532

1527-
// A read-your-writes call passes a client — the writer or an ambient tx, whose just-written row
1528-
// must be read back before the replica has it. Recover the caller's client from the overloaded
1529-
// read args — slot two when it isn't a `{ select | include }` object, else slot three — and report
1530-
// whether one was passed. (Its presence, not its identity, is the signal: even an explicitly
1531-
// passed replica handle can't be forwarded across DBs, so it resolves to the owning primary.)
1533+
// A read-your-writes call passes a WRITER or ambient tx, whose just-written row must be read back
1534+
// before the replica has it. Recover the caller's client from the overloaded read args — slot two
1535+
// when it isn't a `{ select | include }` object, else slot three — and report whether it warrants
1536+
// escalation to the owning primary. A branded replica does NOT: it can't be forwarded across DBs,
1537+
// but it signals a replica-intended read, so the owning store keeps its own replica (read scaling).
15321538
function readYourWrites(
15331539
argsOrClient: { select?: unknown; include?: unknown } | ReadClient | unknown,
15341540
client: ReadClient | undefined
15351541
): boolean {
15361542
const passedClient =
15371543
selectOrIncludeArgs(argsOrClient) === undefined ? (argsOrClient ?? client) : client;
1538-
return passedClient != null;
1544+
return passedClient != null && !isReadReplicaClient(passedClient);
15391545
}
15401546

15411547
// Read a plain scalar string field off a create-data object (e.g. `data.completedByTaskRunId`).

0 commit comments

Comments
 (0)