Skip to content

Commit e33ace2

Browse files
1stvampTrigger.dev RepoOps
authored andcommitted
fix(run-engine,webapp): check for an attempt already executing when one starts
The run engine can detect a second start of an attempt that is already executing (`RUN_ENGINE_DUPLICATE_ATTEMPT_START_CHECK`, logged by default). Mono-RevId: 39d7ef47a458cd9ade110fe244ccca37a85e520e
1 parent a2fe62c commit e33ace2

8 files changed

Lines changed: 457 additions & 2 deletions

File tree

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,6 +1057,8 @@ const EnvironmentSchema = z
10571057
// cutoff pass (grandfathered). Default off = no run-row read, byte-for-byte today's behavior.
10581058
WORKLOAD_CREATED_AT_GATE_ENABLED: z.string().default("0"),
10591059
WORKLOAD_TOKEN_CUTOFF: z.string().datetime().optional(),
1060+
// Whether an attempt start for a run that already has an attempt executing is logged or refused.
1061+
RUN_ENGINE_DUPLICATE_ATTEMPT_START_CHECK: z.enum(["off", "log", "enforce"]).default("log"),
10601062

10611063
CLI_LOGIN_NETWORK_CHECK: z.enum(["off", "log", "confirm"]).default("log"),
10621064
CLI_LOGIN_LEGACY_UNTIL: z.string().datetime().default("2027-01-01T00:00:00.000Z"),
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import { startTestServer, type TestServer } from "@internal/testcontainers/webapp";
2+
import type { WorkerQueueSubscription } from "@trigger.dev/core/v3/workers";
3+
import { afterAll, beforeAll, describe, expect, it } from "vitest";
4+
import {
5+
seedEngineFixtures,
6+
workerHeaders,
7+
type EngineFixtures,
8+
} from "../../test/bench/lib/engineFixtures";
9+
10+
const LEGACY_MASTER_QUEUE = "my workers";
11+
const subscription: WorkerQueueSubscription = {
12+
class: "ondemand",
13+
phase: "fresh",
14+
compat: "container",
15+
channel: "stable",
16+
};
17+
18+
type Ctx = { server: TestServer; fixtures: EngineFixtures };
19+
20+
async function startServer(workerGroupId: string, check: string): Promise<Ctx> {
21+
const server = await startTestServer({
22+
extraEnv: {
23+
RUN_ENGINE_WORKER_QUEUE_SUBSCRIPTIONS: JSON.stringify({ [workerGroupId]: [subscription] }),
24+
RUN_ENGINE_DUPLICATE_ATTEMPT_START_CHECK: check,
25+
},
26+
overrideEnv: { RUN_ENGINE_WORKER_ENABLED: "1" },
27+
});
28+
const fixtures = await seedEngineFixtures(server.prisma, {
29+
taskCount: 1,
30+
workerGroupId,
31+
masterQueue: LEGACY_MASTER_QUEUE,
32+
enableFastPath: true,
33+
});
34+
return { server, fixtures };
35+
}
36+
37+
function workerCall(ctx: Ctx, path: string, body: unknown = {}) {
38+
return ctx.server.webapp.fetch(path, {
39+
method: "POST",
40+
headers: {
41+
...(workerHeaders(ctx.fixtures, "supervisor-1") as Record<string, string>),
42+
"x-trigger-worker-runner-id": "runner-1",
43+
},
44+
body: JSON.stringify(body),
45+
});
46+
}
47+
48+
async function triggerAndDequeue(ctx: Ctx) {
49+
const res = await ctx.server.webapp.fetch(
50+
`/api/v1/tasks/${ctx.fixtures.taskIdentifiers[0]!}/trigger`,
51+
{
52+
method: "POST",
53+
headers: {
54+
Authorization: `Bearer ${ctx.fixtures.environmentApiKey}`,
55+
"content-type": "application/json",
56+
},
57+
body: JSON.stringify({ payload: {} }),
58+
}
59+
);
60+
expect(res.ok).toBe(true);
61+
62+
for (let i = 0; i < 40; i++) {
63+
const dequeued = await workerCall(ctx, "/engine/v1/worker-actions/dequeue");
64+
const messages = (await dequeued.json()) as any[];
65+
if (messages.length > 0) {
66+
return {
67+
run: messages[0].run.friendlyId as string,
68+
snapshot: messages[0].snapshot.friendlyId as string,
69+
};
70+
}
71+
await new Promise((r) => setTimeout(r, 500));
72+
}
73+
throw new Error("dequeue returned nothing");
74+
}
75+
76+
const runPath = (run: string, rest: string) => `/engine/v1/worker-actions/runs/${run}/${rest}`;
77+
78+
function start(ctx: Ctx, run: string, snapshot: string) {
79+
return workerCall(ctx, runPath(run, `snapshots/${snapshot}/attempts/start`));
80+
}
81+
82+
describe("attempt start on a run already executing, enforced", () => {
83+
let ctx: Ctx;
84+
85+
beforeAll(async () => {
86+
ctx = await startServer("duplicate-start-enforce", "enforce");
87+
}, 300_000);
88+
89+
afterAll(async () => {
90+
await ctx?.server.stop();
91+
}, 120_000);
92+
93+
it("refuses a second start while the first attempt is executing", async () => {
94+
const { run, snapshot } = await triggerAndDequeue(ctx);
95+
const first = (await (await start(ctx, run, snapshot)).json()) as any;
96+
expect(first.snapshot.executionStatus).toBe("EXECUTING");
97+
98+
const second = await start(ctx, run, first.snapshot.friendlyId);
99+
expect(second.status).toBe(409);
100+
const body = (await second.json()) as Record<string, unknown>;
101+
expect(Object.keys(body)).toEqual(["error"]);
102+
}, 120_000);
103+
104+
it("starts the next attempt after an immediate retry", async () => {
105+
const { run, snapshot } = await triggerAndDequeue(ctx);
106+
// The fixture task allows one attempt.
107+
await ctx.server.prisma.taskRun.update({
108+
where: { friendlyId: run },
109+
data: { maxAttempts: 3 },
110+
});
111+
const first = (await (await start(ctx, run, snapshot)).json()) as any;
112+
113+
const completed = await workerCall(
114+
ctx,
115+
runPath(run, `snapshots/${first.snapshot.friendlyId}/attempts/complete`),
116+
{
117+
completion: {
118+
ok: false,
119+
id: run,
120+
error: { type: "BUILT_IN_ERROR", name: "Error", message: "boom", stackTrace: "" },
121+
retry: { timestamp: Date.now(), delay: 0 },
122+
},
123+
}
124+
);
125+
expect(completed.status).toBe(200);
126+
const result = ((await completed.json()) as any).result;
127+
expect(result.attemptStatus).toBe("RETRY_IMMEDIATELY");
128+
129+
const retried = await start(ctx, run, result.snapshot.friendlyId);
130+
expect(retried.status).toBe(200);
131+
expect(((await retried.json()) as any).run.attemptNumber).toBe(2);
132+
}, 120_000);
133+
});
134+
135+
describe("attempt start on a run already executing, logged", () => {
136+
let ctx: Ctx;
137+
138+
beforeAll(async () => {
139+
ctx = await startServer("duplicate-start-log", "log");
140+
}, 300_000);
141+
142+
afterAll(async () => {
143+
await ctx?.server.stop();
144+
}, 120_000);
145+
146+
it("allows a second start, as before", async () => {
147+
const { run, snapshot } = await triggerAndDequeue(ctx);
148+
const first = (await (await start(ctx, run, snapshot)).json()) as any;
149+
150+
const second = await start(ctx, run, first.snapshot.friendlyId);
151+
expect(second.status).toBe(200);
152+
expect(((await second.json()) as any).run.attemptNumber).toBe(2);
153+
}, 120_000);
154+
});

