fix(workers): cancellable shutdown wait and per-tick watchdog (#75) - #85
Merged
Conversation
run_forever did `await tick()` then `await sleep(interval)`, checking stop only at the loop top. Two operability gaps followed: - Setting stop did not interrupt an in-flight sleep, so a SIGTERM just after a tick waited up to a full interval before run_workers returned and engine.dispose() ran. With the idempotency reaper's 3600s default that is up to an hour -- past any grace period, so the process is SIGKILLed and disposal is skipped. - No per-tick timeout: a tick hung on a slow query or lock wait pinned the worker indefinitely with no cancellation and no chance to see stop. Make the inter-tick wait cancellable via asyncio.wait_for(stop.wait(), timeout=interval) -- stop firing mid-wait exits promptly; the interval elapsing is a normal boundary. Wrap each tick in asyncio.wait_for(tick(), timeout=tick_timeout) so an overrunning tick is cancelled, logged, and the loop recovers next interval. The timeout is Settings.worker_tick_timeout_s (default 120s), a generous backstop; cancelling mid-tick is safe because each per-item unit of work is its own bounded transaction (ADR-0017), so no partial state is left behind. run_forever's seams change from `sleep` to `run_tick`/`wait`, keeping the orchestration deterministic while the real helpers are unit-tested for the watchdog-cancel and stop-interrupt behaviors. Documented in ADR-0027 (refines ADR-0017). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #75.
Problem
run_foreverdidawait tick()thenawait sleep(interval), checkingstoponly at the top of the loop:stopdoes not interrupt an in-flightawait asyncio.sleep(interval). The idempotency reaper's interval defaults to 3600s, so a SIGTERM arriving just after a tick waits up to an hour beforerun_workersreturns andengine.dispose()runs — past any orchestrator grace period, so the process is SIGKILLed and clean disposal is skipped.stop, silently wedging background reconciliation.Both slipped through because the unit tests injected deterministic
sleep/stopseams that sidestepped realasyncio.sleepblocking.Fix
stopis present, wait withasyncio.wait_for(stop.wait(), timeout=interval)—stopfiring mid-wait exits promptly; the interval elapsing (TimeoutError) is a normal boundary. Shutdown latency drops from up to a full interval to ~0.asyncio.wait_for(tick(), timeout=tick_timeout); an overrunning tick is cancelled, logged at WARNING, and the loop recovers next interval. Timeout isSettings.worker_tick_timeout_s(default 120s), a generous backstop. Cancelling mid-tick is safe — every per-item unit of work is its own bounded transaction (ADR-0017), so no partial state is left behind.The driver's seams change from
sleeptorun_tick/wait, keeping orchestration deterministic; the real_run_tickand_wait_intervalhelpers are unit-tested directly for the watchdog-cancel and stop-interrupt behaviors.Scope
Implements the two core fixes (interruptible sleep + watchdog). The optional "fixed wall-clock cadence to avoid drift" and explicit
gathercancellation from #75 are deliberately omitted — the interruptible wait already makes the stop event prompt, makinggathercancellation redundant.Documented in ADR-0027 (refines ADR-0017).
Tests
test_worker_loop.pyrewritten:_wait_intervalreturns early on stop / interrupts mid-wait;_run_tickwatchdog cancels a hung tick and swallows the timeout;run_foreverorchestration via fast seam fakes.test_run_workers.pyasserts all three loops are armed with the 120s watchdog.test_settings.pycovers the new default.make verifygreen locally: 511 passed, 99.10% coverage.🤖 Generated with Claude Code