Skip to content

Fix DistributeByGroupAffinity assigning a bumped projection version to nodes that cannot build it - #3792

Merged
jeremydmiller merged 5 commits into
JasperFx:mainfrom
erdtsieck:bugfix/blue-green-group-affinity-incumbent
Aug 3, 2026
Merged

Fix DistributeByGroupAffinity assigning a bumped projection version to nodes that cannot build it#3792
jeremydmiller merged 5 commits into
JasperFx:mainfrom
erdtsieck:bugfix/blue-green-group-affinity-incumbent

Conversation

@erdtsieck

Copy link
Copy Markdown
Contributor

DistributeByGroupAffinity documents itself as mirroring DistributeEvenlyWithBlueGreenSemantics, but on a real blue/green rollout of a multi-database store it does the opposite of that: it assigns the new version's agents to a node that cannot build them, and the new version then never starts anywhere.

Test first — the failing one in this PR is the intersection of two tests that already exist, which is why it slipped through.

What happens

Take one shard database mid-rollout. Its group (group key is the database, EventSubscriptionAgentFamily.DatabaseKeyOf) contains:

members of the group declared by
Proj/All/v22/{tenant} — previous version, currently running blue nodes
Proj/All/v23/{tenant} — new version green nodes
agents of projections whose version did not change all nodes

AllNodesHaveSameCapabilities is false, so the capability-matching branch runs. No node is capable of the whole group, so members.All(m => m.CandidateNodes.Contains(n)) is false everywhere — and the per-member fallback below is exactly the right behaviour for this.

It never runs. The || members.Any(m => m.OriginalNode == n) grandfathering keeps the incumbent blue node in candidates, so candidates.Count == 0 is false, the fallback is skipped, and:

  • incumbent resolves to null (the v23 members are unassigned, so not all members share one AssignedNode);
  • "the least-loaded candidate hosts the whole group" assigns every member to the blue node, v23 included;
  • EventSubscriptionAgentFamily.BuildAgentAsync throws Unknown event projection or subscription there, because that node's store registers v22, not v23;
  • green is never assigned anything, and the new version's tables stay empty.

Node.Assign has no capability check and MatchAgentsToCapableNodesFor is only consulted to build CandidateNodes, so nothing downstream catches it.

Single-database stores are unaffected — they take DistributeEvenlyWithBlueGreenSemantics, which matches per agent.

Where it came from

Both on 2026-07-06, in order:

  • afe02783a "Blue/green capability matching inside DistributeByGroupAffinity" — correct.
  • 748762a42 "Group placement: minimal disruption + grandfathering for stale capability snapshots" — adds the OriginalNode clause, which subsumes the genuine blue/green case as a side effect.

The grandfathering is right and this PR keeps it. a_group_lands_only_on_a_node_capable_of_running_it and a_node_already_running_a_groups_agents_stays_a_candidate_despite_a_stale_capability_snapshot both still pass; neither covers the case where both conditions hold at once, which is what a rollout looks like at every evaluation for its whole duration rather than transiently.

The fix, in two steps