‎apps/webapp/app/v3/runEngine.server.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,7 @@ function createRunEngine() {
184184
factor: env.RUN_ENGINE_SUSPENDED_HEARTBEAT_RETRIES_FACTOR,
185185
},
186186
retryWarmStartThresholdMs: env.RUN_ENGINE_RETRY_WARM_START_THRESHOLD_MS,
187+
duplicateAttemptStartCheck: env.RUN_ENGINE_DUPLICATE_ATTEMPT_START_CHECK,
187188
pendingVersionRunIdLookup: runEnginePendingVersionLookup,
188189
externalDeploymentParkDeadlineMs: env.EXTERNAL_DEPLOYMENT_PARK_DEADLINE_MS,
189190
billing: {

‎internal-packages/run-engine/src/engine/consts.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,10 @@ export const MAX_TASK_RUN_ATTEMPTS = 250;
99
*/
1010
export const QUEUED_SNAPSHOT_STATUS = "QUEUED" as const;
1111
export const QUEUED_SNAPSHOT_DESCRIPTION = "Run was QUEUED";
12+
13+
/**
14+
* Written by `attemptFailed` when a failed attempt is retried in the same process. It is the only
15+
* EXECUTING snapshot a runner starts a new attempt from, so `startRunAttempt` matches on it.
16+
*/
17+
export const RETRY_IMMEDIATELY_SNAPSHOT_DESCRIPTION =
18+
"Attempt failed with a short delay, starting a new attempt";

‎internal-packages/run-engine/src/engine/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,7 @@ export class RunEngine {
581581
machines: this.options.machines,
582582
retryWarmStartThresholdMs: this.options.retryWarmStartThresholdMs,
583583
finalizationGuardDelayMs: this.options.finalizationGuardDelayMs,
584+
duplicateAttemptStartCheck: this.options.duplicateAttemptStartCheck,
584585
redisOptions: this.options.cache?.redis ?? this.options.runLock.redis,
585586
});
586587

‎internal-packages/run-engine/src/engine/systems/runAttemptSystem.ts‎

