Bound the shutdown drain so a stuck handler cannot stall the host - #71
Merged
Conversation
StopAsync awaited the loop task with no bound. A handler that ignores its CancellationToken — a blocking HTTP call, a driver that swallows the token, a lock held elsewhere — therefore blocked shutdown forever. With leasing enabled that is worse than a slow exit: the replica keeps holding its processor lease while the host refuses to terminate, so a blue/green handoff stalls until the orchestrator SIGKILLs the pod. Every long-lived loop now bounds its shutdown wait on the new ControlLoopOptions.DrainTimeout (default 5 s) and logs a warning when it elapses: ControlLoop (both the sequential await and the pipelined worker drain), EventStoreHead and DeadLetterRetryLoop. StopAsync also honours the caller's CancellationToken, which it previously ignored. Abandoning the wait cannot lose an event. A worker that never returns also never calls MarkCompleted, so PositionWatermark.SafeCheckpoint stays behind its position and the final flush cannot checkpoint past unprocessed work — the event is simply re-delivered on the next start. Teardown is deferred rather than forced when a drain is abandoned: the pipeline's CancellationTokenSource and the processor are disposed by a continuation that runs once the abandoned tasks actually exit, so their token registrations stay valid meanwhile. DisposeAsync is now idempotent.
This was referenced Jul 31, 2026
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.
The problem
ControlLoop.StopAsyncawaited the loop task with no bound:A handler that ignores its
CancellationToken— a blocking HTTP call, a driver that swallows the token, alockheld by another thread — therefore blocks shutdown forever.EventStoreHead.StopAsyncandDeadLetterRetryLoop.StopAsynchad the same shape, andRunPipelinedAsync'sfinallydid an unboundedawait Task.WhenAll(workers).With leasing enabled this is worse than a slow exit: the replica keeps holding its processor lease while the host refuses to terminate, so
LeaseAwareControlLoopGroupnever releases and a blue/green handoff stalls until the orchestrator SIGKILLs the pod.StopAsyncalso ignored thecancellationTokenthe host passed it, so even the host's own shutdown deadline had no effect.Prompted by Marten 9.16–9.22, which added
StopAndDrainTimeoutfor the same reason.The change
New
ControlLoopOptions.DrainTimeout(default 5 s), applied to every long-lived loop:ControlLoop.StopAsync_loop.WaitAsync(_drainTimeout, cancellationToken)ControlLoop.RunPipelinedAsync(worker drain)Task.WhenAll(workers).WaitAsync(_drainTimeout)EventStoreHead.StopAsync_loop.WaitAsync(_drainTimeout, cancellationToken)DeadLetterRetryLoop.StopAsync_loop.WaitAsync(_drainTimeout, cancellationToken)Each logs a warning on timeout (the pipelined one includes the flushed
SafeCheckpointand the number of abandoned workers).StopAsyncnow honours the caller's token too. The group wrappers (ControlLoopGroup,LeaseAwareControlLoopGroup,DeadLetterRetryLoopGroup) need no change — they become bounded automatically.Why abandoning is safe
Abandoning the wait cannot lose an event. A worker that never returns also never calls
watermark.MarkCompleted(), soPositionWatermark.SafeCheckpoint(_inFlight[0] - 1) stays behind its position. The final flush — which still runs, withCancellationToken.None— therefore cannot checkpoint past unprocessed work. The event is simply re-delivered on the next start. At-least-once holds exactly as before.For the dead-letter loop, an abandoned claim is held by a time-bounded lease, so another worker picks it up once the lease expires.
Teardown of abandoned work
Two ordering hazards the naive version would have introduced, both handled:
RunPipelinedAsyncno longer usesusing var pipelineCts. Abandoned workers still observepipelineCts.Token, so disposal is deferred to a continuation that runs when they actually exit. On a clean drain it disposes inline, as before.ControlLoop.DisposeAsync/DeadLetterRetryLoop.DisposeAsyncdefer disposing the CTS (and, forControlLoop, the processor) the same way when the loop was abandoned.ControlLoop.DisposeAsyncis now idempotent.Validation and config
DrainTimeout <= TimeSpan.Zerois rejected asALB0004, alongside the other control-loop durations. Bound throughControlLoopOverrides(ControlLoop:DrainTimeout) and threaded throughControlLoopAssembler.Createto all four construction sites. Documented in docs/configuration.md, including how to size it againstterminationGracePeriodSeconds.Tests
New
tests/Alberto.Dcb.Tests/Subscriptions/ControlLoopDrainTimeoutTests.cs(7 tests), in the style ofControlLoopPipelinedCancellationTests:StopAsyncreturns within budget with a token-oblivious handler;DisposeAsyncafter an abandoned drain neither throws nor double-tears-down, and is idempotent;EventStoreHead.StopAsyncandDeadLetterRetryLoop.StopAsyncare bounded against backends that ignore cancellation;ControlLoopOverrides.All gates use
TaskCompletionSource, not wall-clock sleeps; every stalled fake is released in afinallyso nothing outlives the test.Local:
Alberto.Dcb.Tests1587 passed / 0 failed / 16 skipped (5 full runs),Alberto.Examples.Tests70 passed. The new file was run 8× on its own with no flake.Compatibility
Additive. The new constructor parameters are optional and trailing, so existing positional and named call sites compile unchanged; omitting them yields the 5 s default.
PublicAPI.Unshipped.txtupdated for the four new members.