Skip to content

Commit f101983

Browse files
authored
fix(run-store,run-engine): fix run-ops split hangs from wrong-store reads on the resume path (#4163)
## Summary On the run-ops split, NEW-residency runs could hang. Time-based waits (`wait.for`, `wait.until`, `delay`, waitpoint tokens), `batchTriggerAndWait`, and attempt starts stalled and never resumed. Each was a run-ops read or update that hit the wrong database: either the owning store's read replica when it needed read-your-writes, or the wrong store entirely because it routed by an id that does not encode residency. ## Fixes **Waitpoint resume (the main hang).** The managed resume path reads a run's completed waitpoints by snapshot id (`findSnapshotCompletedWaitpointIds`). Snapshot ids are cuids, which always classify to the legacy store, so a NEW run's join rows (which live on the new store) were never found. The resumed run saw zero completed waitpoints and hung. It now fans out across both stores and merges, like its sibling readers. **Batch completion.** Batch item completion (`updateManyBatchTaskRunItems`) routed by the item id, which is also a cuid, so a NEW batch's items were updated on the wrong store, matched zero rows, and the batch was treated as already complete (its parent's `batchTriggerAndWait` then hung). It now routes by the batch id, which does encode residency, matching the sibling `countBatchTaskRunItems`. **Read-your-writes on the resume path.** The block-time pending-waitpoint check (`countPendingWaitpoints`) and the attempt-start lock check (`findRun` in `startRunAttempt`) both read the owning store's replica with no read-your-writes guarantee, so a just-committed waitpoint completion or dequeue lock could be missed under replica lag and strand the run. Both now read the owning primary. Each fix ships with a two-database store or engine test that reproduces the hang and passes with the fix.
1 parent 712c7c3 commit f101983

20 files changed

Lines changed: 1884 additions & 70 deletions

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
} from "@trigger.dev/core/v3";
1111

