Skip to content

Commit b86a6a1

Browse files
Akka.Cluster: gossip removal tombstones - stop resurrecting removed members (#8484)
* Akka.Cluster: add removal tombstones to Gossip and consult them when merging A member status on its own cannot tell "this member was removed" apart from "this node has not heard about it yet". Merge had only the status to go on, so a member the leader had removed was put back whenever a lagging peer still held it as Leaving. The resurrected member is dead, so it can never mark itself seen, and convergence stops for good. Gossip now carries a tombstone per removed node - keyed by UniqueAddress, stamped with the epoch milliseconds of the removal. That is positive evidence a removal happened, and it travels with the gossip so any peer can apply it. - Gossip.RemoveAll strips a node from members, seen, reachability and the vector clock and records its tombstone in one step, so no gossip that breaks the invariants is ever built. - Gossip.Merge unions tombstones first, then prunes the merged vector clock by them. Union alone is not enough: a clock entry for a removed node is resurrected by the merge exactly the way the member is. - Gossip.MergeTombstones unions tombstones without touching anything else, for the gossip-reception branches that pick a whole gossip as the winner. - Gossip.PruneTombstones drops expired entries and returns the same instance when it drops nothing, so a caller can skip the update by reference check. - Member.PickHighestPriority gains a tombstone-aware overload. The one-sided drop condition is widened with OR, not replaced: every member dropped before is still dropped, and no input keeps a member the old code dropped. - AssertInvariants now requires members and tombstones to be disjoint. Merge picks a two-sided member by status alone, which is only correct while that holds. Timestamps order nothing; they only decide when a tombstone expires. Union keeps the later timestamp on collision, so two nodes merging the same pair of gossips in opposite order reach the same state. * Akka.Cluster: put gossip tombstones on the wire Adds `repeated Tombstone tombstones = 7` to message Gossip, plus the Tombstone message itself. Field 7 was the one vacant slot in Gossip - allAppVersions already sits at 8 - so nothing needs renumbering. Codegen runs at build time; there is no checked-in generated file to update. The serializer builds its address table from members only, and a tombstoned node is by definition not a member. Tombstone addresses are appended to that table after the member loop, so a tombstone can index into it like everything else. Getting this wrong is silent: it produces tombstones pointing at other nodes' addresses rather than an error. Gossip written before this field decodes to an empty tombstone set, which reduces the merge to its previous status-only rule. * Akka.Cluster: write tombstones on removal and expire them on the leader The leader records a tombstone for every node it removes, on both removal paths - unreachable Down/Exiting members and confirmed-Exiting members. Both already ran through the same block in LeaderActionsOnConvergence; that block now calls Gossip.RemoveAll instead of deriving members, seen, reachability and the vector clock separately. ReceiveGossip unions tombstones on all four comparison branches. Only the concurrent branch merges; the other three pick one whole gossip as the winner and would otherwise drop the loser's tombstones, letting removals decay out of the cluster over time. That failure mode presents as the original bug returning intermittently, which no unit test would catch. Expired tombstones are dropped on the leader at the end of the same method. Pruning has to run on converged ticks where nothing else changed, so it sits outside the change guard: the method now computes the updated gossip - or the local one when nothing changed - prunes it, and publishes only when the result differs from the local gossip by reference. PruneTombstones returns the same instance when it drops nothing, which is what makes that check work. Without it every converged leader tick would bump the vector clock and reset the seen table, and the cluster would never sit still. Retention is akka.cluster.prune-gossip-tombstones-after, default 24h. The bound that matters is how long a gossip carrying the stale member can survive before it merges back in, and under a partition that is unbounded - so the value is set against partition length, not bandwidth. A partition outlasting the window re-opens the hole silently. UpdateLatestGossip now records the invariant it carries: every change this node makes to the gossip goes through it and bumps the vector clock, which is what lets peers tell whose removal set is newer. * Akka.Cluster: tests for gossip removal tombstones GossipSpec - The removal-resurrection case built straight from Gossip values: the leader has removed a member and holds its tombstone, a lagging peer still holds it as Leaving. The merge must not put it back, in either direction. - The same setup without the tombstone, asserting the member IS kept. That case both proves the test above bites and pins the correct behaviour: with no evidence of a removal, a one-sided Leaving member may be a node the other side has not heard about yet, and dropping it would strand a live process. - Union is commutative, and keeps the later timestamp on collision. - The merged vector clock is pruned for every tombstoned node. Each side carries a clock entry for both removed nodes and a tombstone the other lacks; with only the member filter in place, this is the single test that fails. - One-sided Down and Exiting members are still dropped with no tombstones present, so the OR did not turn into a replacement. - A new incarnation at the same host and port survives its predecessor's tombstone. - RemoveAll strips members, seen, reachability as observer and as subject, and the vector clock entry, while leaving other nodes' clock entries alone. - Pruning boundary, and the same-instance identity that the leader's no-op check depends on. - The three non-merge gossip comparison branches each keep both sides' tombstones, and the union refuses to adopt a tombstone for a node it still holds as a member. ClusterMessageSerializerSpec - Round trip with tombstones, over GossipEnvelope and over Welcome. - A tombstoned address that shares a host and port with a member survives with its own UID, which is what catches a serializer resolving tombstones through the member address table. That bug is silent - it yields the wrong address, not an error. - A gossip proto with field 7 cleared decodes to an empty tombstone set with members and version intact. ClusterConfigSpec asserts the 24h default. The API approval files gain the two new public members: the PickHighestPriority overload and ClusterSettings .PruneGossipTombstonesAfter. Also fixes Gossip.Prune, which rebuilt the gossip through the constructor and so dropped tombstones on the pre-merge clock pruning path. * Record gossip removal tombstones in the v1.6 breaking changes ledger Wire addition to cluster gossip, the new retention setting, and the behavior change: a removed member can no longer be resurrected by stale gossip. * Akka.Cluster: property-based specs for gossip removal tombstones The example specs pin the cases a human thought of. These sample the same code over a few hundred to a few thousand random gossips per property and check laws: merge is commutative, idempotent and associative; a tombstone always beats a stale member; a removal never comes back over a random sequence of exchanges. CsCheck 4.8.0, referenced from Akka.Cluster.Tests only. Fixed iteration counts, so the class runs in about five seconds. A failure prints the seed to replay. Generators draw from a bounded universe of six nodes, two of which share a host and port and differ only by UID - the case that catches a serializer resolving a tombstone through the member address table. Member statuses honour the allowed transition table, tombstone timestamps come from a four value pool so collisions are frequent, and every timestamp is passed in rather than read off the clock. P1-P3 merge is commutative, idempotent and associative over members, tombstones with their timestamps, reachability and the vector clock. P4-P6 a tombstoned node loses; a one-sided live member with no tombstone is kept; a one-sided Down or Exiting member is still dropped. P7 disjointness, split by reception branch: the concurrent path lets the tombstone win, the winner-picked paths let the winner's member win. P8 no tombstoned node keeps a vector clock entry through a merge. P9 merged tombstones are the union, collisions take the later timestamp. P10 PruneTombstones drops what expired and hands back the same instance otherwise, which the leader's no-op check depends on. P11 RemoveAll writes a tombstone for every removal and strips it elsewhere. P12 proto round trip preserves members, tombstones, reachability and clock. P13-P14 random histories over three to five nodes exchanging gossip through the same branch selection ReceiveGossip uses, checked against a plain set of removed UIDs. Every node converges on that set, and no node ever loses a tombstone outside a prune. Three properties carry a documented caveat where the law they check is narrower than it first looks. Merge with itself also drops the clock entries of tombstoned nodes, so P2 compares against that form. The one-sided drop for Down and Exiting is not associative on its own, predating tombstones, so P3 draws non-terminal statuses and lets tombstones carry the removals. Reachability merge breaks an equal-version tie by argument order, so the two sides observe from disjoint observer sets. Each property counts the iterations that hit the case it is about and fails if that count is too low, so none of them can pass by never generating anything interesting. * Akka.Cluster: keep only the winner's tombstones on the causally-ordered branches ReceiveGossip unioned the receiver's tombstones into the winning gossip on all four comparison branches, while PruneTombstones is called from one place - the leader's LeaderActionsOnConvergence. A prune was therefore undone by the next exchange with any peer, because peers never prune and always re-unioned. So prune-gossip-tombstones-after reclaimed nothing, tombstones rode every gossip message forever, and once one was past the window the leader looped on every tick: prune, clock bump, seen reset, re-converge, peers hand it back, prune again. That is exactly the never-settles failure the reference-equality guard in PruneTombstones exists to prevent. Tombstones are now unioned only where neither gossip descends from the other - the equal-clock branch, and the concurrent branch inside Gossip.Merge. The two branches that pick a strictly newer gossip keep the winner's tombstones and nothing else. The winner is not behind on removals: a removal bumps the removing node's own clock entry, so a gossip that dominates the loser's clock descends from every removal the loser knows about. The tombstones it does not carry are the ones its own leader pruned, and honouring those prunes is the point. The branch selection moved out of ReceiveGossip into ClusterCoreDaemon.SelectWinningGossip, unchanged apart from the two lines above. The property specs now call it instead of re-implementing it - they had their own copy of the branches, so deleting the production change left them all green. Three new properties: P15 a tombstone pruned on a converged tick does not come back. The pruning node stamps the prune the way UpdateLatestGossip does, which puts it strictly ahead of every peer, so every exchange afterwards takes a winner-picked branch - the branch that used to hand the tombstones straight back. P16 a simulated week of virtual time: sparse jumps forward, a constant per-node clock offset up to five minutes, and a prune tick run by a different drawn node each time, measured against the shipped prune-gossip-tombstones-after default rather than a cutoff invented for the test. Expired tombstones are reclaimed and stay reclaimed, and no tombstone is dropped sooner than the window minus the widest clock disagreement. P17 a removal holds when the node that recorded it is itself removed later. That is where Merge prunes the clock entry that recorded the removal, so it is the one place a gossip could come out strictly newer than a tombstone carrier without having descended from the removal. P15 and P16 both fail if the union is restored on the Before and After branches. P17 documents a resurrection the model can reach and a cluster cannot, because removing a member is a leader action and the leader needs convergence first; the same history resurrects the member with the union restored, so it is not something the union was buying. Also in the suite: the seen table joins the equivalence helper; reachability is built from one shared op history per observer with each side replaying a prefix of it, so sides share observers at different versions and Reachability.Merge's version arbitration is actually exercised; histories gained a status op so peers hold a victim at mixed statuses while its tombstone propagates; every op asserts the gossip carries no clock entry for a non-member, mirroring the "too many vector clock entries" check; P4, P7c and P11 assert no reachability record survives whose subject was removed; the serializer property gained a sample where a tombstone key is also a member, which is the arm of the address-table writer nothing else reached; coverage guards count iterations rather than occurrences; and the properties whose inputs the Gossip constructor rejects under AKKA_CLUSTER_ASSERT=on skip there instead of failing. * ci: retrigger after superseded-run cancellations * Akka.Cluster: fold tombstones into the Gossip constructor and document the file Addresses two review comments on #8484. Drop the four-argument Gossip constructor added by the tombstone work. Gossip is internal, so there is no API risk in changing the existing three-argument constructor instead: tombstones becomes an optional parameter that defaults to no tombstones. Every existing call site compiles unchanged, including the ones that pass tombstones positionally. Replace all 62 TBD placeholders in Gossip.cs with docs that say what each member actually does. The delegating constructors carry a one-line summary noting their defaults and inherit the rest from the primary. Notable details now written down: GetMember hands back a Removed placeholder for a node that is not in the ring, AddMember is keyed by address so it will not replace a member that differs only in status, YoungestMember treats a member that is not up yet as up-number zero, and Prune clears a node's vector clock entry without touching members or tombstones.
1 parent 60a01d0 commit b86a6a1

18 files changed

Lines changed: 2854 additions & 143 deletions

BREAKING_CHANGES_V1.6.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ of `Behavior`, `Wire`, `API` (combine with `+`).
4141
| Status | PR / Branch | Component | Type | Change | Migration |
4242
|--------|-------------|-----------|------|--------|-----------|
4343
| Planned | [#8324](https://github.com/akkadotnet/akka.net/pull/8324) | `Akka.Routing` | Behavior | `ConsistentHash<T>` no longer retains the `SortedDictionary` passed to its public constructor — the ring is snapshotted into internal sorted arrays. Mutating that dictionary after construction no longer affects the instance (previously the aliasing was inconsistent: `IsEmpty` and `operator +`/`-` read it live, while `NodeFor` froze it after the first lookup). A `null` dictionary now throws `ArgumentNullException` from the constructor instead of surfacing later as a `NullReferenceException`. No public API removed; the ring built by `ConsistentHash.Create` is byte-identical. (#8293) | None for normal use — `ConsistentHash.Create` already builds the dictionary fully before constructing, so routers/receptionists are unaffected. If you call the `ConsistentHash(SortedDictionary, int)` constructor directly, populate the dictionary before passing it and don't rely on post-construction mutation being visible. |
44+
| Planned | `fix/gossip-removal-tombstones` | `Akka.Cluster` | Wire + Behavior | Cluster gossip now carries a removal tombstone for every member the leader removes (new `repeated Tombstone tombstones = 7` on the `Gossip` proto message, previously the one vacant field number). A member the leader removed can no longer be put back into the gossip by a peer that has not caught up, which previously blocked convergence permanently and needed a full cluster restart to clear. Tombstones expire after the new `akka.cluster.prune-gossip-tombstones-after` setting (default `24h`), pruned by the leader on a converged tick and reclaimed for good: gossip reception keeps the tombstones of whichever gossip wins the causal comparison, so a peer that has not pruned yet cannot hand a pruned tombstone back. Tombstones are only unioned across the two gossips when neither descends from the other -- equal clocks, or concurrent clocks, where each side may hold a removal the other has not heard about. The field is additive and proto3 ignores unknown fields in both directions, so rolling upgrades are safe; an older node parses the gossip fine but drops the tombstones when it re-emits it, and if that stripped gossip is causally newer than an upgraded node's, the upgraded node adopts it and loses those tombstones too -- so the protection only holds between upgraded nodes and the fix is not fully in effect until every node is upgraded. | No action required. Raise `akka.cluster.prune-gossip-tombstones-after` if the cluster must survive partitions longer than 24 hours, or lower it if a cluster with heavy join/leave churn needs the gossip message kept small -- each tombstone adds a full address plus a timestamp to every gossip message until it expires. |
4445
| Planned | `feature/default-bounded-shard-rebalancing` | `Akka.Cluster.Sharding` | Behavior | The default `rebalance-absolute-limit` is now `20`, selecting the bounded shard allocation strategy instead of the legacy threshold-based strategy. | To retain the legacy strategy temporarily, explicitly set `akka.cluster.sharding.least-shard-allocation-strategy.rebalance-absolute-limit = 0`. Review `rebalance-threshold` and `max-simultaneous-rebalance`, which do not apply while the bounded strategy is active. |
4546
| Planned | `fix/artery-inbound-quarantine-check` | `Akka.Remote` (Artery) | Behavior | Quarantine is now enforced on the INBOUND path too. Previously Artery only gated outbound sends -- an envelope arriving FROM a uid this system has quarantined was still delivered, and the quarantined peer was only notified once, proactively, at the moment `Quarantine()` was called. A new `InboundQuarantineCheckStage`, woven into the inbound pipeline right after handshake, now drops every inbound envelope (ordinary or control, including system messages) whose origin uid is quarantined, and reactively re-sends a `Quarantined` control notice to the origin for each drop (except for a heartbeat or the peer's own `Quarantined` notice, to avoid a reply storm). The existing one-shot proactive notice in `Quarantine()` is unchanged. Additionally, an ordinary/large outbound stream that terminates while its association is quarantined no longer wedges permanently: the materialize-once gate is released (timer-driven auto-reconnect stays suppressed), so a quarantine-piercing `ActorSelection` send -- or any send after a new incarnation's handshake lifts the quarantine -- re-materializes the stream on demand and can reach a restarted peer at the same address. | No action required -- this is a bug fix restoring the documented "no further communication" guarantee of quarantine and the documented new-incarnation piercing behavior; code that (incorrectly) depended on a quarantined peer's replies still arriving is unsupported. |
4647
| Planned | `fix/artery-daemonmsgcreate-control-stream` | `Akka.Remote` (Artery) | Behavior | Remote deployment's `DaemonMsgCreate` now travels over Artery's CONTROL stream (as a plain envelope, no delivery/ack sequencing) instead of the ordinary stream, ordering it ahead of the `Watch` that remote deployment sends immediately afterwards. Previously the two rode independent, unordered TCP connections and `Watch` systematically arrived first, so the receiver replied `DeathWatchNotification(existenceConfirmed: false)` for a not-yet-created actor and the deployer reaped the freshly-deployed routee before its `Supervise` registration landed, emptying cluster router pools. Additionally (Pekko parity), inbound ordinary messages addressed to a remote-deployed recipient that has not been created yet are no longer dead-lettered immediately: the resolve is retried on a bounded schedule (20 attempts x 50ms, buffered per recipient in FIFO order) so first messages that arrive ahead of the in-flight `DaemonMsgCreate` are delivered once the actor exists; paths that never resolve are banned and dead-letter as before. | No action required -- this is a bug fix restoring correct create-before-watch ordering; code that (incorrectly) depended on the old race is unsupported. |

Directory.Build.props

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
<ProduceReferenceAssembly>true</ProduceReferenceAssembly>
4242
<FsCheckVersion>3.3.3</FsCheckVersion>
4343
<FsCheck3Version>3.3.3</FsCheck3Version>
44+
<CsCheckVersion>4.8.0</CsCheckVersion>
4445
<HoconVersion>2.0.3</HoconVersion>
4546
<ConfigurationManagerVersion>6.0.1</ConfigurationManagerVersion>
4647
<MicrosoftLibVersion>[6.0.*,)</MicrosoftLibVersion>

src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCluster.DotNet.verified.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ namespace Akka.Cluster
213213
public System.Collections.Immutable.ImmutableDictionary<string, int> MinNrOfMembersOfRole { get; }
214214
public int MonitoredByNrOfMembers { get; }
215215
public System.TimeSpan PeriodicTasksInitialDelay { get; }
216+
public System.TimeSpan PruneGossipTombstonesAfter { get; }
216217
public System.TimeSpan? PublishStatsInterval { get; }
217218
public int ReduceGossipDifferentViewProbability { get; }
218219
public System.TimeSpan? RetryUnsuccessfulJoinAfter { get; }
@@ -257,6 +258,7 @@ namespace Akka.Cluster
257258
public override string ToString() { }
258259
public static Akka.Cluster.Member HighestPriorityOf(Akka.Cluster.Member m1, Akka.Cluster.Member m2) { }
259260
public static System.Collections.Immutable.ImmutableHashSet<Akka.Cluster.Member> PickHighestPriority(System.Collections.Generic.IEnumerable<Akka.Cluster.Member> a, System.Collections.Generic.IEnumerable<Akka.Cluster.Member> b) { }
261+
public static System.Collections.Immutable.ImmutableHashSet<Akka.Cluster.Member> PickHighestPriority(System.Collections.Generic.IEnumerable<Akka.Cluster.Member> a, System.Collections.Generic.IEnumerable<Akka.Cluster.Member> b, System.Collections.Immutable.IImmutableSet<Akka.Cluster.UniqueAddress> tombstones) { }
260262
public static Akka.Cluster.Member PickNextTransition(Akka.Cluster.Member a, Akka.Cluster.Member b) { }
261263
public static System.Collections.Immutable.ImmutableSortedSet<Akka.Cluster.Member> PickNextTransition(System.Collections.Generic.IEnumerable<Akka.Cluster.Member> a, System.Collections.Generic.IEnumerable<Akka.Cluster.Member> b) { }
262264
}

src/core/Akka.API.Tests/verify/CoreAPISpec.ApproveCluster.Net.verified.txt

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,7 @@ namespace Akka.Cluster
213213
public System.Collections.Immutable.ImmutableDictionary<string, int> MinNrOfMembersOfRole { get; }
214214
public int MonitoredByNrOfMembers { get; }
215215
public System.TimeSpan PeriodicTasksInitialDelay { get; }
216+
public System.TimeSpan PruneGossipTombstonesAfter { get; }
216217
public System.TimeSpan? PublishStatsInterval { get; }
217218
public int ReduceGossipDifferentViewProbability { get; }
218219
public System.TimeSpan? RetryUnsuccessfulJoinAfter { get; }
@@ -257,6 +258,7 @@ namespace Akka.Cluster
257258
public override string ToString() { }
258259
public static Akka.Cluster.Member HighestPriorityOf(Akka.Cluster.Member m1, Akka.Cluster.Member m2) { }
259260
public static System.Collections.Immutable.ImmutableHashSet<Akka.Cluster.Member> PickHighestPriority(System.Collections.Generic.IEnumerable<Akka.Cluster.Member> a, System.Collections.Generic.IEnumerable<Akka.Cluster.Member> b) { }
261+
public static System.Collections.Immutable.ImmutableHashSet<Akka.Cluster.Member> PickHighestPriority(System.Collections.Generic.IEnumerable<Akka.Cluster.Member> a, System.Collections.Generic.IEnumerable<Akka.Cluster.Member> b, System.Collections.Immutable.IImmutableSet<Akka.Cluster.UniqueAddress> tombstones) { }
260262
public static Akka.Cluster.Member PickNextTransition(Akka.Cluster.Member a, Akka.Cluster.Member b) { }
261263
public static System.Collections.Immutable.ImmutableSortedSet<Akka.Cluster.Member> PickNextTransition(System.Collections.Generic.IEnumerable<Akka.Cluster.Member> a, System.Collections.Generic.IEnumerable<Akka.Cluster.Member> b) { }
262264
}

src/core/Akka.Cluster.Tests/Akka.Cluster.Tests.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
<PackageReference Include="xunit.v3" Version="$(Xunit3Version)" />
1818
<PackageReference Include="xunit.runner.visualstudio" Version="$(Xunit3RunnerVersion)" />
1919
<PackageReference Include="FsCheck.Xunit.v3" Version="$(FsCheck3Version)" />
20+
<PackageReference Include="CsCheck" Version="$(CsCheckVersion)" />
2021
<PackageReference Include="FluentAssertions" Version="$(FluentAssertionsVersion)" />
2122
<PackageReference Include="Fsharp.Core" Version="$(FsharpVersion)" />
2223
</ItemGroup>

src/core/Akka.Cluster.Tests/ClusterConfigSpec.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public void Clustering_must_be_able_to_parse_generic_cluster_config_elements()
3737
settings.PeriodicTasksInitialDelay.Should().Be(1.Seconds());
3838
settings.GossipInterval.Should().Be(1.Seconds());
3939
settings.GossipTimeToLive.Should().Be(2.Seconds());
40+
settings.PruneGossipTombstonesAfter.Should().Be(TimeSpan.FromHours(24));
4041
settings.HeartbeatInterval.Should().Be(1.Seconds());
4142
settings.MonitoredByNrOfMembers.Should().Be(9);
4243
settings.HeartbeatExpectedResponseAfter.Should().Be(1.Seconds());

0 commit comments

Comments
 (0)