Skip to content

Extend the pending-assignment ledger to reassignments (GH-3852) - #3853

Merged
jeremydmiller merged 2 commits into
mainfrom
gh-3852/pending-reassignment-ledger
Aug 5, 2026
Merged

Extend the pending-assignment ledger to reassignments (GH-3852)#3853
jeremydmiller merged 2 commits into
mainfrom
gh-3852/pending-reassignment-ledger

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Closes #3852.

The gap

The GH-3698 pending-assignment ledger closed the re-decision hole for first-time assignments and silently excluded every reassignment. An agent being moved is still listed in its source node's persisted ActiveAgents, so Agent.OriginalNode is set, and applyPendingAssignments skipped it outright:

// Already running somewhere: not a pending placement.
if (agent.OriginalNode != null) continue;

The ledger armed on a ReassignAgent but could never apply one, and TryBuildAssignmentCommand's OriginalNode != null branch consulted no ledger at all — it went straight from "assigned != original" to a fresh ReassignAgent. So the leader re-decided the same move from scratch on every evaluation until the source's assignment row finally disappeared.

Scale

Against the reported cluster shape — 512 databases x 17 agents across five nodes, group affinity, three incumbents holding everything and two newcomers ramping in — with the persisted snapshot frozen:

total agents: 8704
cycle 1: reassign decisions = 3468
cycle 2: reassign decisions = 3468
...
cycle 6: reassign decisions = 3468

Roughly 13 cycles of that is the reported ~45,000 in six minutes. Each decision writes an AssignmentChanged row — _observer.AssignmentsChanged runs before batchCommands and before the dispatcher, so nothing downstream dedupes it — feeding the node-record volume problem of #3658 at full rate.

The outcome converged because ReassignAgents carries hand-written set-based value equality, so the dispatcher collapses an identical re-emitted batch while its lane is busy. That hold ends the moment the batch completes, though, so a re-decision landing before the snapshot catches up re-runs a real StopAgents round trip against a source that has already let go.

On the version framing in the issue: 6.24.4 already carried the GH-3698 ledger (landed 7/29, ahead of the 8/1 bump), so the original measurement was taken with the intended mitigation in place. #610 shortens the staleness window and reduces the absolute count on 6.24.6, but cannot close the gap — any slow ramp re-opens it at the same per-cycle rate.

Changes

  • applyPendingAssignments ends the wait only on a matching node, so a move in flight sets PendingNode and holds its placement (before the family distributes, so the distribution balances around it rather than re-splitting and yanking it back).
  • TryBuildAssignmentCommand returns false for a move already dispatched and still live. PendingRetryDue re-drives one that is never confirmed.
  • AgentCommandDispatcher tracks reassignments in a _moving map, keyed to the node the agents are moving to — not the command's lane, which is the source. Kept deliberately separate from _inFlight because Enqueue must never suppress a reassignment: it carries a stop, and dropping that is the two-copies bug StartedAgentsOf guards against. Without this map the hold falls back to the ledger TTL (2 x CheckAssignmentPeriod, 60s), which a batch's own reply window trivially outlives.

Coverage

New pending_reassignment_ledger. 4 of the 8 fail without the fix; the rest guard against over-suppression — a pause still stops an agent mid-move, confirmation still clears the ledger, an unconfirmed move is still re-driven, and the dispatcher still collapses an identical in-flight batch.

Full CoreTests green (2272 passed), wolverine.slnx Release build clean.

🤖 Generated with Claude Code

https://claude.ai/code/session_01WHAuhdWS3XeAk16swV9G8m

jeremydmiller and others added 2 commits August 5, 2026 11:14
The GH-3698 ledger closed the re-decision hole for first-time assignments and
silently excluded every reassignment. An agent being moved is still listed in
its SOURCE node's persisted ActiveAgents, so Agent.OriginalNode is set, and
applyPendingAssignments skipped it outright:

    if (agent.OriginalNode != null) continue;

The ledger armed on a ReassignAgent but could never apply one, and
TryBuildAssignmentCommand's OriginalNode != null branch consulted no ledger at
all. So the leader re-decided the same move from scratch on every evaluation
until the source's assignment row finally disappeared.

Against the reported cluster shape -- 512 databases x 17 agents across five
nodes, group affinity, three incumbents and two newcomers ramping in -- that is
3,468 reassignment decisions per cycle against a frozen snapshot, repeating
indefinitely. Roughly 13 cycles of that is the reported ~45,000 in six minutes.
Each decision writes an AssignmentChanged row (the observer runs before
batchCommands and before the dispatcher, so nothing downstream dedupes it),
feeding the node-record volume problem of #3658 at full rate.

The outcome converged because ReassignAgents carries set-based value equality,
so the dispatcher collapses an identical re-emitted batch while its lane is
busy. That hold ends the moment the batch completes, though, so a re-decision
landing before the snapshot catches up re-runs a real StopAgents round trip
against a source that has already let go.

Three changes:

- applyPendingAssignments ends the wait only on a MATCHING node, so a move in
  flight sets PendingNode and holds its placement.
- TryBuildAssignmentCommand returns false for a move already dispatched and
  still live. PendingRetryDue re-drives one that is never confirmed.