1212
import { AttemptId, getMaxDuration, parseTraceparent } from "@trigger.dev/core/v3/isomorphic";
13+
import { $replica, runOpsNewReplica, runOpsSplitReadEnabled } from "~/db.server";
1314
import {
1415
extractIdempotencyKeyScope,
1516
getUserProvidedIdempotencyKey,
@@ -722,7 +723,11 @@ export class SpanPresenter extends BasePresenter {
722723
return { ...data, entity: null };
723724
}
724725

725-
const presenter = new WaitpointPresenter(undefined, undefined, {});
726+
const presenter = new WaitpointPresenter(undefined, undefined, {
727+
newClient: runOpsNewReplica,
728+
legacyReplica: $replica,
729+
splitEnabled: runOpsSplitReadEnabled,
730+
});
726731
const waitpoint = await presenter.call({
727732
friendlyId: span.entity.id,
728733
environmentId,

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

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,6 @@ export class WaitpointPresenter extends BasePresenter {
4545
completedAfter: true,
4646
completedAt: true,
4747
createdAt: true,
48-
connectedRuns: {
49-
select: {
50-
friendlyId: true,
51-
},
52-
take: 5,
53-
},
5448
tags: true,
5549
environmentId: true,
5650
} as const;
@@ -80,6 +74,66 @@ export class WaitpointPresenter extends BasePresenter {
8074
return result.source === "new" || result.source === "legacy-replica" ? result.value : null;
8175
}
8276

77+
// Connected-run friendlyIds gathered across BOTH stores. The run<->waitpoint join co-locates with
78+
// the RUN (written on the run's DB), so the waitpoint's own store misses a cross-DB connection; we
79+
// read the join on each client and resolve the run's friendlyId on that same client, then union.
80+
// We never relation-select `connectedRuns`: it is not a field on the dedicated subset `Waitpoint`.
81+
async #connectedRunFriendlyIds(waitpointId: string): Promise<string[]> {
82+
const replica = this._replica as unknown as PrismaReplicaClient;
83+
const rawClients: PrismaReplicaClient[] =
84+
this.readThroughDeps?.splitEnabled === true
85+
? [
86+
(this.readThroughDeps.newClient as PrismaReplicaClient | undefined) ?? replica,
87+
(this.readThroughDeps.legacyReplica as PrismaReplicaClient | undefined) ?? replica,
88+
]
89+
: [replica];
90+
const clients = [...new Set(rawClients)];
91+
92+
const friendlyIds = new Set<string>();
93+
for (const client of clients) {
94+
const runIds = await this.#connectedRunIdsOn(client, waitpointId);
95+
if (runIds.length === 0) {
96+
continue;
97+
}
98+
const runs = await client.taskRun.findMany({
99+
where: { id: { in: runIds } },
100+
select: { friendlyId: true },
101+
take: 5,
102+
});
103+
for (const run of runs) {
104+
friendlyIds.add(run.friendlyId);
105+
}
106+
if (friendlyIds.size >= 5) {
107+
break;
108+
}
109+
}
110+
return Array.from(friendlyIds).slice(0, 5);
111+
}
112+
113+
// Schema-aware read of the run ids linked to a waitpoint: the dedicated subset uses the explicit
114+
// `WaitpointRunConnection` model, the control-plane full schema the implicit `_WaitpointRunConnections`
115+
// M2M (A = TaskRun.id, B = Waitpoint.id). The dedicated join delegate is absent on the full client.
116+
async #connectedRunIdsOn(client: PrismaReplicaClient, waitpointId: string): Promise<string[]> {
117+
const joinDelegate = (
118+
client as unknown as {
119+
waitpointRunConnection?: {
120+
findMany: (args: unknown) => Promise<{ taskRunId: string }[]>;
121+
};
122+
}
123+
).waitpointRunConnection;
124+
if (joinDelegate && typeof joinDelegate.findMany === "function") {
125+
const links = await joinDelegate.findMany({
126+
where: { waitpointId },
127+
select: { taskRunId: true },
128+
});
129+
return links.map((link) => link.taskRunId);
130+
}
131+
const rows = await client.$queryRaw<{ A: string }[]>`
132+
SELECT "A" FROM "_WaitpointRunConnections" WHERE "B" = ${waitpointId}
133+
`;
134+
return rows.map((row) => row.A);
135+
}
136+
83137
public async call({
84138
friendlyId,
85139
environmentId,
@@ -119,7 +173,7 @@ export class WaitpointPresenter extends BasePresenter {
119173
}
120174
}
121175

122-
const connectedRunIds = waitpoint.connectedRuns.map((run) => run.friendlyId);
176+
const connectedRunIds = await this.#connectedRunFriendlyIds(waitpoint.id);
123177
const connectedRuns: NextRunListItem[] = [];
124178

125179
if (connectedRunIds.length > 0) {

apps/webapp/app/routes/api.v1.runs.$runParam.result.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { LoaderFunctionArgs } from "@remix-run/server-runtime";
22
import { json } from "@remix-run/server-runtime";
33
import { z } from "zod";
44
import { ApiRunResultPresenter } from "~/presenters/v3/ApiRunResultPresenter.server";
5+
import { $replica, runOpsNewReplica, runOpsSplitReadEnabled } from "~/db.server";
56
import { authenticateApiRequest } from "~/services/apiAuth.server";
67
import { logger } from "~/services/logger.server";
78

@@ -27,7 +28,11 @@ export async function loader({ request, params }: LoaderFunctionArgs) {
2728
const { runParam } = parsed.data;
2829

2930
try {
30-
const presenter = new ApiRunResultPresenter();
31+
const presenter = new ApiRunResultPresenter(undefined, undefined, {
32+
newClient: runOpsNewReplica,
33+
legacyReplica: $replica,
34+
splitEnabled: runOpsSplitReadEnabled,
35+
});
3136
const result = await presenter.call(runParam, authenticationResult.environment);
3237

3338
if (!result) {
Lines changed: 253 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,253 @@
1+
import { describe, expect, vi } from "vitest";
2+
3+
// Uses the REAL dedicated run-ops client (RunOpsPrismaClient / SUBSET schema) as the new-DB handle,
4+
// whose `Waitpoint` model has NO `connectedRuns` relation — so a relation-select of it throws rather
5+
// than missing. The existing suite can't catch that: it wires a full-schema PG17 as the "new" client.
6+
// NextRunListPresenter is stubbed to echo its `runId` set back as `runs`, so `result.connectedRuns` is
7+
// exactly the friendlyIds the presenter gathered cross-DB (isolating the gather from the CH hydrate).
8+
const legacyReplicaHolder = vi.hoisted(() => ({ client: undefined as any }));
9+
const newClientHolder = vi.hoisted(() => ({ client: undefined as any }));
10+
11+
vi.mock("~/db.server", async () => {
12+
const { Prisma } = await import("@trigger.dev/database");
13+
const lazyProxy = (holder: { client: any }, label: string) =>
14+
new Proxy(
15+
{},
16+
{
17+
get(_t, prop) {
18+
if (!holder.client) {
19+
throw new Error(`${label} not set for this test`);
20+
}
21+
return holder.client[prop];
22+
},
23+
}
24+
);
25+
const replicaProxy = lazyProxy(legacyReplicaHolder, "legacyReplicaHolder.client");
26+
return {
27+
prisma: replicaProxy,
28+
$replica: replicaProxy,
29+
runOpsNewPrisma: lazyProxy(newClientHolder, "newClientHolder.client"),
30+
runOpsNewReplica: lazyProxy(newClientHolder, "newClientHolder.client"),
31+
runOpsLegacyPrisma: replicaProxy,
32+
runOpsLegacyReplica: replicaProxy,
33+
sqlDatabaseSchema: Prisma.sql([`public`]),
34+
};
35+
});
36+
37+
vi.mock("~/services/clickhouse/clickhouseFactoryInstance.server", () => ({
38+
clickhouseFactory: {
39+
getClickhouseForOrganization: async () => ({}),
40+
},
41+
}));
42+
43+
// Echo the runId set back as runs so `result.connectedRuns` == the friendlyIds the presenter gathered.
44+
vi.mock("~/presenters/v3/NextRunListPresenter.server", () => ({
45+
NextRunListPresenter: class {
46+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
47+
constructor(..._args: unknown[]) {}
48+
async call(_organizationId: string, _environmentId: string, opts: { runId?: string[] }) {
49+
return {
50+
runs: (opts.runId ?? []).map((friendlyId) => ({
51+
friendlyId,
52+
taskIdentifier: "echoed",
53+
})),
54+
};
55+
}
56+
},
57+
}));
58+
59+
import { heteroRunOpsPostgresTest } from "@internal/testcontainers";
60+
import type { PrismaClient } from "@trigger.dev/database";
61+
import type { RunOpsPrismaClient } from "@internal/run-ops-database";
62+
import { WaitpointPresenter } from "~/presenters/v3/WaitpointPresenter.server";
63+
64+
vi.setConfig({ testTimeout: 90_000 });
65+
66+
type SeedContext = {
67+
organizationId: string;
68+
projectId: string;
69+
environmentId: string;
70+
};
71+
72+
// Parents (org/project/env) only exist on the full control-plane schema; the dedicated subset has no
73+
// such models, so we always seed them on the legacy (PG14) client and let the resolver read them there.
74+
async function seedParents(prisma: PrismaClient, slug: string): Promise<SeedContext> {
75+
const organization = await prisma.organization.create({
76+
data: { title: `org-${slug}`, slug: `org-${slug}` },
77+
});
78+
const project = await prisma.project.create({
79+
data: {
80+
name: `proj-${slug}`,
81+
slug: `proj-${slug}`,
82+
organizationId: organization.id,
83+
externalRef: `proj-${slug}`,
84+
},
85+
});
86+
const runtimeEnvironment = await prisma.runtimeEnvironment.create({
87+
data: {
88+
slug: `env-${slug}`,
89+
type: "DEVELOPMENT",
90+
projectId: project.id,
91+
organizationId: organization.id,
92+
apiKey: `tr_dev_${slug}`,
93+
pkApiKey: `pk_dev_${slug}`,
94+
shortcode: `sc-${slug}`,
95+
},
96+
});
97+
return {
98+
organizationId: organization.id,
99+
projectId: project.id,
100+
environmentId: runtimeEnvironment.id,
101+
};
102+
}
103+
104+
async function seedWaitpoint(
105+
prisma: PrismaClient | RunOpsPrismaClient,
106+
ctx: SeedContext,
107+
friendlyId: string
108+
) {
109+
return (prisma as PrismaClient).waitpoint.create({
110+
data: {
111+
friendlyId,
112+
type: "MANUAL",
113+
status: "COMPLETED",
114+
idempotencyKey: `idem-${friendlyId}`,
115+
userProvidedIdempotencyKey: false,
116+
output: JSON.stringify({ hello: "world" }),
117+
outputType: "application/json",
118+
outputIsError: false,
119+
completedAt: new Date(),
120+
tags: ["a", "b"],
121+
projectId: ctx.projectId,
122+
environmentId: ctx.environmentId,
123+
},
124+
});
125+
}
126+
127+
async function seedRun(
128+
prisma: PrismaClient | RunOpsPrismaClient,
129+
ctx: SeedContext,
130+
friendlyId: string
131+
) {
132+
return (prisma as PrismaClient).taskRun.create({
133+
data: {
134+
friendlyId,
135+
taskIdentifier: "my-task",
136+
status: "PENDING",
137+
payload: JSON.stringify({ foo: friendlyId }),
138+
payloadType: "application/json",
139+
traceId: friendlyId,
140+
spanId: friendlyId,
141+
queue: "test",
142+
runtimeEnvironmentId: ctx.environmentId,
143+
projectId: ctx.projectId,
144+
organizationId: ctx.organizationId,
145+
environmentType: "DEVELOPMENT",
146+
engine: "V2",
147+
},
148+
});
149+
}
150+
151+
const callArgs = (ctx: SeedContext, friendlyId: string) => ({
152+
friendlyId,
153+
environmentId: ctx.environmentId,
154+
projectId: ctx.projectId,
155+
});
156+
157+
describe("WaitpointPresenter against the REAL dedicated run-ops client", () => {
158+
// A NEW-resident waitpoint (on the dedicated subset schema) with no connected runs. The current
159+
// relation-select of `connectedRuns` is invalid on the dedicated Waitpoint model, so the read
160+
// throws PrismaClientValidationError. Desired: resolves the waitpoint, connectedRuns empty.
161+
heteroRunOpsPostgresTest(
162+
"resolves a new-resident waitpoint without a connectedRuns relation-select (no throw)",
163+
async ({ prisma14, prisma17 }) => {
164+
const ctx = await seedParents(prisma14, "dedself");
165+
const seeded = await seedWaitpoint(prisma17, ctx, "waitpoint_dedself");
166+
167+
legacyReplicaHolder.client = prisma14;
168+
newClientHolder.client = prisma17;
169+
170+
const presenter = new WaitpointPresenter(undefined, undefined, {
171+
splitEnabled: true,
172+
newClient: prisma17 as unknown as PrismaClient,
173+
legacyReplica: prisma14,
174+
});
175+
176+
const result = await presenter.call(callArgs(ctx, seeded.friendlyId));
177+
178+
expect(result?.id).toBe(seeded.friendlyId);
179+
expect(result?.connectedRuns).toEqual([]);
180+
}
181+
);
182+
183+
// Cross-DB connection: waitpoint on LEGACY (PG14), the connected run + its WaitpointRunConnection
184+
// join on the NEW dedicated DB (PG17). A single-DB gather off the waitpoint's own store misses the
185+
// run entirely; the fix reads the join from BOTH stores (dedicated `waitpointRunConnection`
186+
// delegate + legacy raw `_WaitpointRunConnections`) and unions the friendlyIds.
187+
heteroRunOpsPostgresTest(
188+
"gathers a cross-DB connected run whose join lives on the other database",
189+
async ({ prisma14, prisma17 }) => {
190+
const ctx = await seedParents(prisma14, "crossdb");
191+
const waitpoint = await seedWaitpoint(prisma14, ctx, "waitpoint_crossdb");
192+
193+
// The connected run + join live only on the NEW dedicated DB (co-resident with the run).
194+
const run = await seedRun(prisma17, ctx, "run_crossnew");
195+
await prisma17.waitpointRunConnection.create({
196+
data: { taskRunId: run.id, waitpointId: waitpoint.id },
197+
});
198+
199+
legacyReplicaHolder.client = prisma14;
200+
newClientHolder.client = prisma17;
201+
202+
const presenter = new WaitpointPresenter(undefined, undefined, {
203+
splitEnabled: true,
204+
newClient: prisma17 as unknown as PrismaClient,
205+
legacyReplica: prisma14,
206+
});
207+
208+
const result = await presenter.call(callArgs(ctx, waitpoint.friendlyId));
209+
210+
expect(result?.id).toBe(waitpoint.friendlyId);
211+
expect(result?.connectedRuns.map((r) => r.friendlyId)).toEqual(["run_crossnew"]);
212+
}
213+
);
214+
215+
// Same-DB legacy connection (no regression): waitpoint + connected run both on LEGACY, joined via
216+
// the implicit `_WaitpointRunConnections` M2M. The gather must read the legacy raw join path.
217+
heteroRunOpsPostgresTest(
218+
"still gathers a same-DB legacy connected run via the implicit M2M",
219+
async ({ prisma14, prisma17 }) => {
220+
const ctx = await seedParents(prisma14, "legsame");
221+
const run = await seedRun(prisma14, ctx, "run_legsame");
222+
const waitpoint = await prisma14.waitpoint.create({
223+
data: {
224+
friendlyId: "waitpoint_legsame",
225+
type: "MANUAL",
226+
status: "COMPLETED",
227+
idempotencyKey: "idem-waitpoint_legsame",
228+
userProvidedIdempotencyKey: false,
229+
outputType: "application/json",
230+
outputIsError: false,
231+
completedAt: new Date(),
232+
tags: [],
233+
projectId: ctx.projectId,
234+
environmentId: ctx.environmentId,
235+
connectedRuns: { connect: [{ id: run.id }] },
236+
},
237+
});
238+
239+
legacyReplicaHolder.client = prisma14;
240+
newClientHolder.client = prisma17;
241+
242+
const presenter = new WaitpointPresenter(undefined, undefined, {
243+
splitEnabled: true,
244+
newClient: prisma17 as unknown as PrismaClient,
245+
legacyReplica: prisma14,
246+
});
247+
248+
const result = await presenter.call(callArgs(ctx, waitpoint.friendlyId));
249+
250+
expect(result?.connectedRuns.map((r) => r.friendlyId)).toEqual(["run_legsame"]);
251+
}
252+
);
253+
});
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
-- Run-ops split: allow a cross-DB token connection (a LEGACY run blocking on a NEW-resident token,
2+
-- whose Waitpoint row lives on the other database). Matches the split's control-plane FK-removal pattern.
3+
4+
-- Fail fast instead of queueing behind a long txn/VACUUM for the ACCESS EXCLUSIVE lock.
5+
SET lock_timeout = '5s';
6+
7+
ALTER TABLE "_WaitpointRunConnections" DROP CONSTRAINT IF EXISTS "_WaitpointRunConnections_B_fkey";

0 commit comments

Comments
 (0)