Skip to content

Commit 95179cf

Browse files
committed
feat(supervisor): make checkpoint request timeouts configurable
1 parent d072615 commit 95179cf

4 files changed

Lines changed: 18 additions & 14 deletions

File tree

apps/supervisor/src/env.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@ export const Env = z
115115
TRIGGER_WARM_START_URL: z.string().optional(),
116116
TRIGGER_WARM_START_DISPATCH_URL: z.string().optional(),
117117
TRIGGER_CHECKPOINT_URL: z.string().optional(),
118+
TRIGGER_CHECKPOINT_TIMEOUT_MS: z.coerce.number().int().positive().default(5_000),
119+
TRIGGER_CHECKPOINT_RESTORE_TIMEOUT_MS: z.coerce.number().int().positive().default(30_000),
118120
TRIGGER_METADATA_URL: z.string().optional(),
119121

120122
// Warm-start delivery verification: after a warm-start hit, probe the

apps/supervisor/src/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,8 @@ class ManagedSupervisor {
382382
apiUrl: new URL(env.TRIGGER_CHECKPOINT_URL),
383383
workerClient: this.workerSession.httpClient,
384384
orchestrator: this.isKubernetes ? "KUBERNETES" : "DOCKER",
385+
timeoutMs: env.TRIGGER_CHECKPOINT_TIMEOUT_MS,
386+
restoreTimeoutMs: env.TRIGGER_CHECKPOINT_RESTORE_TIMEOUT_MS,
385387
});
386388
}
387389

apps/supervisor/src/workloadServer/index.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -290,12 +290,7 @@ export class WorkloadServer extends EventEmitter<WorkloadServerEvents> {
290290
checkpointDeleteRequests.inc({ result: "sent" });
291291
}
292292

293-
/**
294-
* cancelCheckpoints tells the checkpoint service a resumed run's in-flight checkpoint is moot.
295-
* Must be called after the reply is sent: it never delays the runner. Without it the checkpoint
296-
* is abandoned by a periodic snapshot poll instead, up to that interval later.
297-
*/
298-
private async cancelCheckpoints(runFriendlyId: string): Promise<void> {
293+
private async cancelCheckpointsAfterReply(runFriendlyId: string): Promise<void> {
299294
if (!this.checkpointClient) {
300295
checkpointCancelRequests.inc({ result: "no_client" });
301296
return;
@@ -686,7 +681,7 @@ export class WorkloadServer extends EventEmitter<WorkloadServerEvents> {
686681

687682
reply.json(continuationResult.data as WorkloadContinueRunExecutionResponseBody);
688683

689-
await this.cancelCheckpoints(params.runFriendlyId);
684+
await this.cancelCheckpointsAfterReply(params.runFriendlyId);
690685
}
691686
),
692687
}

packages/core/src/v3/serverOnly/checkpointClient.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,15 +11,22 @@ export type CheckpointClientOptions = {
1111
apiUrl: URL;
1212
workerClient: SupervisorHttpClient;
1313
orchestrator: CheckpointType;
14+
timeoutMs?: number;
15+
restoreTimeoutMs?: number;
1416
};
1517

16-
const CANCEL_TIMEOUT_MS = 5_000;
18+
const DEFAULT_TIMEOUT_MS = 5_000;
19+
const DEFAULT_RESTORE_TIMEOUT_MS = 30_000;
1720

1821
export class CheckpointClient {
1922
private readonly logger = new SimpleStructuredLogger("checkpoint-client");
2023

2124
constructor(private readonly opts: CheckpointClientOptions) {}
2225

26+
private timeout(ms = this.opts.timeoutMs ?? DEFAULT_TIMEOUT_MS): AbortSignal {
27+
return AbortSignal.timeout(ms);
28+
}
29+
2330
async suspendRun({
2431
runFriendlyId,
2532
snapshotFriendlyId,
@@ -43,6 +50,7 @@ export class CheckpointClient {
4350
type: this.opts.orchestrator,
4451
...body,
4552
} satisfies CheckpointServiceSuspendRequestBodyInput),
53+
signal: this.timeout(),
4654
}
4755
);
4856

@@ -107,6 +115,7 @@ export class CheckpointClient {
107115
"Content-Type": "application/json",
108116
},
109117
body: JSON.stringify(body),
118+
signal: this.timeout(this.opts.restoreTimeoutMs ?? DEFAULT_RESTORE_TIMEOUT_MS),
110119
}
111120
);
112121

@@ -146,6 +155,7 @@ export class CheckpointClient {
146155
"Content-Type": "application/json",
147156
},
148157
body: JSON.stringify(body),
158+
signal: this.timeout(),
149159
}
150160
);
151161

@@ -160,19 +170,14 @@ export class CheckpointClient {
160170
return true;
161171
}
162172

163-
/**
164-
* cancelCheckpoints returns "unsupported" when the route is absent, which is expected while a
165-
* newer caller runs against an older checkpoint service. The route answers 202 even when the run
166-
* has nothing in flight, so a 404 only ever means the route itself is missing.
167-
*/
168173
async cancelCheckpoints({
169174
runFriendlyId,
170175
}: {
171176
runFriendlyId: string;
172177
}): Promise<"ok" | "unsupported" | "failed"> {
173178
const res = await fetch(
174179
new URL(`/api/v1/runs/${runFriendlyId}/checkpoints/cancel`, this.opts.apiUrl),
175-
{ method: "POST", signal: AbortSignal.timeout(CANCEL_TIMEOUT_MS) }
180+
{ method: "POST", signal: this.timeout() }
176181
);
177182

178183
if (res.status === 404) {

0 commit comments

Comments
 (0)