Lines changed: 53 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import type {
3838
PrismaClientOrTransaction,
3939
RuntimeEnvironmentType,
4040
} from "@trigger.dev/database";
41-
import { MAX_TASK_RUN_ATTEMPTS } from "../consts.js";
41+
import { MAX_TASK_RUN_ATTEMPTS, RETRY_IMMEDIATELY_SNAPSHOT_DESCRIPTION } from "../consts.js";
4242
import { runStatusFromError, ServiceValidationError } from "../errors.js";
4343
import { sendNotificationToWorker } from "../eventBus.js";
4444
import { getMachinePreset, machinePresetFromName } from "../machinePresets.js";
@@ -88,6 +88,7 @@ export type RunAttemptSystemOptions = {
8888
delayedRunSystem: DelayedRunSystem;
8989
retryWarmStartThresholdMs?: number;
9090
finalizationGuardDelayMs?: number;
91+
duplicateAttemptStartCheck?: RunEngineOptions["duplicateAttemptStartCheck"];
9192
machines: RunEngineOptions["machines"];
9293
redisOptions: RedisOptions;
9394
};
@@ -142,6 +143,7 @@ export class RunAttemptSystem {
142143
private readonly rederivationsCounter: Counter;
143144
private readonly releasesCounter: Counter;
144145
private readonly restoreOutcomesCounter: Counter;
146+
private readonly duplicateAttemptStartsCounter: Counter;
145147
private readonly cache: UnkeyCache<{
146148
tasks: BackwardsCompatibleTaskRunExecution["task"];
147149
machinePresets: MachinePreset;
@@ -175,6 +177,13 @@ export class RunAttemptSystem {
175177
description: "Failed restores reported by a supervisor, by outcome, reason and result",
176178
unit: "reports",
177179
});
180+
this.duplicateAttemptStartsCounter = this.$.meter.createCounter(
181+
"run_engine.duplicate_attempt_starts",
182+
{
183+
description: "Attempt starts for a run that already has an attempt executing, by outcome",
184+
unit: "requests",
185+
}
186+
);
178187

179188
const ctx = new DefaultStatefulContext();
180189
const memory = createLRUMemoryStore(5000);
@@ -397,6 +406,8 @@ export class RunAttemptSystem {
397406
});
398407
}
399408

409+
this.#checkDuplicateAttemptStart(latestSnapshot, runnerId);
410+
400411
const taskRun = await this.$.runStore.findRun(
401412
{
402413
id: runId,
@@ -1271,7 +1282,7 @@ export class RunAttemptSystem {
12711282
run,
12721283
snapshot: {
12731284
executionStatus: "EXECUTING",
1274-
description: "Attempt failed with a short delay, starting a new attempt",
1285+
description: RETRY_IMMEDIATELY_SNAPSHOT_DESCRIPTION,
12751286
},
12761287
previousSnapshotId: latestSnapshot.id,
12771288
previousSnapshot: latestSnapshot,
@@ -2045,6 +2056,46 @@ export class RunAttemptSystem {
20452056
});
20462057
}
20472058

2059+
/**
2060+
* A runner starts an attempt from the PENDING_EXECUTING snapshot a dequeue wrote, or from the
2061+
* EXECUTING snapshot written when a failed attempt is retried in the same process. Any other
2062+
* executing snapshot means an attempt is already running.
2063+
*/
2064+
#checkDuplicateAttemptStart(
2065+
latestSnapshot: Pick<
2066+
EnhancedExecutionSnapshot,
2067+
"id" | "runId" | "executionStatus" | "description" | "attemptNumber" | "runnerId"
2068+
>,
2069+
runnerId: string | undefined
2070+
) {
2071+
const mode = this.options.duplicateAttemptStartCheck ?? "log";
2072+
if (
2073+
mode === "off" ||
2074+
!isExecuting(latestSnapshot.executionStatus) ||
2075+
latestSnapshot.description === RETRY_IMMEDIATELY_SNAPSHOT_DESCRIPTION
2076+
) {
2077+
return;
2078+
}
2079+
2080+
const enforced = mode === "enforce";
2081+
this.duplicateAttemptStartsCounter.add(1, { outcome: enforced ? "refused" : "logged" });
2082+
this.$.logger.warn("startRunAttempt: the run already has an attempt executing", {
2083+
runId: latestSnapshot.runId,
2084+
snapshotId: latestSnapshot.id,
2085+
executionStatus: latestSnapshot.executionStatus,
2086+
attemptNumber: latestSnapshot.attemptNumber,
2087+
executingRunnerId: latestSnapshot.runnerId,
2088+
runnerId,
2089+
enforced,
2090+
});
2091+
2092+
if (enforced) {
2093+
throw new ServiceValidationError("The run already has an attempt executing", 409, {
2094+
code: "ATTEMPT_ALREADY_EXECUTING",
2095+
});
2096+
}
2097+
}
2098+
20482099
async #permanentlyFailRun({
20492100
runId,
20502101
latestSnapshot,

0 commit comments

Comments
 (0)