ScadaBridge/archreview/01-cluster-host-failover.md

Architecture Review 01 — Cluster Infrastructure, Host, Failover, Health Monitoring, Deployment Topology (Round 2)

Date: 2026-07-12 (round 2; round-1 report 2026-07-08, baseline commit b910f5eb; current HEAD 8c888f13)

Scope

Same domain as round 1: src/ZB.MOM.WW.ScadaBridge.ClusterInfrastructure, src/ZB.MOM.WW.ScadaBridge.Host, src/ZB.MOM.WW.ScadaBridge.HealthMonitoring, docker/, docker-env2/, deploy/wonder-app-vd03/, Traefik config, the four design docs (Component-ClusterInfrastructure.md, Component-Host.md, Component-HealthMonitoring.md, Component-TraefikProxy.md), and the test projects.

Method

1. Re-read the round-1 report and archreview/plans/PLAN-01-cluster-host-failover.md (findings-coverage table) plus the master tracker's cross-plan notes.
2. Verified every round-1 finding against the current source (not the plan's claims), with file:line evidence.
3. Fresh sweep over git diff b910f5eb..HEAD --stat scoped to the domain paths (~2,550 insertions across 68 files: ClusterActivityEvaluator, CentralSingletonRegistrar, OldestNodeActiveHealthCheck, the SBR downing line, acked health transport, metrics-staleness state machine, failover-drill.sh, options validators, the two-node cluster test rig, and adjacent new code that landed in this domain from other plans — InProcessScriptArtifactChangeBus, CentralHealthAuditBacklogProvider).

One-line verdict: Round 1 found one Critical foundation crack (SBR never enabled) under high polish; round 2 finds the crack fixed and behaviorally proven, 18 of 28 findings fully fixed and verified, the active-node model coherently unified — but the shipped failover drill and recovery narrative overpromise: a hard crash of the active (oldest) central node still ends in total central outage (registered deferred decision), and the never-executed drill script codifies a ~25s recovery that topology (first-seed bootstrap + keep-oldest asymmetry) cannot deliver in its most likely case.

---

Round-1 Finding Disposition

Every disposition below was verified by reading the current source; plan claims were not trusted.

#	Round-1 finding	R1 severity	Disposition	Evidence (file:line)
S1	SBR downing provider never enabled — no automatic failover on crash/partition	Critical	Fixed (verified) — provider named in HOCON; behaviorally proven by a real two-node kill test. Residual: two-node keep-oldest cannot survive an oldest-node crash — a registered deferred user decision, honestly documented in the test's XML doc	AkkaHostedService.cs:290 (downing-provider-class = "Akka.Cluster.SBR.SplitBrainResolverProvider, Akka.Cluster"), doc at :235-238; SbrFailoverTests.cs:46-80 (crash younger → member removed + singleton survives), asymmetry documented at :15-23; fixture builds from production BuildHocon (TwoNodeClusterFixture.cs:56-64)
S2	"Active node" = leader vs oldest diverge (wrong purge node, wrong Primary label, wrong self-report)	High	Fixed (verified) — single oldest-member evaluator consumed by every product gate; divergence proven by a rejoin test	ClusterActivityEvaluator.cs:20-47; ActiveNodeGate.cs:47-48; AkkaClusterNodeProvider.cs:36 (SelfIsPrimary), :47-58 (Primary label from oldest); purge gate delegates to SelfIsPrimary (SiteServiceRegistration.cs:118-122); central self-report gate (CentralHealthReportLoop.cs:93); S&F delivery gate reuses it (AkkaHostedService.cs:878-882); ActiveNodeSemanticsTests.cs:29-68
S3	Partition: both centrals return 200 on /health/active; Traefik serves both	High	Fixed (verified) — active = oldest Up member (exactly one in both partition views, since an unreachable member stays in gossip) AND database reachable	OldestNodeActiveHealthCheck.cs:27-33; Program.cs:231-234 (database check tagged Ready + Active), :252-257 (active-node = OldestNodeActiveHealthCheck); Component-TraefikProxy.md:29,116
S4	10s singleton drains inside the 5s cluster-leave phase	Medium	Fixed (verified)	AkkaHostedService.cs:304-307 (phases.cluster-leave.timeout = 15s); parsed-config test in HoconBuilderTests.cs
S5	NotificationOutbox + AuditLogIngest singletons had no drain task	Medium	Fixed (verified) — all seven central singletons routed through the registrar, which always adds a PhaseClusterLeave GracefulStop drain	CentralSingletonRegistrar.cs:46-62; call sites AkkaHostedService.cs:466 (outbox), :499 (ingest), :564, :609, :632, :663, :689; rationale comment :459-465
S6	Health-report loss recovery was dead code (fire-and-forget transport never throws)	Medium	Fixed (verified) — transport is now an acked Ask round-trip; the counter-restore path is live	AkkaHealthReportTransport.cs:30-40 (Ask + throw on nack/timeout); acks at SiteCommunicationActor.cs:395-407 and CentralCommunicationActor.cs:425; restore fires on throw (HealthReportSender.cs:170-187)
S7	Offline detection keyed off heartbeats masks a dead metrics pipeline; spec/code disagreed	Medium	Fixed (verified) — two-signal model: heartbeat liveness + IsMetricsStale on report silence; options cross-validated; spec updated to match. Residual: never-reported sites can't be flagged (new Low finding N3)	CentralHealthAggregator.cs:295-312; HealthMonitoringOptions.cs (MetricsStaleTimeout, CentralOfflineTimeout); validator cross-field rules (HealthMonitoringOptionsValidator.cs); Component-HealthMonitoring.md:46-49; UI badge Health.razor:209
S8	appsettings.Site.json second seed targeted MetricsPort 8084; validator gap	Medium	Fixed (verified) — seed now 8085 with an explanatory _seedNodes note; validator rejects seeds on GrpcPort and MetricsPort	appsettings.Site.json (SeedNodes → localhost:8085); StartupValidator.cs:117-134
S9	Docker stop grace shorter than CoordinatedShutdown	Medium	Fixed (verified) — stop_grace_period: 30s on all 8 docker/ app services and all 4 docker-env2/ services, each with a rationale comment	docker/docker-compose.yml:7,34,61,81,101,121,141,161; docker-env2/docker-compose.yml:7,34,61,81
S10	Aggregator never forgets deleted sites	Low	Fixed (verified) — eviction piggybacks on the existing 60s site-address-cache refresh, no dedicated deletion event needed	CentralHealthAggregator.cs:200-211 (PruneUnknownSites, protects the synthetic $central entry); caller CentralCommunicationActor.cs:599-605
S11	Dead-letter monitor: unbounded warning volume	Low	Fixed (verified) — 10 warnings/min window, suppressed-count summary on rollover, metric counting never throttled	DeadLetterMonitorActor.cs:22-26,51-80,89
S12	Mandatory two-seed rule forces phantom seeds in single-node installs	Low	Partially fixed — AllowSingleNodeCluster flag + validator with a phantom-seed-naming message shipped; the one real single-node artifact still ships the phantom seed and lacks the flag (overlay edit deferred out-of-band; see N2)	ClusterOptions.cs:86; ClusterOptionsValidator.cs:29-35; unfixed artifact: deploy/wonder-app-vd03/appsettings.Central.json (seeds still localhost:8081 + phantom localhost:8091, no AllowSingleNodeCluster)
P1	Readiness probes fan out 5 cluster Asks per poll	Low	Accepted (per plan) — unchanged, bounded 2s concurrent Asks	RequiredSingletonsHealthCheck.cs (only doc-comment change since baseline)
P2	Startup compiles every inbound method sequentially	Low	Fixed (verified) (delivered via plan 06) — parallel compile with a thread-safety rationale	Program.cs:412-415 (Parallel.ForEach(methods, …))
P3	Per-tick allocations in health collection	Low	Won't-fix (per report's own "no action") — unchanged	CentralHealthAggregator.cs:187-190
C1	REQ-HOST-6 mandated Akka.Hosting; code hand-rolls HOCON	Medium	Fixed (verified) — resolved by amending the spec (hand-rolled HOCON is now the documented mechanism, with rationale) and dropping the three unused Akka.*.Hosting packages	Component-Host.md:108-116 (bootstrap-mechanism note); ZB.MOM.WW.ScadaBridge.Host.csproj:15-21 (packages removed, comment); Component-Host.md:208
C2	deploy/wonder-app-vd03 shipped DisableLogin: true	High	Fixed (verified) (delivered via plan 07) — fail-fast startup guard refuses DisableLogin outside Development without an explicit second acknowledgement key; the artifact no longer sets it and its _comment_Security documents the removal + a bundled localhost GLAuth for real logins	DisableLoginGuard.cs:17-29; call in Program.cs (Central branch, before AddSecurity); deploy/wonder-app-vd03/appsettings.Central.json:40
C3	LDAP transport plaintext everywhere, contradicting "LDAPS/StartTLS required"	Medium	Partially fixed / accepted posture — docker remains dev-only plaintext (accepted); the deploy artifact now binds a bundled loopback GLAuth (localhost:3893) rather than plaintext across a network, with documented instructions to repoint to corporate AD — but Transport: None, AllowInsecure: true remains and a future AD repoint over the wire would still be plaintext unless changed	deploy/wonder-app-vd03/appsettings.Central.json:29-30,40
C4	Secrets in checked-in docker dev configs	Medium	Accepted (per plan) — labelled dev-only; ConfigSecretsTests still tracks; ${...} env-injection pattern remains the model in the deploy artifact	unchanged
C5	Stale Component-TraefikProxy.md references	Low	Fixed (verified) — doc now names OldestNodeActiveHealthCheck with the correct path, documents /health/ready = database + akka-cluster + required-singletons, and the active check's DB tag	Component-TraefikProxy.md:29,115-121,129
C6	Five copy-pasted ~60-line singleton registration blocks	Low	Fixed (verified) for the central role — CentralSingletonRegistrar collapses manager/drain/proxy; the two site singletons remain hand-rolled by documented intent (role-scoped; see N5)	CentralSingletonRegistrar.cs:31-71; AkkaHostedService.cs:459-465
U1	No real multi-node failover test anywhere	—	Fixed (verified) — real two-node in-process cluster rig built from production HOCON, SBR kill test, active-node divergence test, plus a scripted docker drill (but see N1 — the drill itself was never executed)	TwoNodeClusterFixture.cs; SbrFailoverTests.cs; ActiveNodeSemanticsTests.cs; CentralSingletonRegistrarTests.cs; docker/failover-drill.sh
U2	Windows Service lifecycle / down-if-alone recovery story undefined	—	Partially fixed — WhenTerminated watchdog stops the host process on an out-of-band ActorSystem termination so a supervisor restarts it (UnexpectedTerminationTests cover it); the recovery contract is now spec'd. But the install.ps1 sc.exe failure recovery actions were deferred to the artifact owner, and the contract does not converge for a non-first-seed survivor (see N1)	AkkaHostedService.cs:203-217 (watchdog), :342-346 (_stopRequested guard); Component-ClusterInfrastructure.md:104-111; tracker "Deferred during Wave 1"
U3	TLS absent at every layer	—	Deferred (accepted, spec-visible) — explicit "Production TLS profile: not yet implemented" roadmap note added as planned	Component-TraefikProxy.md:133
U4	No Dockerfile HEALTHCHECK; /healthz runs zero checks	—	Deferred (accepted per plan) — unchanged (grep -c HEALTHCHECK docker/Dockerfile = 0; Program.cs:381)	—
U5	NodeName missing from wonder-app-vd03 overlays → NULL SourceNode	—	Not fixed — the artifact's Node section still has no NodeName (verified by parsing the JSON). Deferred out-of-band with an owner; see N2 for why that rationale is shaky	deploy/wonder-app-vd03/appsettings.Central.json (Node: Role/NodeHostname/RemotingPort only)
U6	Known-but-open follow-ups (SecuredWrite SourceNode NULL; cert-trust persistence; docker-env2 mirrors findings)	—	Mixed, all tracked — SecuredWrite SourceNode fixed (register row 7, via ICentralAuditWriter); cert-trust persistence still deferred with trigger (register row 11); docker-env2 got stop_grace_period	docs/plans/2026-07-08-deferred-work-register.md:23,35; docker-env2/docker-compose.yml:7-81
U7	Online/offline transitions unrecorded	—	Fixed (verified) — LastStatusChangeAt stamped on every online↔offline flip (report-recovery, heartbeat-recovery, offline-detection), never moved by status-preserving traffic; surfaced as "offline since" in the UI; spec'd	SiteHealthState.cs (field + doc); CentralHealthAggregator.cs:86,170-172,282; Health.razor:216; Component-HealthMonitoring.md:49

