Skip to content

Commit dfa3a32

Browse files
hastyyTrigger.dev RepoOps
authored andcommitted
fix(run-engine,webapp,supervisor,core): resume runs through database blips
Runs resuming from a checkpoint could be aborted during a brief database outage, even though they would have resumed once the database recovered. A retried resume whose first request had already gone through now gets the same result instead of failing the run. The continue route can also answer temporary database errors with 503 and a busy run lock with 409, and the supervisor can keep retrying those for a time budget instead of a fixed number of attempts. Those two are opt-in for now: `RUN_ENGINE_CONTINUE_RETRYABLE_ERRORS_ENABLED=true` on the webapp and `TRIGGER_CONTINUE_RETRY_BUDGET_MS` on the supervisor. Mono-RevId: d40c723db309ebfb84a9c60d626ff7f90bb8d7a7
1 parent 0f16de5 commit dfa3a32

18 files changed

Lines changed: 1138 additions & 45 deletions
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Fixed runs failing to resume from a wait when the platform had already resumed them but the confirmation was lost.

‎apps/supervisor/src/env.test.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,3 +202,25 @@ describe("worker cell name", () => {
202202
expect(() => Env.parse({ ...base, TRIGGER_WORKER_CELL_NAME: "bad/name" })).toThrow();
203203
});
204204
});
205+
206+
describe("continue retry budget", () => {
207+
it("is off when unset", () => {
208+
expect(Env.parse(base).TRIGGER_CONTINUE_RETRY_BUDGET_MS).toBeUndefined();
209+
});
210+
211+
it("accepts a budget within the bounds", () => {
212+
for (const value of ["120000", "270000", "290000"]) {
213+
expect(
214+
Env.parse({ ...base, TRIGGER_CONTINUE_RETRY_BUDGET_MS: value })
215+
.TRIGGER_CONTINUE_RETRY_BUDGET_MS
216+
).toBe(Number(value));
217+
}
218+
});
219+
220+
it.each(["", "0", "-1", "60000", "119999", "290001", "not-a-number"])(
221+
"refuses %j, so a bad value stops the supervisor from starting",
222+
(value) => {
223+
expect(() => Env.parse({ ...base, TRIGGER_CONTINUE_RETRY_BUDGET_MS: value })).toThrow();
224+
}
225+
);
226+
});

‎apps/supervisor/src/env.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,15 @@ export const Env = z
5757
TRIGGER_WORKLOAD_API_HOST_INTERNAL: z.string().default("0.0.0.0"),
5858
TRIGGER_WORKLOAD_API_PORT_INTERNAL: z.coerce.number().default(8020), // This is the port the workload API listens on
5959
TRIGGER_WORKLOAD_API_PORT_EXTERNAL: z.coerce.number().default(8020), // This is the exposed port passed to the run controller
60+
// How long to keep retrying temporary failures of `continue` for a run restored from a
61+
// checkpoint; unset keeps the default retries.
62+
TRIGGER_CONTINUE_RETRY_BUDGET_MS: z.coerce
63+
.number()
64+
.int()
65+
.min(120_000)
66+
// The runner waits at most 300s for this response: Node's default `fetch` header timeout.
67+
.max(290_000)
68+
.optional(),
6069

6170
// Runner settings
6271
RUNNER_HEARTBEAT_INTERVAL_SECONDS: z.coerce.number().optional(),

‎apps/supervisor/src/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -604,6 +604,7 @@ class ManagedSupervisor {
604604
runnerSnapshotter: this.runCrdManager,
605605
wideEventOpts: this.wideEventOpts,
606606
wideEventsNoisyRoutes: this.wideEventsNoisyRoutes,
607+
continueRetryBudgetMs: env.TRIGGER_CONTINUE_RETRY_BUDGET_MS,
607608
});
608609

609610
this.workloadServer.on("runConnected", this.onRunConnected.bind(this));

‎apps/supervisor/src/workloadServer/index.ts‎

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,13 @@ const checkpointDeleteRequests = new Counter({
5656
registers: [register],
5757
});
5858

59+
const continueRetryOutcomes = new Counter({
60+
name: "supervisor_continue_retry_outcomes_total",
61+
help: "Count of continue calls retried within a budget, by how the retries ended (succeeded, non_retryable, server_errors, budget_exhausted).",
62+
labelNames: ["outcome"],
63+
registers: [register],
64+
});
65+
5966
const checkpointCancelRequests = new Counter({
6067
name: "checkpoint_cancel_requests_total",
6168
help: "Checkpoint cancel requests attempted when a run continues, by outcome",
@@ -115,6 +122,8 @@ type WorkloadServerOptions = {
115122
wideEventsNoisyRoutes: boolean;
116123
/** Where runs started through this server execute. Overrides anything the runner sends. */
117124
workerStamp?: { workerCell?: string; workerLane?: string };
125+
/** How long to keep retrying temporary failures of `continue`. Unset keeps the default retries. */
126+
continueRetryBudgetMs?: number;
118127
};
119128

120129
export class WorkloadServer extends EventEmitter<WorkloadServerEvents> {
@@ -145,6 +154,7 @@ export class WorkloadServer extends EventEmitter<WorkloadServerEvents> {
145154

146155
private readonly workerClient: SupervisorHttpClient;
147156
private readonly workerStamp: { workerCell?: string; workerLane?: string };
157+
private readonly continueRetryBudget?: { budgetMs: number };
148158

149159
constructor(opts: WorkloadServerOptions) {
150160
super();
@@ -158,6 +168,14 @@ export class WorkloadServer extends EventEmitter<WorkloadServerEvents> {
158168
this.wideEventOpts = opts.wideEventOpts;
159169
this.wideEventsNoisyRoutes = opts.wideEventsNoisyRoutes;
160170

171+
this.continueRetryBudget =
172+
opts.continueRetryBudgetMs === undefined
173+
? undefined
174+
: { budgetMs: opts.continueRetryBudgetMs };
175+
this.logger.info("Continue retries", {
176+
budgetMs: opts.continueRetryBudgetMs ?? "unset, default retries",
177+
});
178+
161179
if (opts.runnerSnapshotter?.snapshotsEnabled) {
162180
this.snapshotService = new ComputeSnapshotService({
163181
runnerSnapshotter: opts.runnerSnapshotter,
@@ -661,11 +679,24 @@ export class WorkloadServer extends EventEmitter<WorkloadServerEvents> {
661679
params.runFriendlyId,
662680
params.snapshotFriendlyId,
663681
this.runnerIdFromRequest(req),
664-
auth.environmentId
682+
auth.environmentId,
683+
{ retryBudget: this.continueRetryBudget }
665684
);
666685

686+
if (continuationResult.retry) {
687+
continueRetryOutcomes.inc({ outcome: continuationResult.retry.outcome });
688+
}
689+
667690
if (!continuationResult.success) {
668-
this.logger.error("Failed to continue run execution", { params });
691+
this.logger.error("Failed to continue run execution", {
692+
params,
693+
statusCode: continuationResult.statusCode,
694+
error: continuationResult.error,
695+
attempts: continuationResult.retry?.attempts,
696+
elapsedMs: continuationResult.retry?.elapsedMs,
697+
outcome: continuationResult.retry?.outcome,
698+
budgetMs: this.continueRetryBudget?.budgetMs,
699+
});
669700
reply.json(
670701
{
671702
ok: false,

‎apps/webapp/app/env.server.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1562,6 +1562,9 @@ const EnvironmentSchema = z
15621562
RUN_ENGINE_QUEUE_RECONCILE_LOCK_TTL_SECONDS: z.coerce.number().int().positive().default(10),
15631563
RUN_ENGINE_QUEUE_RECONCILE_MAX_PASSES_PER_DEQUEUE: z.coerce.number().int().min(0).default(2),
15641564
RUN_ENGINE_TREAT_PRODUCTION_EXECUTION_STALLS_AS_OOM: z.string().default("0"),
1565+
// When on, the worker continue route answers temporary database errors with 503 and a busy run
1566+
// lock with 409, which the supervisor keeps retrying, instead of a 422 that aborts the run.
1567+
RUN_ENGINE_CONTINUE_RETRYABLE_ERRORS_ENABLED: BoolEnv.default(false),
15651568
RUN_ENGINE_READ_REPLICA_SNAPSHOTS_SINCE_ENABLED: z.string().default("0"),
15661569
RUN_ENGINE_SNAPSHOTS_SINCE_REPLICA_RETRY_MIN_MS: z.coerce.number().int().default(50),
15671570
RUN_ENGINE_SNAPSHOTS_SINCE_REPLICA_RETRY_MAX_MS: z.coerce.number().int().default(200),
Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
import { startTestServer, type TestServer } from "@internal/testcontainers/webapp";
2+
import { ApiClient } from "@trigger.dev/core/v3";
3+
import { SupervisorHttpClient, WORKER_HEADERS } from "@trigger.dev/core/v3/runEngineWorker";
4+
import { setTimeout } from "node:timers/promises";
5+
import { Client } from "pg";
6+
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
7+
import {
8+
MANAGED_WORKER_SECRET,
9+
seedEngineFixtures,
10+
workerHeaders,
11+
type EngineFixtures,
12+
} from "../../test/bench/lib/engineFixtures";
13+
14+
const INSTANCE_NAME = "continue-e2e";
15+
const RUNNER_ID = "runner-continue-e2e";
16+
17+
let server: TestServer;
18+
let fixtures: EngineFixtures;
19+
let api: ApiClient;
20+
let supervisor: SupervisorHttpClient;
21+
22+
beforeAll(async () => {
23+
server = await startTestServer({
24+
extraEnv: { RUN_ENGINE_CONTINUE_RETRYABLE_ERRORS_ENABLED: "true" },
25+
// The engine worker is what requeues a run once its waitpoint completes.
26+
overrideEnv: { RUN_ENGINE_WORKER_ENABLED: "1" },
27+
});
28+
fixtures = await seedEngineFixtures(server.prisma, { taskCount: 1 });
29+
api = new ApiClient(server.webapp.baseUrl, fixtures.environmentApiKey);
30+
supervisor = new SupervisorHttpClient({
31+
apiUrl: server.webapp.baseUrl,
32+
workerToken: fixtures.workerGroupToken,
33+
instanceName: INSTANCE_NAME,
34+
managedWorkerSecret: MANAGED_WORKER_SECRET,
35+
});
36+
}, 180_000);
37+
38+
afterAll(async () => {
39+
await server?.stop();
40+
}, 120_000);
41+
42+
describe("continue after a restore from a checkpoint", () => {
43+
it("answers a retry of a continue that already succeeded with the same result", async () => {
44+
const { runId, snapshotId } = await restoreFromCheckpoint();
45+
46+
const first = await continueRun(runId, snapshotId);
47+
const retry = await continueRun(runId, snapshotId);
48+
49+
expect(first.success).toBe(true);
50+
expect(retry).toEqual(first);
51+
}, 60_000);
52+
53+
it("keeps retrying through a database outage and succeeds once it ends", async () => {
54+
const { runId, snapshotId } = await restoreFromCheckpoint();
55+
56+
const outage = await startDatabaseOutage();
57+
let continued: ReturnType<typeof continueRun>;
58+
try {
59+
// A single request, because the client retries. It gets past worker auth because that is
60+
// served from the in-process cache the earlier calls warmed; a cold cache would answer 401.
61+
const duringOutage = await server.webapp.fetch(
62+
`/engine/v1/worker-actions/runs/${runId}/snapshots/${snapshotId}/continue`,
63+
{
64+
headers: {
65+
...workerHeaders(fixtures, INSTANCE_NAME),
66+
[WORKER_HEADERS.ENVIRONMENT_ID]: fixtures.environmentId,
67+
},
68+
}
69+
);
70+
expect(duringOutage.status).toBe(503);
71+
72+
continued = continueRun(runId, snapshotId, { budgetMs: 30_000 });
73+
await setTimeout(3_000);
74+
} finally {
75+
await outage.end();
76+
}
77+
78+
const { success, retry } = await continued;
79+
expect(success).toBe(true);
80+
expect(retry?.outcome).toBe("succeeded");
81+
expect(retry?.attempts).toBeGreaterThan(1);
82+
}, 60_000);
83+
});
84+
85+
function continueRun(runId: string, snapshotId: string, retryBudget?: { budgetMs: number }) {
86+
return supervisor.continueRunExecution(runId, snapshotId, RUNNER_ID, fixtures.environmentId, {
87+
retryBudget,
88+
});
89+
}
90+
91+
/**
92+
* Takes a run to the point where its runner, restored from a checkpoint, calls `continue`: the
93+
* run starts, blocks on a waitpoint and is checkpointed, then the waitpoint completes and the run
94+
* is dequeued again. Returns the snapshot `continue` is called with.
95+
*/
96+
async function restoreFromCheckpoint() {
97+
const { id: runId } = await api.triggerTask(fixtures.taskIdentifiers[0]!, { payload: {} });
98+
const dequeued = await dequeue();
99+
ok(await supervisor.startRunAttempt(runId, dequeued.snapshot.friendlyId, {}));
100+
101+
const token = await api.createWaitpointToken({});
102+
await api.waitForWaitpointToken({ runFriendlyId: runId, waitpointFriendlyId: token.id });
103+
104+
const { execution } = ok(await supervisor.getLatestSnapshot(runId));
105+
ok(
106+
await supervisor.submitSuspendCompletion({
107+
runId,
108+
snapshotId: execution.snapshot.friendlyId,
109+
body: { success: true, checkpoint: { type: "DOCKER", location: "e2e-checkpoint" } },
110+
})
111+
);
112+
113+
await api.completeWaitpointToken(token.id, {});
114+
const restored = await dequeue();
115+
expect(restored.snapshot.executionStatus).toBe("PENDING_EXECUTING");
116+
117+
return { runId, snapshotId: restored.snapshot.friendlyId };
118+
}
119+
120+
function dequeue() {
121+
return vi.waitFor(
122+
async () => {
123+
const [message] = ok(await supervisor.dequeue({}));
124+
if (!message) throw new Error("Nothing dequeued yet");
125+
return message;
126+
},
127+
{ timeout: 10_000, interval: 100 }
128+
);
129+
}
130+
131+
/** Closes the webapp's database to new connections and drops the open ones. */
132+
async function startDatabaseOutage() {
133+
const url = new URL(server.databaseUrl);
134+
const database = url.pathname.slice(1);
135+
url.pathname = "/postgres";
136+
137+
const admin = new Client({ connectionString: url.toString() });
138+
await admin.connect();
139+
await admin.query(`ALTER DATABASE "${database}" WITH ALLOW_CONNECTIONS false`);
140+
await admin.query(`SELECT pg_terminate_backend(pid) FROM pg_stat_activity WHERE datname = $1`, [
141+
database,
142+
]);
143+
144+
return {
145+
async end() {
146+
await admin.query(`ALTER DATABASE "${database}" WITH ALLOW_CONNECTIONS true`);
147+
await admin.end();
148+
},
149+
};
150+
}
151+
152+
function ok<T>(result: { success: true; data: T } | { success: false; error: string }): T {
153+
if (!result.success) throw new Error(result.error);
154+
return result.data;
155+
}

‎apps/webapp/app/routes/engine.v1.worker-actions.runs.$runFriendlyId.snapshots.$snapshotFriendlyId.continue.ts‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@ import type { TypedResponse } from "@remix-run/server-runtime";
22
import { json } from "@remix-run/server-runtime";
33
import type { WorkerApiContinueRunExecutionRequestBody } from "@trigger.dev/core/v3/workers";
44
import { z } from "zod";
5+
import { env } from "~/env.server";
56
import { logger } from "~/services/logger.server";
67
import { createLoaderWorkerApiRoute } from "~/services/routeBuilders/apiBuilder.server";
7-
import { clientSafeErrorMessage, isInfrastructureError } from "~/utils/prismaErrors";
8+
import { continueRunErrorResponse } from "~/utils/continueRunErrors.server";
89

910
export const loader = createLoaderWorkerApiRoute(
1011
{
@@ -46,20 +47,18 @@ export const loader = createLoaderWorkerApiRoute(
4647
environmentId,
4748
});
4849

49-
// A Prisma infrastructure error (e.g. P1001 "Can't reach database
50-
// server") means the DB was transiently unreachable while resuming. A 422
51-
// is non-retryable, so the worker would permanently abort a run over a
52-
// blip. Let it propagate to the generic 500 handler, which scrubs the
53-
// message and is retried by the worker's HTTP client.
54-
if (isInfrastructureError(error)) {
55-
throw error;
56-
}
50+
// A 422 is non-retryable, so the worker would permanently abort a run over a
51+
// database blip. Temporary errors get a status the worker retries instead.
52+
const response = continueRunErrorResponse(error, {
53+
retryableErrors: env.RUN_ENGINE_CONTINUE_RETRYABLE_ERRORS_ENABLED,
54+
});
5755

58-
if (error instanceof Error) {
59-
throw json({ error: clientSafeErrorMessage(error) }, { status: 422 });
56+
// No response: the generic 500 handler takes it and scrubs the message.
57+
if (!response) {
58+
throw error;
6059
}
6160

62-
throw json({ error: "Failed to continue run execution" }, { status: 422 });
61+
throw response;
6362
}
6463
}
6564
);
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { json } from "@remix-run/server-runtime";
2+
import { LockAcquisitionTimeoutError } from "@internal/run-engine";
3+
import { isPrismaRetriableError } from "@trigger.dev/database";
4+
import { clientSafeErrorMessage, isInfrastructureError } from "./prismaErrors";
5+
6+
/**
7+
* The response the worker `continue` route sends for an error thrown while continuing a run, or
8+
* undefined to let the error reach the generic 500 handler, which scrubs the message.
9+
*
10+
* With `retryableErrors` on, a temporary database error (unreachable, pool or transaction-start
11+
* timeout, write conflict) gets 503 and a busy run lock gets 409. The supervisor keeps retrying both
12+
* for its full budget, so a run restored during a brief database outage resumes instead of being
13+
* aborted. A plain 500 then only comes from outside this handler (authentication, parameter
14+
* parsing), and the supervisor retries it briefly; an unexpected error inside it is a 422, as
15+
* before.
16+
*
17+
* With it off, the responses are unchanged: infrastructure errors 500, everything else 422.
18+
*/
19+
export function continueRunErrorResponse(
20+
error: unknown,
21+
{ retryableErrors }: { retryableErrors: boolean }
22+
): Response | undefined {
23+
if (retryableErrors) {
24+
// This sends pool exhaustion (P2024) and a Rust-engine panic to 503, although
25+
// `isRetryableInfrastructureError` in the database package never retries either. That gate
26+
// decides an immediate retry in the same process, where another attempt competes for the same
27+
// exhausted pool or the same broken engine. Here the alternative is aborting the run, and the
28+
// retry is a separate request from the supervisor, seconds apart with jitter, bounded by its
29+
// budget, and possibly served by another instance.
30+
if (isInfrastructureError(error) || isPrismaRetriableError(error)) {
31+
return json({ error: "Temporarily unable to continue run execution" }, { status: 503 });
32+
}
33+
34+
if (error instanceof LockAcquisitionTimeoutError) {
35+
return json({ error: "Run is busy, try again" }, { status: 409 });
36+
}
37+
} else if (isInfrastructureError(error)) {
38+
return undefined;
39+
}
40+
41+
return json(
42+
{
43+
error:
44+
error instanceof Error ? clientSafeErrorMessage(error) : "Failed to continue run execution",
45+
},
46+
{ status: 422 }
47+
);
48+
}

0 commit comments

Comments
 (0)