Forward-port #8294 to dev: consistent-hashing router 32-bit collision fix (#8031) - #8305
Merged
Aaronontheweb merged 3 commits intoJul 3, 2026
Conversation
… 32-bit hash collision (akkadotnet#8294) * Fix akkadotnet#8031: consistent-hashing router wedges cluster-wide on 32-bit hash collision ConsistentHash.Create now linear-probes to the next free slot when two virtual nodes collide in the 32-bit ring, instead of letting SortedDictionary.Add throw. The throw was swallowed by ConsistentHashingRoutingLogic.Select and returned NoRoutee for every message until a manual restart (and crashed the unguarded ClusterReceptionist). The ring is now built in canonical node order so every node in the cluster produces an identical ring even when a collision is resolved. For any routee set without a collision the ring is byte-identical to prior versions (safe for rolling upgrades) - proven by ConsistentHashSpec. operator + is hardened the same way. Adds ConsistentHashSpec (collision tolerance, distribution-neutrality, cross-node determinism, and a byte-identical before/after proof), a router-level no-wedge test, and a Create scaling benchmark. Perf follow-up: akkadotnet#8293. * Address xhigh review: make ConsistentHash +/- consistent with Create The akkadotnet#8031 fix made Create and operator+ linear-probe past 32-bit collisions, but left operator- computing only natural vnode keys — so it could not remove a vnode that had been relocated to a probed slot, leaving a phantom entry that still routed to the removed node. operator+ also silently duplicated an already-present node's vnodes and resolved collisions in insertion order rather than Create's canonical order (so incremental rings could diverge from Create). Rewrite operator+/- to rebuild deterministically via Create, so `Create(S) + x == Create(S ∪ {x})` and `Create(S) - x == Create(S \ {x})` hold by construction: symmetric, canonical-order collision resolution, idempotent add, and removal that drops probed slots. Drops the now-unused SortedDictionary CopyAndAdd/CopyAndRemove path and duplicated probe loop. Adds regression tests: add==Create-across-collision, idempotent add, and remove-drops-probed-slots. * Address re-review: unify ConsistentHash node identity on ToString() The prior review-fix used EqualityComparer<T>.Default in operator +/- but the ring identifies nodes by ToString() (the value its keys are derived from; the class contract requires ToString to be distinct per node). That mismatch left three confirmed issues: - Create did not de-duplicate, so a node supplied twice was probed into a second vnode set (distribution skew); the dedup guard was only on +/-. - operator+ idempotency relied on Distinct()'s reference equality, so re-adding a fresh-but-equal reference-type node duplicated its vnodes unbounded. - operator- removed by EqualityComparer<T>.Default, so a T whose Equals is broader than ToString could over-remove a different node. Unify identity on ToString(): Create now de-duplicates input by ToString (and +/- inherit it by delegating to Create); operator- matches the removed node by ToString rather than T.Equals. Adds tests for dedup, ToString-based idempotent add, and ToString-based removal using a reference type without an Equals override. * Address 3rd review: full-width collision relocation + cheaper +/- rebuild Two follow-ups from the third code-review pass on the akkadotnet#8031 fix: - Distribution (finding #2): the key+1 linear probe placed a relocated colliding virtual node on a near-zero-width ring segment, so a collided node lost ~1/factor of its traffic - the "distribution unchanged" claim was false. Re-hash the loser to a well-distributed slot (full-width segment) instead, preserving the node's ring share, then linear-probe from there to guarantee termination. Non-colliding builds are unchanged (probe never fires); the sequence is a pure function of the node hash so every node still builds an identical ring. - Perf (finding #3): operator +/- passed _nodes.Values (N*virtualNodesFactor entries) to Create, so it sorted/ToString'd N*V items per membership change. Distinct() the same-reference repeats down to N first; Create's ToString de-dup remains the correctness guarantee. Not changed: NullReferenceException on a null ToString() (finding #1) is pre-existing (old Create hashed node.ToString() identically), unreachable from the router (ConsistentRoutee.ToString is never null), and outside the akkadotnet#8031 scope. (cherry picked from commit cdec84e)
Aaronontheweb
enabled auto-merge (squash)
July 2, 2026 22:35
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.
Forward-port of #8294 to
devThis is the forward-port to
devof the consistent-hashing router collision fix that was merged tov1.5in #8294 (fixes #8031).devcarried the identical pre-fixConsistentHash.cs, so without this it would regress the fix the next timev1.5merges up.Fixes #8031.
The fix
When two virtual nodes collided in the 32-bit consistent-hash ring (increasingly likely at high routee counts, e.g. when the ring is rebuilt after a node is downed),
ConsistentHash.Createthrew"An entry with the same key already exists". The consistent-hashing router swallowed the exception and returnedNoRouteefor every subsequent message until a manual restart — and crashed the unguardedClusterReceptionist, which builds the same ring.src/core/Akka/Routing/ConsistentHash.csnow:ConsistentHash.Create<T>— de-duplicates input nodes byToString()(aHashSet<string>withStringComparer.Ordinal), builds the ring in canonicalOrderBy(ToString, Ordinal)order, and on a 32-bit ring-key collision re-hashes the loser to a full-width, well-distributed slot (ConcatenateNodeHash(nodeHash, vnode + virtualNodesFactor)) then linear-probes to the next free slot (guaranteed termination) — instead of the oldSortedDictionary.Addthrow. This preserves the collided node's ring share (distribution unchanged) and, when no collision occurs, produces a byte-identical ring to prior versions (safe for rolling upgrades).operator +/operator -— rebuild deterministically viaCreate, so node identity isToString-based andoperator -filters survivors byToString. This makesCreate(S) + x == Create(S ∪ {x})andCreate(S) - x == Create(S \ {x})hold by construction (idempotent add; removal drops relocated/probed slots).No public API change
No public API or wire-format change — this only defines behavior for the previously-throwing collision case.
Akka.API.Testspasses unchanged with no approved-file regeneration. NoBREAKING_CHANGES_V1.6.mdentry is needed.How it was applied
Cherry-picked the squashed
v1.5merge commit (cdec84e00) with-x. The only conflict wasRELEASE_NOTES.md(thev1.51.5.70-beta3heading does not apply todev); resolved by adding a dev-appropriate bullet under the existing1.6.0unreleased section. All four code/test/benchmark files applied cleanly and are byte-identical to thev1.5-merged versions.Validation (net10.0, Linux)
dotnet build src/core/Akka/Akka.csproj -c Release -warnaserror-> 0 warnings, 0 errorsdotnet test src/core/Akka.Tests --framework net10.0 --filter "FullyQualifiedName~ConsistentHash"-> 21 passed, 0 failed (newConsistentHashSpec+ router no-wedge test)dotnet test src/core/Akka.API.Tests --framework net10.0-> 18 passed, 0 failed, working tree clean (no public API delta)Changes brought over
src/core/Akka/Routing/ConsistentHash.cs— the fixsrc/core/Akka.Tests/Routing/ConsistentHashSpec.cs— new (collision tolerance, distribution-neutrality, cross-node determinism, byte-identical before/after proof, add/remove == Create)src/core/Akka.Tests/Routing/ConsistentHashingRouterSpec.cs— addedNamedRoutee+ router-level no-wedge testsrc/benchmark/Akka.Benchmarks/Utils/ConsistentHashBenchmarks.cs— addedConsistentHashCreateBenchmarksRELEASE_NOTES.md— dev-appropriate entry under1.6.0Related: forward-port of #8294; perf follow-up #8293.