Disposition counts: 18 Fixed (verified) · 3 Partially fixed (S12, C3, U2) · 1 Not fixed (U5, deferred out-of-band with owner) · 5 Deferred/accepted with rationale (P1, P3, C4, U3, U4) · 1 Mixed-tracked (U6). Plus one registered residual design gap on S1 (keep-oldest active-crash, deferred user decision).

---

New Findings (Round 2)

[High] N1 — The down-if-alone recovery story does not converge for the non-first-seed survivor, and the shipped failover drill codifies a recovery the topology cannot deliver

The keep-oldest asymmetry itself is a registered deferred user decision (master tracker, 2026-07-08: crashing the oldest/active central node makes the younger survivor down itself — total central outage) and is honestly documented in SbrFailoverTests.cs:15-23. What is new in this round is that the artifacts built on top of that gap contradict it:

1. docker/failover-drill.sh:12-16 kills the ACTIVE node — under the now-unified oldest-member semantics, the active node is the oldest, i.e. the drill exercises exactly the crash keep-oldest cannot survive. The intended safety net is Task 20's restart loop: survivor self-downs → run-coordinated-shutdown-when-down terminates its ActorSystem → the watchdog (AkkaHostedService.cs:203-217) exits the process → docker restart: unless-stopped (docker-compose.yml:27,54) restarts the container.
2. But the restarted survivor can only re-bootstrap if it is the FIRST seed. Both central nodes list scadabridge-central-a first (docker/central-node-a/appsettings.Central.json:10-13, central-node-b/…:10-13). Akka's seed-join rule lets only the first seed self-join to form a new cluster; a restarted central-b with central-a still dead loops on InitJoin forever — never Up, never /health/active 200. So when the drill's victim is central-a (the likelier oldest, since it is listed first and probed first by active_container), the drill fails at its 90s timeout and central stays dark until the victim returns. Only a central-b victim (or a younger-node crash) produces the advertised PASS.
3. The drill has never been executed: docker/README.md:288 — "run after next deploy and record here (the script was validated with bash -n; no live cluster was up when it was committed)" — yet docker/README.md:286 promises "Expected failover: ~25s … plus the Traefik ~5s health-check interval", and Component-ClusterInfrastructure.md:111 claims "the restarted process rejoins as a fresh incarnation (the keep-oldest resolver handles the rejoin cleanly)" — true only while the peer is reachable.

