From acaa86e88c0a93d39e98e685efe1158919aa25dd Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 30 Oct 2026 16:10:20 -0500 Subject: [PATCH] fix: release settled deadline work holds during drain Preserve dirty generations with an in-memory resume deadline while allowing settled transaction holds to end during drain. Keep unresolved database intent fenced by the coordinator. Co-Authored-By: Paperclip --- server/src/__tests__/deadline-work.test.ts | 85 +++++++++++++++++++++- server/src/services/deadline-work.ts | 13 ++-- 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/server/src/__tests__/deadline-work.test.ts b/server/src/__tests__/deadline-work.test.ts index 7b852977345..7657a7e9d99 100644 --- a/server/src/__tests__/deadline-work.test.ts +++ b/server/src/__tests__/deadline-work.test.ts @@ -1,4 +1,5 @@ -import { installDatabaseWorkSignals } from "../../../packages/db/src/work-signals.js"; +import { idleWorkSnapshot } from "../services/task-admission.js"; +import { installDatabaseWorkSignals, signalDatabaseWork, databaseWorkPending } from "../../../packages/db/src/work-signals.js"; import { afterEach, expect, it, vi } from "vitest"; import { createDeliveryWorkCoordinator } from "../services/delivery-work-coordinator.js"; import { registerDeadlineWork } from "../services/deadline-work.js"; @@ -39,3 +40,85 @@ it.each([ expect(run).toHaveBeenCalledTimes(2); expect(onError).not.toHaveBeenCalled(); }); + +it("releases a settled first-write hold during drain and retains a probe-free resume wake", async () => { + vi.useFakeTimers(); + let draining = false; + const owner = installDatabaseWorkSignals({ + execute: vi.fn(async () => [{ status: "committed" }]), + transaction: async (callback: (tx: object) => Promise) => callback({ execute: async () => [{ xid: "1" }] }), + }); + const run = vi.fn(async () => {}), next = vi.fn(async () => null); + coordinator = createDeliveryWorkCoordinator({ owner, canRun: () => !draining, canReconcile: () => true, onError: vi.fn() }); + const worker = registerDeadlineWork(coordinator, DELIVERY_QUEUES.agentTimers, { run, next, canRun: () => !draining, retryMs: 30_000 }); + await worker.ready; + draining = true; + const before = idleWorkSnapshot().active; + await owner.transaction(async tx => { await signalDatabaseWork(tx, DELIVERY_QUEUES.agentTimers); }); + await vi.advanceTimersByTimeAsync(0); + expect(idleWorkSnapshot().active).toBe(before); + expect(coordinator.nextWakeAt()).not.toBeNull(); + await vi.advanceTimersByTimeAsync(3 * 30_000); + expect(run).toHaveBeenCalledOnce(); expect(next).toHaveBeenCalledOnce(); expect(owner.execute).not.toHaveBeenCalled(); + draining = false; + await vi.advanceTimersByTimeAsync(30_000); + expect(run).toHaveBeenCalledTimes(2); expect(coordinator.nextWakeAt()).toBeNull(); +}); + +it("retains failed work through drain and preserves a future domain deadline", async () => { + vi.useFakeTimers(); + let draining = false; + const owner = installDatabaseWorkSignals({ execute: vi.fn(async () => []) }); + const run = vi.fn(async () => { if (run.mock.calls.length === 1) { draining = true; throw new Error("retry debt"); } }); + const deadline = Date.now() + 600_000; + const next = vi.fn(async () => deadline); + const onError = vi.fn(); + coordinator = createDeliveryWorkCoordinator({ owner, canRun: () => !draining, canReconcile: () => true, onError }); + const worker = registerDeadlineWork(coordinator, DELIVERY_QUEUES.agentTimers, { run, next, canRun: () => !draining, retryMs: 30_000 }); + await worker.ready; + await vi.advanceTimersByTimeAsync(90_000); + expect(run).toHaveBeenCalledOnce(); expect(next).not.toHaveBeenCalled(); expect(owner.execute).not.toHaveBeenCalled(); + draining = false; await vi.advanceTimersByTimeAsync(30_000); + expect(run).toHaveBeenCalledTimes(2); expect(coordinator.nextWakeAt()).toBe(deadline); + draining = true; await vi.advanceTimersByTimeAsync(30_000); + expect(coordinator.nextWakeAt()).toBe(deadline); expect(onError).toHaveBeenCalledOnce(); +}); + +it("does no domain probes in warm standby and reconstructs after admission opens", async () => { + vi.useFakeTimers(); + let standby = true; + const owner = installDatabaseWorkSignals({ execute: vi.fn(async () => []) }); + const run = vi.fn(async () => {}), next = vi.fn(async () => null); + coordinator = createDeliveryWorkCoordinator({ owner, canRun: () => !standby, canReconcile: () => !standby, onError: vi.fn() }); + const worker = registerDeadlineWork(coordinator, DELIVERY_QUEUES.agentTimers, { run, next, canRun: () => !standby, retryMs: 30_000 }); + await worker.ready; await vi.advanceTimersByTimeAsync(90_000); + expect(run).not.toHaveBeenCalled(); expect(next).not.toHaveBeenCalled(); expect(owner.execute).not.toHaveBeenCalled(); + standby = false; await vi.advanceTimersByTimeAsync(30_000); + expect(run).toHaveBeenCalledOnce(); expect(coordinator.nextWakeAt()).toBeNull(); +}); + +it("does not release an ambiguous database intent while dispatch is drained", async () => { + vi.useFakeTimers(); + let draining = false, committed = false; + const owner = installDatabaseWorkSignals({ + execute: vi.fn(async () => [{ status: committed ? "committed" : "in progress" }]), + transaction: async (callback: (tx: object) => Promise) => { + await callback({ execute: async () => [{ xid: "2" }] }); + throw new Error("lost acknowledgement"); + }, + }); + coordinator = createDeliveryWorkCoordinator({ owner, canRun: () => !draining, canReconcile: () => true, onError: vi.fn() }); + const run = vi.fn(async () => {}); + const worker = registerDeadlineWork(coordinator, DELIVERY_QUEUES.agentTimers, { run, next: async () => null, canRun: () => !draining, retryMs: 30_000 }); + await worker.ready; draining = true; + const before = idleWorkSnapshot().active; + await expect(owner.transaction(async tx => { await signalDatabaseWork(tx, DELIVERY_QUEUES.agentTimers); })).rejects.toThrow("lost acknowledgement"); + await vi.advanceTimersByTimeAsync(0); + expect(idleWorkSnapshot().active).toBe(before + 1); + expect(databaseWorkPending(owner, DELIVERY_QUEUES.agentTimers)).toBe(true); + committed = true; await vi.advanceTimersByTimeAsync(30_000); + expect(databaseWorkPending(owner, DELIVERY_QUEUES.agentTimers)).toBe(false); + expect(idleWorkSnapshot().active).toBe(before); + draining = false; await vi.advanceTimersByTimeAsync(30_000); + expect(run).toHaveBeenCalledTimes(2); expect(coordinator.nextWakeAt()).toBeNull(); +}); diff --git a/server/src/services/deadline-work.ts b/server/src/services/deadline-work.ts index 306f340d9fc..15ba76abdae 100644 --- a/server/src/services/deadline-work.ts +++ b/server/src/services/deadline-work.ts @@ -26,7 +26,7 @@ export function registerDeadlineWork( pausePassiveMaintenance: true, onSettled: hints => { requested++; task.onSettled?.(hints); }, run: async signal => { - if (!task.canRun()) { requested++; next = null; return; } + if (!task.canRun()) { requested++; return; } if (Date.now() < earliest) { next = earliest; return; } if (Date.now() < coalesceUntil && (next === null || next > Date.now())) { next = next === null ? coalesceUntil : Math.min(next, coalesceUntil); @@ -39,8 +39,6 @@ export function registerDeadlineWork( if (signal.aborted || !task.canRun()) { next = null; return; } const deadline = await task.next(); - - if (!task.canRun()) { next = null; return; } earliest = deadline === null || deadline > Date.now() ? (task.minimumSpacingMs ? Date.now() + task.minimumSpacingMs : 0) @@ -49,8 +47,13 @@ export function registerDeadlineWork( coalesceUntil = Date.now() + (task.notificationCoalesceMs ?? 0); reconciled = generation; }, - hasPending: async () => requested !== reconciled, - nextRunAt: () => next, + // A settled producer must release its admission hold during drain. Keep + // the unreconciled generation and an in-memory resume wake, without asking + // the domain to probe its database while dispatch is closed. + hasPending: async () => task.canRun() && requested !== reconciled, + nextRunAt: () => !task.canRun() && requested !== reconciled + ? Math.min(next ?? Infinity, Date.now() + task.retryMs) + : next, }); return { ...worker, wake: () => { requested++; worker.wake(); } }; }