1. A group is only a single placement unit when its members can actually share a host. Members are sub-partitioned by the set of nodes that declare them, and each partition placed whole. That preserves what the method exists for — affinity inside a version, so a shard database keeps one owner per version and pools scale with databases rather than nodes x databases (#486 / marten#4806). "Just always take the per-member fallback" is not the fix: it scatters one database's agents across every capable node, which is the pool explosion this method was written to prevent.

2. Partitions of one group prefer a node already hosting a sibling partition. Step 1 alone can make the pool budget worse than before. A database's group during a bump has three partitions, not two: previous version (blue-only), new version (green-only), and every projection whose version did not change — and that last one is declared by every node, so it is free to land on a third node and add a third pool set to that database. What a database costs is the number of distinct nodes holding any of its agents, so partitions now prefer a sibling's host, bounded by the same per-node ceiling the incumbent rule already respects.

With homogeneous capabilities there is exactly one partition per group and no sibling to prefer, so the common path is unchanged.

before:  Failed: 1, Passed: 8    (distribute_by_group_affinity)
after:   Passed: 10
         Passed: 287             (all of CoreTests.Runtime.Agents, net9.0 + net10.0)

Both new tests fail without their respective half — the second asserts the number of hosts per database rather than a specific placement, and without the sibling preference four databases across two blue and two green nodes spread onto three nodes each.

Scale context

This is the same 512-shard-database, ~854-tenant deployment as #3746 / #3747 and #594. The rollout shape we need is a second Deployment on the new image that builds the bumped projection version off the load balancer and then takes over serving, which leaves the cluster in mixed-capability state for the whole build — an hour or more, not the seconds a rolling update spends there. The pool arithmetic is why step 2 matters to us specifically: at 512 databases, two owners per database is affordable and three is not.

I have not run this on the cluster yet; the evidence here is the tests and the code path. If it helps, I can put a build of this on our canary (production snapshot, all 512 shards) and report what assignment actually does with a real version bump — that is the measurement loop you asked for, and the one thing this class of bug cannot get from a dev-scale reproduction.

One judgement call worth your eye: the partition key is the ordered set of declaring node ids, so two members share a partition only when the same nodes declare them. That is stricter than strictly necessary — members with overlapping-but-unequal candidate sets could sometimes still share a capable host — but it never assigns to an incapable node, and the sibling preference recovers most of what the strictness would otherwise cost. Happy to relax it to "group by whether a common capable node exists" if you prefer coarser partitions.

DistributeByGroupAffinity claims to mirror DistributeEvenlyWithBlueGreenSemantics,
but on a real blue/green rollout of a multi-database store it does the opposite.

One shard database's group spans the previous version's agents — declared by, and
running on, the blue nodes — and the new version's agents, declared only by the
green nodes. No node is capable of the whole group, so the candidate set should be
empty and the per-member fallback should place each agent on a capable node. It
isn't empty: the OriginalNode grandfathering added for stale capability snapshots
keeps the incumbent blue node as a candidate, the fallback is skipped, and the
whole group — new version included — is assigned to a node whose store does not
register that version. BuildAgentAsync then throws for every one of those agents,
the new version never starts anywhere, and the rollout silently does not happen.

The grandfathering itself is right; what was missing is that a group is only a
single placement unit when its members can actually share a host. Members are now
sub-partitioned by the set of nodes that declare them, and each partition placed
whole. That keeps affinity inside a version — a database still has one owner per
version, which is what the connection-pool budget depends on — while letting the
two versions land on their own nodes. With homogeneous capabilities there is one
partition per group, so the common path is untouched.

Covered by a test that is the intersection of the two existing capability tests:
no node can host the whole group AND an incumbent is running part of it. It fails
on main with the new version's agent assigned to the blue node.
Partitioning by capability set fixes the assignment, but it can make the thing
this method exists for worse. A shard database's group during a version bump has
three partitions, not two: the previous version (blue-only), the new version
(green-only), and every projection whose version did not change — and that last
one is declared by every node, so it is free to land on a third node and put a
third connection pool set on that database.

What a database costs is the number of distinct nodes holding any of its agents.
So partitions of one group now prefer a node that already hosts a sibling
partition, bounded by the same per-node ceiling the incumbent rule already
respects. A split group settles on one node per version, and the unchanged
projections ride along with one of them instead of claiming a node of their own.

The new test asserts the host count per database rather than a placement, and
fails without the preference: four databases across two blue and two green nodes
spread onto three nodes each.
Copilot AI review requested due to automatic review settings August 3, 2026 09:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a blue/green rollout failure mode in AssignmentGrid.DistributeByGroupAffinity where a version-bumped projection agent could be assigned to a node that cannot build/run it, causing the new version to never start. The fix preserves group affinity (to control connection-pool fanout) while safely splitting groups only when mixed capabilities make a single-host placement impossible.

Changes:

  • Split a “group” into capability-based partitions under mixed capabilities so incompatible agents (e.g., old vs. bumped projection versions) cannot be co-assigned to an incapable node.
  • Prefer placing sibling partitions of the same group onto already-used hosts (within the existing per-node ceiling) to minimize distinct hosts per database and avoid pool explosion.
  • Add regression tests covering version-bump splitting and ensuring split groups occupy only as many nodes as the capability split requires.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/Wolverine/Runtime/Agents/AssignmentGrid.Distribution.cs Partition group placement by candidate-node sets under mixed capabilities and add sibling-host preference to reduce host fanout per group/database.
src/Testing/CoreTests/Runtime/Agents/distribute_by_group_affinity.cs Add tests reproducing the blue/green version-bump scenario and asserting bounded host count per database during a split.
Suppressed comments (1)

src/Wolverine/Runtime/Agents/AssignmentGrid.Distribution.cs:331

  • Private static helper methods in this codebase use camelCase; CapabilityKey is private static but PascalCase. Renaming it will keep naming consistent with other private helpers like canSelfHeal(...) (NodeAgentController.cs:448).
    private static string CapabilityKey(Agent agent) =>
        string.Join(",", agent.CandidateNodes.Select(n => n.AssignedId).OrderBy(id => id));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Wolverine/Runtime/Agents/AssignmentGrid.Distribution.cs
The two unit tests drive AssignmentGrid.DistributeByGroupAffinity directly, so
they prove the placement rule but not that a real store reaches it. This goes
through EventSubscriptionAgentFamily.EvaluateAssignmentsAsync instead, which adds
the two steps that sit between a leader and the placement: the per-store pass
selection keyed off IEventStore.DatabaseCardinality, and RetireSupersededAgents.

The scenario is the rollout itself, over three real tenant databases. The blue
store registers a projection at V2 and is already running its agents; the green
store registers the same projection at V3 and one other projection unchanged, so
the group for each database spans a blue-only, a green-only and an every-node
partition exactly as it does in production. Capabilities come from the family's
own SupportedAgentsAsync, the grid is seeded from their union the way
NodeAgentController does, and the family that evaluates is the BLUE one — so the
green agents are reachable only through persisted node capabilities, which is the
case that broke.

It fails on main with the new version's agents on a blue node:

  event-subscriptions://marten/main/localhost.bgtenant3/trip/all/v3
  may only run on a node that declares it

The pool bound stays in the unit test rather than here: it needs agents to
outnumber nodes the way a real cluster does, and with three databases over four
nodes the per-node ceiling binds first and legitimately spreads a group's third
partition to balance load. That is the intended trade, so asserting against it
here would be asserting the wrong thing.
@erdtsieck

Copy link
Copy Markdown
Contributor Author

Pushed an end-to-end test, because the two unit tests prove the placement rule but not that a real store reaches it.

blue_green_version_bump_assignment in MartenTests/MultiTenancy runs over three real tenant databases and goes through EventSubscriptionAgentFamily.EvaluateAssignmentsAsync rather than DistributeByGroupAffinity directly, so it also covers the two steps between a leader and the placement: the per-store pass selection keyed off IEventStore.DatabaseCardinality, and RetireSupersededAgentsAsync.

The setup is the rollout itself. The blue store registers a projection at V2 and is already running its agents; the green store registers the same projection at V3 plus one other projection unchanged — so each database's group spans a blue-only, a green-only and an every-node partition, the same shape production has. Capabilities come from the family's own SupportedAgentsAsync, the grid is seeded from their union the way NodeAgentController does, and the family that evaluates is the blue one, so the green agents are reachable only through persisted node capabilities. That last detail is the case that breaks.

Verified both ways on a real Postgres:

main:        the_bumped_version_is_assigned_to_the_fleet_that_declares_it  FAIL
             event-subscriptions://marten/main/localhost.bgtenant3/trip/all/v3
             may only run on a node that declares it
with the fix: Passed: 2

It also pins down the premise the whole rollout rests on, which nothing covered before: the two fleets advertise the bumped projection under disjoint identities and share the agents of the projection that was not bumped. That one passes on main too — it is the thing that makes the fix meaningful rather than the thing the fix changes.

Two notes for review:

  • The pool bound stays in the unit test, deliberately. I tried asserting "exactly one owner per version per database" here too and it fails for a legitimate reason: with three databases over four nodes the per-node ceiling binds before the sibling preference does, and spreading a group's third partition to balance load is the intended trade. The bound only holds where agents outnumber nodes the way they do in a real cluster, which the unit test models and this one cannot.
  • Most of the diff in AssignmentGrid.Distribution.cs is indentation. The body moved inside a foreach (var members in partitions), so ~120 of those lines are whitespace-only. git diff -w main is 3 files / ~90 real lines if that is easier to read.

House style for private methods and local functions in this code — canSelfHeal in
NodeAgentController, and countOn a few lines up in this same file. Copilot caught
the static method; the local function had the same problem.
The first version used AddSingleTenantDatabase — one tenant per database — which
is not the shape this bug lives in. With one tenant per database a group is two
or three agents deep, so it exercises the version split but not the thing group
affinity exists for: several tenants sharing a shard database, with events
partitioned per tenant, which is what makes IEventStore.DistributesAgentsPerTenant
true and produces an agent per (database, tenant, projection version).

Now three databases carry three tenants each with Conjoined event tenancy and
UseTenantPartitionedEvents, so every database's group is nine agents across three
capability classes instead of three. The failing assertion on main now reads

  event-subscriptions://marten/main/localhost.bgshard1/trip/all/v3/bgshard1-alpha
  may only run on a node that declares it

which is the real URI grammar down to the tenant segment.

Also asserts the shape itself. Without that, a change that stopped distributing
per tenant would shrink every group back to one agent per projection and this file
would keep passing while testing something much weaker.
@erdtsieck

Copy link
Copy Markdown
Contributor Author

Two updates, one of them a correction to something I claimed above.

The end-to-end test now models the shape the bug lives in

My first version used AddSingleTenantDatabase — one tenant per database — which exercises the version split but not what group affinity is actually for. It now uses three databases carrying three tenants each, with Events.TenancyStyle = Conjoined and UseTenantPartitionedEvents = true, so IEventStore.DistributesAgentsPerTenant is on and there is an agent per (database, tenant, projection version). Each database's group is nine agents across three capability classes instead of three, and the failure on main now reads with the full grammar:

event-subscriptions://marten/main/localhost.bgshard1/trip/all/v3/bgshard1-alpha
may only run on a node that declares it

It also asserts the shape itself, because without that a change which stopped distributing per tenant would shrink every group back to one agent per projection and the file would keep passing while testing something much weaker.

Correction: the bound is one host per capability class, not two

I wrote above that "at 512 databases, two owners per database is affordable and three is not". The second half is a fair description of our connection budget; the first half overstated what this PR guarantees.

What the partitioning actually bounds is one host per distinct capability set in the group — during a single-version bump that is three (previous-version-only, new-version-only, and the projections that did not change, which every node declares). The sibling preference collapses that to two whenever the shared partition can join one of the other two, but it is bounded by the same per-node ceiling the incumbent rule respects, so under load it legitimately spills to a third node to keep the distribution balanced. Two is the common case, three is the guarantee.

That matters for how the fix should be read: the value is not "exactly two" but that the count is bounded by capability classes at all. The per-member fallback — the other way to fix the assignment — bounds it by nothing: it would place each agent individually, so a database's nine agents could land on nine nodes and each one opens a pool on that shard.

each_database_keeps_one_owner_per_version_and_no_more is therefore deliberately not in the end-to-end test; the exact-two assertion in distribute_by_group_affinity holds for that test's fixed arithmetic, where the ceiling does not bind. If you would rather see the general bound asserted somewhere, "at most one host per capability class" is the honest form and I am happy to add it.

@jeremydmiller
jeremydmiller merged commit 776b956 into JasperFx:main Aug 3, 2026
35 of 36 checks passed
jeremydmiller added a commit that referenced this pull request Aug 3, 2026
…ion-pins

Pin the group-affinity invariants GH-3792 relies on, and simulate a version bump with slow starts
erdtsieck pushed a commit to erdtsieck/wolverine that referenced this pull request Aug 4, 2026
…late a version bump with slow starts

Follow-up test coverage for JasperFxGH-3792 (blue/green group affinity), all four verified to
fail against the pre-3792 implementation:

- a_settled_blue_green_split_does_not_churn_on_the_next_evaluation: the settled split
  state is a fixed point of the placement. JasperFxGH-3785 counted ~45,000 ReassignAgent
  decisions during a rollout ramp, and the partition incumbent rule
  (members[0].AssignedNode, all members on one node) is subtle enough to regress
  silently.
- a_rolling_restart_keeps_every_database_on_at_most_two_hosts: pins the invariant that
  makes the strict partition key (exact declaring node-id set) safe to keep or later
  relax -- overlapping-but-unequal capability sets still cost no third pool set.
- asymmetric_fleets_strand_nothing_and_load_every_capable_node: the per-node ceiling is
  computed over ALL nodes including incapable ones, so a brand-new green fleet that
  declares only the bumped agents is where the arithmetic would break first.
- a_version_bump_converges_with_slow_starts_and_no_cross_fleet_placement: the JasperFxGH-3753
  deploy shape end to end. JasperFxGH-3753 is not slow starts in general -- production converges
  without a version bump -- it is slow starts AND a capability split at once, and until
  now each condition was covered by tests that passed while their intersection failed.
  Drives the real leader evaluation with DistributeByGroupAffinity, a blue leader whose
  family cannot enumerate the green agents, long-tailed start costs, and asserts
  convergence, zero cross-fleet placement, zero churn, and the two-hosts-per-database
  pool bound throughout.

Harness seams, added without touching existing behavior: FakeAgentFamily takes explicit
agent names and an overridable Distribution; SlowStartCluster takes a leader family and
per-node capability sets, deriving the agent universe from the capability union exactly
as NodeAgentController seeds the real grid.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA
erdtsieck pushed a commit to erdtsieck/wolverine that referenced this pull request Aug 4, 2026
…es (JasperFxGH-3785)

Agent assignment honoured database affinity WITHIN the event-subscriptions family
(JasperFxGH-3792 / marten#4806) and within the durability family, but not BETWEEN them: a shard
database's durability agent was distributed independently of that database's projection
agents. Measured on a 512-shard production cluster, 73% of databases ended up with
durability and projections on different nodes -- ~425 connections held by durability
owners against databases they otherwise never open, on a server that had just spent half
an hour at 2,393 of 2,400 max_connections.

The fix, in three parts:

- AssignmentGrid.DistributeEvenlyWithAffinity: an even distribution where any agent with
  a preferred node goes there regardless of the even spread. Preferred placements are
  deliberately not ceiling-bounded (they piggyback on the other family's own balanced
  distribution); the remainder spreads evenly counting only itself, so no-affinity
  agents don't crowd onto whichever nodes hold no projections.
- DurabilityProjectionAffinity: joins a wolverinedb:// agent URI to the node owning that
  database's event-subscriptions agents in the current pass. The two families describe
  the same physical database through different pipelines, so the join keys on the
  database NAME when unambiguous and only falls back to comparing normalized server
  spellings when two servers carry the same name. A miss is never wrong, only
  not-better: the agent falls back to today's even spread. During a blue/green split
  the durability agent follows the larger side, deterministically.
- NodeAgentController now explicitly orders the durability family last in the
  evaluation loop -- previously true only by Dictionary insertion-order accident, and
  the affinity only works if the event-subscription assignments are already in the
  shared grid.

An agent running away from its preferred node is MOVED (a normal ReassignAgent), which
is the one-time migration that converges an existing cluster; the settled co-located
state is a fixed point, pinned by test.

The MartenTests integration test runs both REAL URI pipelines (Marten database
descriptors on one side, Weasel-described Postgres message stores on the other) against
the same three tenant databases, because a spelling divergence between them makes the
join silently never engage -- which looks exactly like the feature working, minus the
benefit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0116vfBcKwcjWn8msM4ZjkuA
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.

3 participants