Recommendation: (a) run the drill live once per direction (victim = a, victim = b) and record both outcomes in the README — the asymmetric result is itself the documentation the deferred decision needs; (b) fix the drill/README narrative to state which crash it can prove; (c) as a cheap partial mitigation independent of the strategy decision, make each node list itself first in its own SeedNodes (Akka tolerates asymmetric seed ordering) or document the operator action ("restart the dead node / restart the survivor with a self-first seed override") so a lone surviving central can actually re-form; (d) fold this into the pending keep-oldest topology/strategy decision (keep-majority + lighthouse node, static-quorum, or an accepted-risk note).

[Medium] N2 — The wonder-app-vd03 overlay edits remain unapplied while the artifact sits editable in the working tree; the deferral rationale is factually wrong

Three PLAN-01 sub-edits (Tasks 16/20/23) were deferred with the rationale "that production deploy artifact is not tracked in this repo … the artifact directory itself is not present" (master tracker, Deferred during Wave 1). The directory is present at deploy/wonder-app-vd03/ — it is merely gitignored (.gitignore:48: /deploy/) — and was clearly edited during this initiative (its _comment_Security at appsettings.Central.json:40 describes the PLAN-07 DisableLogin removal and the new bundled GLAuth). So the artifact is being maintained in this working tree, yet still lacks:

• Node.NodeName → every audit row from the only production install stamps a NULL SourceNode, defeating IX_AuditLog_Node_Occurred and the per-node stuck KPIs (round-1 U5, unchanged);
• Cluster.AllowSingleNodeCluster: true + removal of the phantom seed localhost:8091 (round-1 S12's artifact half — the node still dials a nonexistent seed forever);
• the install.ps1 sc.exe failure service-recovery actions that the down-if-alone watchdog contract requires ("the Host process must exit so the service supervisor restarts it" — Component-ClusterInfrastructure.md:106-110); without them, a self-downed single node on that host stays down.

Recommendation: apply the three documented edits directly to the on-disk artifact (they are one-liners specified in PLAN-01 Tasks 16/20/23), or correct the deferral record and add a RUNBOOK step so the gap is operator-visible.

[Low] N3 — Metrics-staleness can never fire for a site that has never delivered a report

CentralHealthAggregator.CheckForOfflineSites guards the staleness check with state.LastReportReceivedAt is { } lastReport (CentralHealthAggregator.cs:300-302), so a site registered via heartbeat only (MarkHeartbeat registration path, :114-122, leaves LastReportReceivedAt null) is never flagged IsMetricsStale and never logs the staleness warning. This is the "stuck from first boot" variant of round-1 S7: a site whose DeploymentManager singleton never starts (so IsActiveNode is false on both nodes and HealthReportSender skips every report — HealthReportSender.cs:85-86) shows online-with-no-metrics indefinitely. Mitigation: the UI renders "awaiting first report" for the null timestamp (Health.razor:222), so the state is at least visible — but it carries no warning styling or aggregator log line, unlike the stale case. Fix: treat LastReportReceivedAt == null with now - LastHeartbeatAt > MetricsStaleTimeout (or first-seen time) as stale too.

[Low] N4 — CentralHealthReportLoop still narrates "cluster leader (Primary)"

The class doc ("Only the cluster leader (Primary) generates reports", CentralHealthReportLoop.cs:12-14) and the sequence-seed comment ("reports from a newly-elected central leader", :44-45) survived the S2 rewiring. The behavior is correct (:93 gates on IClusterNodeProvider.SelfIsPrimary, now oldest-member), but the wording re-teaches the exact leader/oldest conflation the fix eradicated. Two-line comment fix.

[Low] N5 — Site-role singletons still lack drain tasks; the registrar can't express role scoping

CentralSingletonRegistrar deliberately excludes the two site singletons because they need .WithRole(siteRole) (AkkaHostedService.cs:464-465), so deployment-manager (:791-810) and event-log-handler (:839-845) remain hand-rolled with bare PoisonPill termination and no PhaseClusterLeave drain. The S5 rationale (let in-flight writes drain before handover) applies to the DeploymentManager's SQLite static-override/native-alarm persistence just as it did to the central EF writers; the site side is safe-but-lossy on graceful failover. Adding an optional role parameter to CentralSingletonRegistrar.Start (and renaming it SingletonRegistrar) closes the asymmetry for a few lines.

[Low] N6 — PLAN-01's two live deferrals are missing from the canonical deferred-work register

docs/plans/2026-07-08-deferred-work-register.md (established by PLAN-08 T11 as the triage home, with rationale + revisit-trigger columns) contains neither (a) the keep-oldest active-crash availability gap nor (b) the wonder-app-vd03 overlay edits — both live only in archreview/plans/00-MASTER-TRACKER.md prose ("Follow-up discovered during P0 execution", "Deferred during Wave 1"). The register's structure (owner, rationale, revisit trigger) is exactly what the SBR gap — an availability-critical open decision — needs; today a reader of the register alone would conclude this domain has no open items.

---

What's Genuinely Good

• The Critical fix is proven, not just present. The downing provider is one HOCON line (AkkaHostedService.cs:290), but the team built a real two-node in-process cluster rig from production BuildHocon output (TwoNodeClusterFixture.cs:56-64) and asserted member removal + singleton survival after an abrupt terminate — the exact test shape whose absence let round-1's Critical survive. HoconBuilderTests now parse the config instead of grepping strings.
• Honest empiricism over plan compliance. When the live rig revealed that killing the oldest node makes the survivor self-down, Task 4 was rewritten to the achievable guarantee and the asymmetry documented in the test XML doc (SbrFailoverTests.cs:15-23) and ActiveNodeSemanticsTests.cs:16-24 (which switched to a graceful Leave with a documented deviation note) rather than shipping a green-but-lying test.
• The active-node unification is genuinely single-sourced. One 48-line static evaluator (ClusterActivityEvaluator) now backs the inbound-API gate, /health/active, the Primary/Standby labels, the event-log purge gate, the central self-report loop, and (via the SelfIsPrimary seam) plan 02's S&F delivery gate — with a comment in each consumer pointing back at the review finding. The seam design let a different plan consume it without inventing a parallel check (AkkaHostedService.cs:873-882).
• CentralSingletonRegistrar removed ~300 lines of copy-paste and fixed the drift it caused (missing drains) in one move — the correct root-cause response to a copy-paste bug.
• The health state machine is careful engineering: immutable SiteHealthState records swapped via CAS loops with correct lost-race semantics (a lost CAS on offline-marking means the site was just heard from — deliberately not retried, CentralHealthAggregator.cs:279-292), clock-skew-anchored heartbeat recovery (:145-156), a collision-proof $central synthetic key, and cross-field options validation with key-naming messages (HealthMonitoringOptionsValidator).
• Doc reconciliation was done properly: REQ-HOST-6 was amended with a rationale note and the unused packages dropped (instead of quietly deleting the requirement); the Traefik doc, the health-monitoring spec, and the cluster-infrastructure recovery contract all now describe the code as built.
• DisableLoginGuard is the right shape for the C2 fix: fail-fast at boot, environment-gated, with an explicit consciously-named acknowledgement key — a durable control rather than a one-time artifact edit.

Severity Tally — New Findings Only

Severity	Count	Findings
Critical	0	—
High	1	N1 (recovery story doesn't converge for non-first-seed survivor; never-run drill codifies an undeliverable ~25s recovery)
Medium	1	N2 (wonder-app-vd03 overlay edits unapplied; deferral rationale factually wrong)
Low	4	N3 (metrics-stale null-report gap), N4 (stale "leader" comments), N5 (site singletons undrained / registrar role gap), N6 (deferrals missing from canonical register)