diff --git a/docs/Redundancy.md b/docs/Redundancy.md index fcd6ca7f..f5831ebd 100644 --- a/docs/Redundancy.md +++ b/docs/Redundancy.md @@ -314,7 +314,21 @@ operational unattended. | `"00:00:10"` (**default**) | A node with no membership after 10 s self-forms. A live peer answers `InitJoin` in milliseconds, so on any normal boot the fallback never fires. | | `null` / `≤ 0` | Disabled — the pre-2026-07-22 behaviour: wait on `InitJoin` indefinitely. | -**The island guard is the load-bearing part.** The fallback fires **only when this node's own address +**Two guards keep the fallback from creating the split it exists to avoid.** + +**1 — Reachability.** An expired window is not evidence the peer is down, so before self-forming the +fallback TCP-probes the other seed addresses; if any accepts a connection it waits another window instead. +This was added after the live gate below caught the fallback islanding a node: a node bounced by a manual +failover restarted, received `InitJoinAck` from its live peer — a join in flight and healthy — but did not +get the Welcome inside the window, because the peer's ring still held the node's previous incarnation +(`Exiting` → `Down` → `Removed`). The fallback fired on the timer and the node formed a **second cluster**. +The original design assumed `Cluster.Join(SelfAddress)` would be ignored mid-handshake; **it is not — it +wins.** Since manual failover deliberately produces exactly that restart, without this guard every manual +failover could island the node it bounced. Pinned by +`SelfFormBootstrapTests.Reachable_peer_suppresses_self_forming_until_it_goes_away`, whose "peer" is a bare +`TcpListener` that speaks no Akka: reachable, never answering a join. + +**2 — Seed membership.** The fallback fires **only when this node's own address appears in its own `SeedNodes`**. A node that is not one of its own seeds is never legitimately first, and if it self-formed, a later-booting real seed would form a second cluster the two could never merge. That is exactly the current docker-dev topology: the site-a/site-b driver nodes list only `central-1` as a seed, so @@ -330,8 +344,9 @@ strategy already accepts, with the same recovery (restart one side). Pinned by `SelfFormBootstrapTests` (`tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/`), which starts real hosts through the production bootstrap: a lone non-first seed comes Up; a disabled window keeps waiting; a node -absent from its own seed list never self-forms. The two negative cases each carry a positive control (an -explicit self-join) so they cannot pass merely because the node was unformable. +absent from its own seed list never self-forms; a node whose peer is merely *reachable* keeps waiting. Every +negative case carries a positive control (an explicit self-join, or dropping the peer listener) so none can +pass merely because the node was unformable. > **Live gate: PASSED (docker-dev, 2026-07-22).** Drilled on the six-node rig, defect first and fix > second. On the pre-change image, `central-2` started alone (central-1 stopped) and logged **zero** join diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ClusterBootstrapFallback.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ClusterBootstrapFallback.cs index e3eeb76e..82a69226 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ClusterBootstrapFallback.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Cluster/ClusterBootstrapFallback.cs @@ -20,11 +20,22 @@ namespace ZB.MOM.WW.OtOpcUa.Cluster; /// (a booting node only self-forms when NO seed answers InitJoin), so a peer booting after this /// node self-formed simply joins it. /// -/// Races are benign. If the join completes between window expiry and -/// Cluster.Join(SelfAddress), Akka ignores the join — a node joins a cluster at most once -/// per incarnation. The residual risk is both pair nodes cold-starting inside the window while -/// mutually unreachable (a boot-time partition): both self-form, the same dual-active class the -/// auto-down downing strategy already accepts, with the same recovery (restart one side). +/// Reachability guard (added 2026-07-22 after a live-gate failure). An expired window +/// is NOT sufficient evidence that the peer is down, and joining on the timer alone islands nodes. +/// Drilled on the docker-dev rig: a node bounced by a manual failover restarted, received +/// InitJoinAck from its live peer — a join in flight and healthy — but did not get the +/// Welcome inside the window, because the peer's ring still held the node's previous incarnation +/// (Exiting → Down → Removed). The fallback fired anyway and the node formed a SECOND cluster, +/// islanded until an operator restarted it. So Join(SelfAddress) is not ignored +/// mid-handshake — it wins, which is the opposite of what the original design assumed. Before +/// self-forming, this therefore TCP-probes the other seed addresses: if any accepts a connection +/// the peer is alive, a join is likely already in flight, and the fallback waits another window +/// instead. That is also exactly what the fallback claims to detect — "no seed answered InitJoin +/// (peer down at boot)". +/// +/// Residual risk. Both pair nodes cold-starting inside the window while mutually +/// unreachable (a boot-time partition): both self-form, the same dual-active class the auto-down +/// downing strategy already accepts, with the same recovery (restart one side). /// /// Deliberately duplicated from ScadaBridge's equivalent (like the termination watchdogs) — /// the two repos share the behavior, not a package. @@ -39,7 +50,16 @@ public static class ClusterBootstrapFallback /// The actor system whose cluster membership is being watched. /// The bound cluster options carrying the window and the seed list. /// Logger for the armed / inert / self-forming decisions. - public static void Arm(ActorSystem system, AkkaClusterOptions options, ILogger logger) + /// + /// Overrides the reachability probe (host, port) → reachable. Production passes + /// for the real TCP connect; tests substitute it to drive the guard + /// deterministically. + /// + public static void Arm( + ActorSystem system, + AkkaClusterOptions options, + ILogger logger, + Func>? peerProbe = null) { ArgumentNullException.ThrowIfNull(system); ArgumentNullException.ThrowIfNull(options); @@ -66,28 +86,108 @@ public static class ClusterBootstrapFallback return; } + // Every seed that is not this node. These are the addresses whose reachability decides + // whether an expired window means "peer is down" or "peer is alive and we are mid-join". + var peerSeeds = options.SeedNodes + .Select(s => TryParseAddress(s, out var a) ? a : null) + .Where(a => a is not null && !a.Equals(self)) + .Select(a => (Host: a!.Host ?? string.Empty, Port: a!.Port ?? 0)) + .Where(p => !string.IsNullOrWhiteSpace(p.Host) && p.Port > 0) + .ToList(); + + var probe = peerProbe ?? TryConnectAsync; + var joined = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); cluster.RegisterOnMemberUp(() => joined.TrySetResult()); _ = Task.Run(async () => { - var winner = await Task.WhenAny(joined.Task, Task.Delay(window)); - if (winner == joined.Task || system.WhenTerminated.IsCompleted) + while (true) { + var winner = await Task.WhenAny(joined.Task, Task.Delay(window)); + if (winner == joined.Task || system.WhenTerminated.IsCompleted) + { + return; + } + + var reachable = await AnyReachableAsync(peerSeeds, probe); + if (reachable is not null) + { + // Do NOT self-form. A reachable peer means either a join already in flight (the + // failure this guard exists for) or a peer about to answer — self-forming here + // creates a second cluster that can never merge. + logger.LogInformation( + "Self-form window ({Window}) expired, but seed peer {Peer} is reachable — a join is " + + "most likely already in flight (a node re-joining after a restart is not admitted " + + "until its previous incarnation is removed). Continuing to wait rather than " + + "forming a second cluster.", + window, + reachable); + continue; + } + + logger.LogWarning( + "No cluster membership after {Window} and no seed peer is reachable — the peer is down " + + "at boot. Self-forming a cluster at {Self} so this node becomes operational; if the " + + "peer was merely partitioned (not dead), the pair is now dual-active — restart one " + + "side after the partition heals (accepted availability-first trade, decision " + + "2026-07-22).", + window, + self); + cluster.Join(self); return; } - - logger.LogWarning( - "No cluster membership after {Window} — no seed answered InitJoin (peer down at boot). " - + "Self-forming a cluster at {Self} so this node becomes operational; if the peer was " - + "merely partitioned (not dead), the pair is now dual-active — restart one side after " - + "the partition heals (accepted availability-first trade, decision 2026-07-22).", - window, - self); - cluster.Join(self); }); } + /// How long a single peer-reachability connect attempt may take. + public static readonly TimeSpan ProbeTimeout = TimeSpan.FromSeconds(2); + + /// + /// Returns the first reachable peer as host:port, or when none + /// answered — including when there are no peer seeds at all (a lone-seed config, where an + /// expired window really does mean this node is on its own). + /// + private static async Task AnyReachableAsync( + IReadOnlyList<(string Host, int Port)> peers, + Func> probe) + { + foreach (var (host, port) in peers) + { + bool ok; + try + { + ok = await probe(host, port); + } + catch (Exception) + { + // A probe that cannot even be attempted (DNS gone, socket exhaustion) is treated as + // unreachable — it must not throw out of the watchdog and disarm the fallback. + ok = false; + } + + if (ok) return $"{host}:{port}"; + } + + return null; + } + + private static async Task TryConnectAsync(string host, int port) + { + using var client = new System.Net.Sockets.TcpClient(); + using var cts = new CancellationTokenSource(ProbeTimeout); + try + { + await client.ConnectAsync(host, port, cts.Token); + return client.Connected; + } + catch (Exception) + { + // Refused, unresolvable, or timed out — all mean "not reachable right now". + return false; + } + } + private static bool TryParseAddress(string seed, out Address address) { try diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SelfFormBootstrapTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SelfFormBootstrapTests.cs index 13155071..0cbdffcf 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SelfFormBootstrapTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests/SelfFormBootstrapTests.cs @@ -173,6 +173,60 @@ public sealed class SelfFormBootstrapTests } } + /// + /// Regression for the live-gate islanding defect (2026-07-22). A peer that is reachable + /// but not yet admitting the join must NOT be treated as "peer down at boot". + /// + /// + /// + /// On the docker-dev rig, a node bounced by a manual failover restarted, received + /// InitJoinAck from its live peer — the join was in flight and healthy — but did + /// not get the Welcome inside the window, because the peer's ring still held the node's + /// previous incarnation. The fallback fired on the timer and the node formed a + /// second cluster, islanded until an operator restarted it. The original design + /// assumed Join(SelfAddress) would be ignored mid-handshake; it is not. + /// + /// + /// The peer here is a bare that accepts connections and speaks + /// no Akka at all — reachable at the transport level, never answering a join. That is the + /// defect's shape with nothing faked: under the pre-fix code this node self-formed on the + /// timer; under the guard it keeps waiting. The positive control then proves the guard is + /// a guard and not a disablement: drop the listener and the same node self-forms. + /// + /// + [Fact] + public async Task Reachable_peer_suppresses_self_forming_until_it_goes_away() + { + var selfPort = FreePort(); + var peerPort = FreePort(); + + // A peer that accepts TCP and does nothing else — reachable, but no join will ever complete. + var peer = new TcpListener(IPAddress.Loopback, peerPort); + peer.Start(); + + var host = await StartNodeAsync( + "otopcua-selfform-4", selfPort, peerPort, TimeSpan.FromSeconds(2)); + try + { + var system = host.Services.GetRequiredService(); + + // Several windows pass. Pre-fix this self-formed after the first one. + (await WaitForUpMemberAsync(system, TimeSpan.FromSeconds(10))) + .ShouldBeFalse("a reachable peer means a join may be in flight — self-forming here islands this node"); + + // Positive control: the peer really goes away, and now the fallback does its job. + peer.Stop(); + + (await WaitForUpMemberAsync(system, TimeSpan.FromSeconds(30))) + .ShouldBeTrue("once the peer is genuinely gone the fallback must still self-form"); + } + finally + { + try { peer.Stop(); } catch (SocketException) { /* already stopped by the test body */ } + await StopAsync(host); + } + } + /// /// THE island guard. A node that is not in its own seed list — the current docker-dev site-node /// topology, where site-a/site-b list only central-1 — must never self-form: it would island