- AgentCommandDispatcher tracks reassignments in a _moving map, keyed to the
  node the agents are moving TO (not the command's lane, which is the source).
  Kept separate from _inFlight because Enqueue must never suppress a
  reassignment -- it carries a stop. Without this the hold falls back to the
  ledger TTL (60s), which a batch's own reply window trivially outlives.

Regression coverage in pending_reassignment_ledger; 4 of the 8 fail without the
fix, the rest guard against over-suppression -- a pause still stops an agent
mid-move, confirmation still clears the ledger, an unconfirmed move is still
re-driven.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WHAuhdWS3XeAk16swV9G8m
StartAgents.StartBatchAsync got bounded parallelism in GH-3604/D3 for exactly
this reason -- "daemon-agent starts are I/O bound, so a 50-agent chunk started
one-at-a-time was seconds of dead wall-clock that blew the reply window". The
stop side is the same shape and never got it: StopAgents ran a plain serial
foreach.

At AgentStartBatchSize = 50 against a source whose shards are slow to let go,
that is the whole chunk's stop cost in series before ReassignAgents can cascade
a single start, and every agent in the chunk is down for the duration.

This was survivable only because the leader re-decided the same move on every
evaluation cycle, trickling agents onto the destination through individual
ReassignAgent commands alongside the batch. Removing that churn in the previous
commit is what exposed the serial stop as the real cost -- SlowTests'
agent_reassignment_at_scale caught it, and it is the reason that test is worth
running by hand (it is in no CI workflow).

agent_reassignment_at_scale, three runs each:

                          main    ledger fix   + parallel stops
  fresh nodes >=20 each   38.0s   74.1s        14.0s
  converged at           176.3s  176.3s        31.1s
  worst backslide             8      72           40

Convergence is 5.7x faster than main, not merely restored. The backslide is
deeper than main's 8 but well inside the test's <= 70 bar ("never worse than one
reassignment chunk in flight") and it now lasts seconds rather than the ~100s a
serial chunk took, so far fewer agent-seconds are spent down.

StopLocallyAsync is per-agent independent -- a ConcurrentDictionary lookup and
removal, the agent's own StopAsync, and a persistence delete -- so the fan-out
is safe for distinct URIs, same as the start side. The per-agent try/catch is
preserved so one failing stop still costs only itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WHAuhdWS3XeAk16swV9G8m
@jeremydmiller

Copy link
Copy Markdown
Member Author

SlowTests follow-up — found and fixed a real regression, then a pre-existing one

SlowTests runs in no CI workflow, so I ran the agent suites by hand. agent_reassignment_at_scale (the GH-3753/GH-3749 harness) went red on the first commit. Baselined against origin/main in a worktree to be sure it wasn't ambient noise — main passed, the branch failed on two assertions.

The timelines are identical for the first 15 seconds, then split hard:

        main                              branch (ledger fix only)
  16s | 1: 234  2: 238  3:   2  | 474    16s | 1: 234  2: 238  3:   0  | 472
  20s | 1: 232  2: 236  3:   4  | 472    20s | 1: 232  2: 236  3:   0  | 468
  30s | 1: 227  2: 231  3:  13  | 471    30s | 1: 227  2: 231  3:   0  | 458
  40s | 1: 222  2: 226  3:  23  | 471    40s | 1: 222  2: 226  3:   0  | 448

Main's node 3 climbs while the total stays flat at 471 — stops and starts interleave. The branch's node 3 sits at 0 while the total declines 2/sec: the stops land and the starts never do, then arrive in big step jumps (90 → 110 → 120 → 140 → 160) at the very end.

Cause

StopAgents.ExecuteAsync ran a plain serial foreach. StartAgents.StartBatchAsync got bounded parallelism in GH-3604/D3 for exactly this reason — "daemon-agent starts are I/O bound, so a 50-agent chunk started one-at-a-time was seconds of dead wall-clock that blew the reply window" — and the stop side, which is the same shape, never did.

So at AgentStartBatchSize = 50 against a source slow to let go, the entire chunk's stop cost runs in series before ReassignAgents can cascade a single start, and every agent in the chunk is down for the duration.

That was survivable only because of the churn this PR removes. The leader re-deciding the same move every cycle was trickling agents onto the destination through individual ReassignAgent commands alongside the batch. Take the churn away and the serial stop is exposed as the actual cost. The bug was always there; the wasteful re-decisions were hiding it.

Result

agent_reassignment_at_scale:

main ledger fix + parallel stops
fresh nodes >=20 each 38.0s 74.1s 14.0s
converged at 176.3s 176.3s 31.1s
worst backslide 8 72 40

Convergence is 5.7x faster than main, not merely restored. The backslide is deeper than main's 8 but well inside the test's <= 70 bar ("never worse than one reassignment chunk in flight"), and it now lasts seconds instead of the ~100s a serial chunk took — far fewer agent-seconds spent down.

StopLocallyAsync is per-agent independent (a ConcurrentDictionary lookup and removal, the agent's own StopAsync, a persistence delete), so the fan-out is safe for distinct URIs exactly as on the start side. The per-agent try/catch is preserved so one failing stop still costs only itself.

Verification

  • All 3 SlowTests.Agents suites green.
  • Full CoreTests green (2275 passed).
  • New stop_agents_batch_parallelism — 3 tests, and the bounded-parallelism one is red against the old serial loop.
  • wolverine.slnx Release build clean.

@jeremydmiller
jeremydmiller merged commit 254789c into main Aug 5, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Leader re-decides ~45,000 ReassignAgent placements in six minutes during a ramp — is it still true now that jasperfx#610 has landed?

1 participant