Stop Balanced-mode host shutdown paying a full agent reply window (GH-3781) - #3782
Merged
Conversation
…-3781) `IHost.StopAsync()` on a Balanced node holding a large agent load did not return for tens of minutes. A dump of the wedged process (`dumpasync`) put it in `AgentCommandDispatcher.DisposeAsync()`, awaiting a lane worker executing an `AssignAgents` chunk aimed at a peer that had already shut down. Measured, by letting the repro run out rather than killing it: 17m17s, then it released and passed. Against the bounded-teardown control (2m18s) that is ~15 minutes inside StopAsync -- exactly `AgentBatchTimeouts.ReplyWindowFor(30)`. It was never a deadlock; it was one reply window per queued command, per lane, and 25.5 minutes at the shipped `AgentStartBatchSize = 50`. Now 1m57s. Three compounding causes: 1. `ReplyListener` registered the caller's cancellation token BEFORE assigning `_completion`. `CancellationTokenRegistration` runs the callback synchronously for a token that is already cancelled, so `onCancellation`'s `_completion?.TrySetException(...)` fired against null and did nothing -- the listener then waited out its whole reply window. Every shutdown path passes an already-cancelled token, so this is not agent-specific: any `InvokeAsync<T>` handed a cancelled token sat out its full window instead of failing fast. Agent shutdown is only where that window is 25 minutes long. `ReplyTracker` also no longer registers a listener that completed inside its own constructor, which nothing would ever have removed. 2. The dispatcher was built with the runtime-wide `Cancellation` rather than `_agentCancellation`, unlike `NodeAgentController` and `DeferredAgentCommandRunner`. `StopAsync` cancels `_agentCancellation` first and only calls `DurabilitySettings.Cancel()` AFTER `teardownAgentsAsync` has returned -- so the token that would have unwedged the lane was cancelled after the thing waiting on it had already finished waiting. 3. `DisposeAsync` drained its lanes instead of abandoning them. Completing a channel writer does not discard buffered items, so shutdown executed every command still queued for a cluster the node was leaving, one reply window at a time. It now latches, empties the queues, and waits each lane against `LaneShutdownTimeout` so no future non-cancellable await can wedge `IHost.StopAsync()` again. Both new dispatcher tests are written to fail on an assertion rather than to hang, which for a regression test about a shutdown that never returns is the whole point. All five new tests were mutation-tested against their own fix. Closes #3781 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA
`SlowTests` has never run in any workflow — no job, no Nuke target — despite holding the only real-host reproductions of the GH-3753 agent assignment chain. That is how the Balanced-mode shutdown wedge fixed in this PR sat undetected in the very suite written to catch that class of bug: nothing ever ran it. Postgres is the only infrastructure it needs; the SqlServer and Kafka project references are transitive and nothing in the suite opens either. Deliberately one unsharded job to start with. The point of this commit is to MEASURE what the suite costs on a hosted runner against the same 20 minute cap every other job answers to. If it does not fit, the answer is to shard it the way CIMarten and CIPolecat were sharded (#3350), balanced on the measured per-class durations — not to raise the cap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA
Measured rather than guessed: the first CI run of the whole SlowTests project was still going when the standard 20 minute cap killed it (20m15s, cancelled). That cap is a real signal for the PR matrix -- a job that needs longer is a job that needs splitting -- but splitting is the wrong answer here. This suite is wall-clock bound by construction: it waits out health checks, assignment evaluations and agent starts against real Postgres-backed hosts, so no amount of sharding makes it cheap, it only makes it wide. And it would add that cost to every push for a suite whose subject barely changes. So it moves to slow-tests.yml, workflow_dispatch only, with a 60 minute cap that is still bounded -- a genuine wedge like GH-3781 fails the job rather than burning a runner for six hours. Run it deliberately: before a release, after anything touching agent assignment / node lifecycle / durability, and when triaging a report like #3753 or #3781. The CISlowTests Nuke target stays, so `./build.sh CISlowTests` is the one command a developer needs locally, and tests.yml keeps a note saying where the job went and why -- the failure mode this whole exercise exposed is a suite nobody knows is not running. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA
This was referenced Aug 4, 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.
Closes #3781.
IHost.StopAsync()on a Balanced-mode node holding a large agent load did not return. A dump of thewedged process (
dotnet-dump collect+dumpasync --coalesce—dotnet-stackcannot see this,because the wedge is an await on the GC heap, not on a thread stack) put it here:
The leader was stuck in
AgentCommandDispatcher.DisposeAsync(), waiting on a lane worker executingan
AssignAgentschunk aimed at a peer that had already shut down.It was never a deadlock
I let the wedged repro run out instead of killing it:
agent_reassignment_at_scale, unfixed.WaitAsync(20.Seconds())teardown control~15 minutes inside
StopAsync, which is exactlyAgentBatchTimeouts.ReplyWindowFor(30)=30s + 30 x 30s= 930s. So it is one full agent-batch reply window per queued command, per lane— 25.5 minutes at the shipped
AgentStartBatchSize = 50— not "hangs forever". Nobody hadwaited it out. That distinction is the fix: there is no lock to break, there is a timeout being paid
in full during shutdown because the cancellation that should short-circuit it was dropped.
Three compounding causes
1.
ReplyListenersilently dropped an already-cancelled tokenCancellationTokenRegistrationinvokes the callback synchronously when the token is alreadycancelled, so
onCancellation's_completion?.TrySetException(...)fired againstnulland didnothing. The listener then sat out its entire reply window.
This one is not agent-specific. Every shutdown path passes an already-cancelled token, so any
InvokeAsync<T>in Wolverine handed a cancelled token waited its whole window rather than failingfast. Agent shutdown is just where that window is 25 minutes long.
ReplyTrackeradditionally nolonger registers a listener that completed inside its own constructor — nothing would ever have
taken that entry back out.
2. The dispatcher held the wrong cancellation token
It was built with the runtime-wide
Cancellation;NodeAgentControllerandDeferredAgentCommandRunnerboth take_agentCancellation.Token. AndStopAsynccancels_agentCancellationat the top but only callsDurabilitySettings.Cancel()afterteardownAgentsAsynchas returned — so the token that would end the lane loop was cancelled afterthe thing awaiting it had already finished awaiting.
3.
DisposeAsyncdrained its lanes instead of abandoning themWriter.TryComplete()does not discard what is already buffered —ReadAsynckeeps handing it out— so shutdown executed every command still queued for a cluster the node was in the middle of
leaving, one reply window each. It now latches
_disposing, empties the lane queues (releasingtheir in-flight claims so a later leader can re-issue the work), and waits each lane against
LaneShutdownTimeout, logging the lane it gives up on. That last part is a backstop: the node's ownderegistration sits behind this in
teardownAgentsAsync, and no future non-cancellable await insidea command should be able to hold
IHost.StopAsync()again.Tests
Five new tests, all mutation-tested — each was confirmed to fail with only its own fix reverted,
not merely observed to pass:
response_handling.a_token_that_is_already_cancelled_fails_the_listener_immediately(+ theafter-registration twin as the control)
per_destination_lane_dispatch.disposal_abandons_the_commands_still_queuedper_destination_lane_dispatch.disposal_gives_up_on_a_lane_that_ignores_cancellationBoth dispatcher tests are deliberately written so the unfixed code fails them on an assertion
rather than by hanging — the first version deadlocked the test runner under mutation, which for a
regression test whose subject is a shutdown that never returns rather defeats the point.
Verified: CoreTests 2230/2230 green; all three
SlowTests.Agentsscale classes green (5m10s);dotnet build wolverine.slnx -c Releaseclean, 0 warnings.Deliberately not done
WolverineRuntime.StopAsync(CancellationToken)accepts the host's shutdown token and never uses it,so
HostOptions.ShutdownTimeoutis inert for every slow teardown path. Threading it through lookstempting but would cut off
stopAllAgentsAsync, which GH-3604 deliberately built to use the wholeSIGTERM grace window for thousands of subscription shards — a 30s default would start SIGKILLing
mid-drain, which is the failure that change existed to fix. The bounded lane wait is the safer
backstop. Worth its own issue if we want
ShutdownTimeoutto mean something here.Follow-on for #3779
With this in,
agent_reassignment_at_scale.DisposeAsyncneeds no timeout guard andSlowTestscango into a CI job. Bounding the teardown in the test alone would have made CI green while hiding a
production shutdown wedge.
🤖 Generated with Claude Code
https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA