Skip to content

Commit 90ff994

Browse files
authored
fix: abort a running job when its row is gone, not just when canceled (#185)
The cancellation watcher polls the job row to detect cancellation. If the row no longer exists (deleted or truncated while the job runs), it could never observe "canceled", so a long-running, non-cooperative job was never aborted and blocked engine shutdown (executorManager.destroy waits for active jobs). Treat a missing row the same as a cancellation: abort the run so it stops and shutdown can proceed. Fixes the integration suite hang where a test truncates the DB and then stops Sidequest while a long TimeoutJob is still running.
1 parent a8a71b4 commit 90ff994

2 files changed

Lines changed: 32 additions & 4 deletions

File tree

packages/engine/src/execution/executor-manager.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,31 @@ describe("ExecutorManager", () => {
236236
await executorManager.destroy();
237237
});
238238

239+
sidequestTest("aborts a running job when its row no longer exists", async ({ backend, config }) => {
240+
await backend.updateJob({ ...jobData, state: "claimed", claimed_at: new Date() });
241+
242+
const queryConfig = await grantQueueConfig(backend, { name: "default", concurrency: 1 });
243+
const executorManager = new ExecutorManager(backend, config);
244+
245+
// Simulate the row being deleted/truncated while the job runs: the watcher must still stop it.
246+
const getJobSpy = vi.spyOn(backend, "getJob").mockResolvedValue(undefined);
247+
runMock.mockImplementationOnce(
248+
(_job: JobData, signal: AbortSignal) =>
249+
new Promise((_, reject) => {
250+
signal.addEventListener("abort", () => reject(new Error("The task has been aborted")));
251+
}),
252+
);
253+
254+
await executorManager.execute(queryConfig, jobData);
255+
256+
// eslint-disable-next-line @typescript-eslint/unbound-method
257+
expect(JobTransitioner.apply).toHaveBeenCalledWith(backend, jobData, expect.any(CancelTransition));
258+
expect(executorManager.totalActiveWorkers()).toBe(0);
259+
260+
await executorManager.destroy();
261+
getJobSpy.mockRestore();
262+
});
263+
239264
sidequestTest("should abort job execution on timeout", async ({ backend, config }) => {
240265
jobData = await backend.updateJob({ ...jobData, state: "claimed", claimed_at: new Date(), timeout: 100 });
241266

packages/engine/src/execution/executor-manager.ts

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,11 +113,14 @@ export class ExecutorManager {
113113
isRunning = true;
114114
const cancellationCheck = async () => {
115115
while (isRunning) {
116-
// The row can be missing transiently or if it was deleted; treat that as "not canceled"
117-
// rather than dereferencing undefined and crashing the polling loop.
118116
const watchedJob = await this.backend.getJob(job.id);
119-
if (watchedJob?.state === "canceled") {
120-
logger("Executor Manager").debug(`Aborting job ${job.id}: canceled`);
117+
// Abort when the job was canceled, and also when its row no longer exists (deleted or
118+
// truncated): the record is gone, so the run must be stopped rather than left to block
119+
// shutdown forever.
120+
if (!watchedJob || watchedJob.state === "canceled") {
121+
logger("Executor Manager").debug(
122+
`Aborting job ${job.id}: ${watchedJob ? "canceled" : "row no longer exists"}`,
123+
);
121124
controller.abort(new JobCanceled());
122125
isRunning = false;
123126
return;

0 commit comments

Comments
 (0)