From b9ee04165585f881f70487f46d2c5a33e8922ae9 Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 8 Oct 2026 15:03:36 -0500 Subject: [PATCH] fix(server): retain idle hold for uncertain delivery commits Keep an ambiguous database write visible to sleep admission between recovery attempts. Release the hold only when the worker stops. Co-Authored-By: Paperclip --- .../2026-10-08-instance-sleep-and-scheduler-inventory.md | 4 ++-- server/src/__tests__/delivery-queue-worker.test.ts | 4 ++++ server/src/services/delivery-queue-worker.ts | 8 +++++++- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/doc/plans/2026-10-08-instance-sleep-and-scheduler-inventory.md b/doc/plans/2026-10-08-instance-sleep-and-scheduler-inventory.md index 072f87d1ab2..c57dd0a25db 100644 --- a/doc/plans/2026-10-08-instance-sleep-and-scheduler-inventory.md +++ b/doc/plans/2026-10-08-instance-sleep-and-scheduler-inventory.md @@ -201,8 +201,8 @@ acknowledgement fails, the worker conservatively keeps recovery checks active until the process restarts. Neither an empty read nor delivery of an unrelated record can prove that an ambiguous commit will not appear later. An autocommit receipt write that fails uses the same conservative recovery mode. This exceptional -path can keep an otherwise empty queue awake; it deliberately favors delivery -recovery. Normal successful transactions have no empty-queue backstop. +path also holds idle admission, so an empty durable scan cannot authorize sleep +while the commit outcome is unknown. It deliberately favors delivery recovery. Normal successful transactions have no empty-queue backstop. The supported writer boundary is one server process using its `createDb` instance and the instrumented service mutations. Startup recovers durable rows diff --git a/server/src/__tests__/delivery-queue-worker.test.ts b/server/src/__tests__/delivery-queue-worker.test.ts index eaa7371f75c..0fef220c7ac 100644 --- a/server/src/__tests__/delivery-queue-worker.test.ts +++ b/server/src/__tests__/delivery-queue-worker.test.ts @@ -46,12 +46,16 @@ describe("delivery queue workers", () => { const s = setup(); await s.worker.ready; signalDatabaseWork(s.db, s.topic, true); + expect(idleWorkSnapshot().active).toBe(1); await vi.advanceTimersByTimeAsync(3000); expect(s.run.mock.calls.length).toBeGreaterThan(3); expect(vi.getTimerCount()).toBe(1); s.hasPending.mockResolvedValueOnce(true); await vi.advanceTimersByTimeAsync(2000); expect(vi.getTimerCount()).toBe(1); + expect(idleWorkSnapshot().active).toBe(1); + await s.worker.stop(); + expect(idleWorkSnapshot().active).toBe(0); }); it("does not lose a commit racing the final empty check or overlap deliveries", async () => { diff --git a/server/src/services/delivery-queue-worker.ts b/server/src/services/delivery-queue-worker.ts index 8ee487b5e60..fb13e44d7af 100644 --- a/server/src/services/delivery-queue-worker.ts +++ b/server/src/services/delivery-queue-worker.ts @@ -26,6 +26,7 @@ export function createDeliveryQueueWorker(input: { let stopped = false; let dirty = false; let uncertainCommit = false; + let finishUncertainCommit: (() => void) | null = null; let running: Promise | null = null; let timer: ReturnType | null = null; @@ -67,7 +68,10 @@ export function createDeliveryQueueWorker(input: { // A lost COMMIT acknowledgement can precede visibility on another // connection. Even observing other pending work cannot resolve that // ambiguity. Retain a conservative recovery backstop until restart. - uncertainCommit ||= uncertain; + if (uncertain && !uncertainCommit) { + uncertainCommit = true; + finishUncertainCommit = beginIdleTrackedWork(); + } dirty = true; if (!running) arm(0); } @@ -83,6 +87,8 @@ export function createDeliveryQueueWorker(input: { if (timer) clearTimeout(timer); timer = null; await running; + finishUncertainCommit?.(); + finishUncertainCommit = null; }, }; }