From 956582727522742d8fb4f24443eb38391515af65 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 23 Jul 2026 11:55:43 -0400 Subject: [PATCH] feat(mesh-phase4): retire DB-reachability from ServiceLevel on DB-less nodes Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW --- ...6-07-23-mesh-phase4-cut-driver-configdb.md | 16 ++ .../OpcUa/OpcUaPublishActor.cs | 52 ++++++- .../ServiceCollectionExtensions.cs | 29 +++- .../OpcUa/OpcUaPublishActorTests.cs | 137 ++++++++++++++++++ 4 files changed, 223 insertions(+), 11 deletions(-) diff --git a/docs/plans/2026-07-23-mesh-phase4-cut-driver-configdb.md b/docs/plans/2026-07-23-mesh-phase4-cut-driver-configdb.md index 7c34c9db..8dce877e 100644 --- a/docs/plans/2026-07-23-mesh-phase4-cut-driver-configdb.md +++ b/docs/plans/2026-07-23-mesh-phase4-cut-driver-configdb.md @@ -276,6 +276,22 @@ apply through the fetcher), so the change is defensive nullability, not new cont 1920/2096/2588 is inside a Direct-mode-only path before guarding — if any is on the shared apply path, that is a Phase-3 gap to surface, not silently guard. +**Two specific sites the Task-2 code review surfaced (MUST be handled here):** +- **Remove the interim `dbFactory!` at `ServiceCollectionExtensions.cs:~470`** (added in Task 2 to preserve + compilation) — once `DriverHostActor._dbFactory` is nullable, pass the plain (nullable) `dbFactory`. +- **`UpsertNodeDeploymentState` (2613-2643) is called UNCONDITIONALLY on the FetchAndCache apply path** + (`BeginFetchAndCacheApply`/`HandleFetchedForApply`, ~1716/1723/1774/1785/1801). Its try/catch swallows the + null-factory NRE and logs a misleading "transient DB" warning on every deploy on a driver-only node. The + `_dbFactory is not null` guard fixes this. +- **`SpawnScriptedAlarmHost` (called unconditionally from PreStart ~524) constructs + `new EfAlarmConditionStateStore(_dbFactory, ...)` at ~587-588** — with a null factory every alarm + Load/Save NREs (swallowed → scripted-alarm state silently dead on driver-only nodes). **This is fixed by + Task 6** (which swaps in the LocalDb-backed store), NOT by a null guard — do NOT leave the Ef store + constructed with a null factory. If Task 6 lands after this task, add a temporary `_dbFactory is not null` + guard around the Ef-store construction so a DB-less node skips it cleanly until Task 6 wires the LocalDb + store; the two tasks together must leave a DB-less node with a WORKING (LocalDb) condition store, not a + skipped one. + **Step 1: Write failing tests** — construct a `DriverHostActor` with `dbFactory: null`, `fetchAndCacheMode: true`, a fake fetcher + LocalDb cache (mirror `DriverHostActorFetchAndCacheTests`): (a) a full dispatch → Applied ack, **no** NullReferenceException from a factory deref; (b) bootstrap diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs index c96faec6..7cf523c9 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -103,6 +103,11 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers private readonly IDbContextFactory? _dbFactory; private readonly AddressSpaceApplier? _applier; private readonly IActorRef? _dbHealthProbe; + /// Per-cluster mesh Phase 4: this is a driver-only node with NO ConfigDb (LocalDb is its + /// config store). There is no DB to probe, so the ServiceLevel DB-reachability input is retired — + /// DbReachable is fed constant true and only snapshot staleness can demote. See + /// . + private readonly bool _dbLess; private readonly TimeSpan _staleWindow; private readonly TimeSpan _probeFreshnessWindow; private readonly TimeSpan _healthTickInterval; @@ -153,6 +158,9 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers /// The period between self-driven DB-health refresh ticks (each /// Asks for its cached status); defaults to 5 seconds. No timer is /// started when is null. + /// Per-cluster mesh Phase 4: this is a driver-only node with no ConfigDb, so the + /// ServiceLevel DB-reachability input is retired (DbReachable treated constant-true, only + /// staleness demotes). No is wired on such a node. /// The configured for creating this actor, pinned to the /// dispatcher. public static Props Props( @@ -164,7 +172,8 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers IActorRef? dbHealthProbe = null, TimeSpan? staleWindow = null, TimeSpan? probeFreshnessWindow = null, - TimeSpan? healthTickInterval = null) => + TimeSpan? healthTickInterval = null, + bool dbLess = false) => Akka.Actor.Props.Create(() => new OpcUaPublishActor( sink ?? NullOpcUaAddressSpaceSink.Instance, serviceLevel ?? NullServiceLevelPublisher.Instance, @@ -175,7 +184,8 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers dbHealthProbe, staleWindow, probeFreshnessWindow, - healthTickInterval)).WithDispatcher(DispatcherId); + healthTickInterval, + dbLess)).WithDispatcher(DispatcherId); /// Test-only Props that omits the pinned-dispatcher requirement and skips the /// DPS subscribe so unit tests can spin up the actor on a vanilla TestKit cluster. @@ -195,6 +205,9 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers /// The period between self-driven DB-health refresh ticks (each /// Asks for its cached status); defaults to 5 seconds. No timer is /// started when is null. + /// Per-cluster mesh Phase 4: this is a driver-only node with no ConfigDb, so the + /// ServiceLevel DB-reachability input is retired (DbReachable treated constant-true, only + /// staleness demotes). No is wired on such a node. /// The configured for creating this actor in tests, without dispatcher /// pinning or the DPS subscribe. public static Props PropsForTests( @@ -207,7 +220,8 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers IActorRef? dbHealthProbe = null, TimeSpan? staleWindow = null, TimeSpan? probeFreshnessWindow = null, - TimeSpan? healthTickInterval = null) => + TimeSpan? healthTickInterval = null, + bool dbLess = false) => Akka.Actor.Props.Create(() => new OpcUaPublishActor( sink ?? NullOpcUaAddressSpaceSink.Instance, serviceLevel ?? NullServiceLevelPublisher.Instance, @@ -218,7 +232,8 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers dbHealthProbe, staleWindow, probeFreshnessWindow, - healthTickInterval)); + healthTickInterval, + dbLess)); /// Initializes a new instance of the class. /// The OPC UA address space sink. @@ -237,6 +252,9 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers /// The period between self-driven DB-health refresh ticks (each /// Asks for its cached status); defaults to 5 seconds. No timer is /// started when is null. + /// Per-cluster mesh Phase 4: this is a driver-only node with no ConfigDb, so the + /// ServiceLevel DB-reachability input is retired (DbReachable treated constant-true, only + /// staleness demotes). No is wired on such a node. public OpcUaPublishActor( IOpcUaAddressSpaceSink sink, IServiceLevelPublisher serviceLevel, @@ -247,7 +265,8 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers IActorRef? dbHealthProbe = null, TimeSpan? staleWindow = null, TimeSpan? probeFreshnessWindow = null, - TimeSpan? healthTickInterval = null) + TimeSpan? healthTickInterval = null, + bool dbLess = false) { _sink = sink; _serviceLevel = serviceLevel; @@ -256,6 +275,7 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers _dbFactory = dbFactory; _applier = applier; _dbHealthProbe = dbHealthProbe; + _dbLess = dbLess; _staleWindow = staleWindow ?? TimeSpan.FromSeconds(30); _probeFreshnessWindow = probeFreshnessWindow ?? TimeSpan.FromSeconds(30); _healthTickInterval = healthTickInterval ?? TimeSpan.FromSeconds(5); @@ -635,6 +655,28 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers return; } + // Per-cluster mesh Phase 4: on a driver-only (DB-less) node there is no ConfigDb to probe — + // LocalDb IS the config store and is in-process, so it can never be "unreachable". Retire the + // DB-reachability input by feeding DbReachable=true CONSTANT and deriving staleness from the + // snapshot age ALONE (no DB-health term). This is the survive-alone posture and it is + // client-visible: a healthy DB-less node must publish 240 (240+10=250 as Primary) even with + // central down, so clients keep steering to it — the old DbReachable=false would have computed + // the (false,true,false) → 0 arm and driven clients AWAY from a node serving perfectly. + // Staleness ("running behind on config") still demotes to 200. The Detached guard above still + // wins, so a DB-less Detached node stays 0. + if (_dbLess) + { + var utcNow = DateTime.UtcNow; + var dbLessInputs = new NodeHealthInputs( + MemberState: SafeSelfStatus(), + DbReachable: true, + OpcUaProbeOk: OpcUaProbeOk(), + Stale: (utcNow - entry.AsOfUtc) > _staleWindow, + IsDriverPrimary: entry.IsDriverPrimary); + Self.Tell(new ServiceLevelChanged(ServiceLevelCalculator.Compute(dbLessInputs))); + return; + } + // Legacy / back-compat seam: with no DB-health probe wired (or before the first sample // arrives) fall back to the old role-only switch. This preserves historical behaviour and // is the bootstrap value until the first DbHealthStatus lands. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs index e374671a..5a8ae862 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs @@ -326,10 +326,19 @@ public static class ServiceCollectionExtensions // (durable AVEVA sink is infra-gated); a deployment binding a real IHistoryWriter overrides. var historyWriter = resolver.GetService() ?? NullHistoryWriter.Instance; - var dbHealth = system.ActorOf( - DbHealthProbeActor.Props(dbFactory), - DbHealthProbeActorName); - registry.Register(dbHealth); + // Per-cluster mesh Phase 4: a driver-only node has NO ConfigDb (LocalDb is its config + // store), so dbFactory is null and there is nothing to probe. Skip spawning the probe + // entirely — spawning it with a null factory would NRE on the first SELECT 1 — and do not + // register its marker key. OpcUaPublishActor then runs in dbLess mode (DbReachable retired, + // treated constant-true) so a healthy DB-less node still publishes full ServiceLevel. + IActorRef? dbHealth = null; + if (dbFactory is not null) + { + dbHealth = system.ActorOf( + DbHealthProbeActor.Props(dbFactory), + DbHealthProbeActorName); + registry.Register(dbHealth); + } // Dependency mux must be spawned before DriverHostActor so the host can forward // AttributeValuePublished into it from the very first driver spawn. @@ -436,7 +445,8 @@ public static class ServiceCollectionExtensions localNode: roleInfo.LocalNode, dbFactory: dbFactory, applier: applier, - dbHealthProbe: dbHealth), + dbHealthProbe: dbHealth, + dbLess: dbFactory is null), OpcUaPublishActorName); registry.Register(publishActor); @@ -452,7 +462,14 @@ public static class ServiceCollectionExtensions // ackRouter: under ClusterClient mode the node comm actor relays ApplyAcks across // the boundary; under Dps it stays null and the host publishes on the // deployment-acks topic exactly as before. - DriverHostActor.Props(dbFactory, roleInfo.LocalNode, + // dbFactory! — KNOWN INTERIM. A driver-only node genuinely passes a null factory here + // (ConfigDb is now gated on hasAdmin, and driver-only nodes are forced to FetchAndCache), + // so DriverHostActor's ConfigDb-touching paths — scripted-alarm state persistence + // (EfAlarmConditionStateStore) and UpsertNodeDeploymentState — fault-and-swallow until + // Task 4 makes the factory nullable + guards those sites (and removes this !) and Task 6 + // re-homes the alarm store to LocalDb. Inert in practice: no driver-only node is + // configured to boot until the Task 10 rig change. + DriverHostActor.Props(dbFactory!, roleInfo.LocalNode, ackRouter: useClusterClient ? nodeComm : null, driverFactory: driverFactory, localRoles: roleInfo.LocalRoles, dependencyMux: mux, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs index 2a2c03ac..e10e9329 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorTests.cs @@ -580,6 +580,143 @@ public sealed class OpcUaPublishActorTests : RuntimeActorTestBase AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), TimeSpan.FromSeconds(2)); } + // ── Per-cluster mesh Phase 4: DB-less driver-only nodes ───────────────────────────────────── + // A driver-only node has no ConfigDb (LocalDb is its config store), so there is no DB to probe. + // Such nodes run with dbHealthProbe: null + dbLess: true and must still publish the full + // survive-alone ServiceLevel (240 follower / 250 Primary) — DbReachable is treated as a + // constant true because the in-process LocalDb can never be "unreachable"; only snapshot + // staleness demotes. + + /// DB-less, member Up, snapshot fresh, follower (non-Primary) → 240. No probe wired, + /// no DbHealthStatus ever arrives; the DB-less branch must still reach full service. + [Fact] + public void DbLess_healthy_follower_publishes_240() + { + var publisher = new RecordingPublisher(); + var local = NodeId.Parse("db-less-follower"); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests( + serviceLevel: publisher, localNode: local, dbLess: true, + staleWindow: TimeSpan.FromSeconds(30))); + + actor.Tell(new RedundancyStateChanged( + Nodes: new[] + { + new NodeRedundancyState(local, RedundancyRole.Secondary, + IsClusterLeader: false, IsDriverPrimary: false, DateTime.UtcNow), + }, + CorrelationId.NewId())); + + AwaitAssert(() => publisher.Levels.ShouldContain((byte)240), + duration: TimeSpan.FromMilliseconds(500)); + } + + /// DB-less, member Up, snapshot fresh, Primary → 250 (basis 240 + 10 Primary bonus). + [Fact] + public void DbLess_healthy_primary_publishes_250() + { + var publisher = new RecordingPublisher(); + var local = NodeId.Parse("db-less-primary"); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests( + serviceLevel: publisher, localNode: local, dbLess: true, + staleWindow: TimeSpan.FromSeconds(30))); + + actor.Tell(new RedundancyStateChanged( + Nodes: new[] + { + new NodeRedundancyState(local, RedundancyRole.Primary, + IsClusterLeader: true, IsDriverPrimary: true, DateTime.UtcNow), + }, + CorrelationId.NewId())); + + AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), + duration: TimeSpan.FromMilliseconds(500)); + } + + /// DB-less, snapshot stale (entry.AsOfUtc older than the stale window) → 200. Staleness + /// is derived from snapshot age ALONE (no DB-health term), so a DB-less node still demotes to 200 + /// when it is running behind on config. + [Fact] + public void DbLess_stale_snapshot_publishes_200() + { + var publisher = new RecordingPublisher(); + var local = NodeId.Parse("db-less-stale"); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests( + serviceLevel: publisher, localNode: local, dbLess: true, + staleWindow: TimeSpan.FromSeconds(2))); + + actor.Tell(new RedundancyStateChanged( + Nodes: new[] + { + new NodeRedundancyState(local, RedundancyRole.Secondary, + IsClusterLeader: false, IsDriverPrimary: false, + DateTime.UtcNow - TimeSpan.FromMinutes(1)), + }, + CorrelationId.NewId())); + + AwaitAssert(() => publisher.Levels.ShouldContain((byte)200), + duration: TimeSpan.FromMilliseconds(500)); + } + + /// DB-less, Detached → 0. The Detached / missing-entry guard runs ABOVE the DB-less + /// branch, so a DB-less node that detaches still fails closed to 0. Goes healthy (250) first so + /// the transition down to 0 is observable (not dedup'd against the initial 0). + [Fact] + public void DbLess_detached_publishes_0() + { + var publisher = new RecordingPublisher(); + var local = NodeId.Parse("db-less-detached"); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests( + serviceLevel: publisher, localNode: local, dbLess: true, + staleWindow: TimeSpan.FromSeconds(30))); + + actor.Tell(new RedundancyStateChanged( + Nodes: new[] + { + new NodeRedundancyState(local, RedundancyRole.Primary, + IsClusterLeader: true, IsDriverPrimary: true, DateTime.UtcNow), + }, + CorrelationId.NewId())); + AwaitAssert(() => publisher.Levels.ShouldContain((byte)250), + duration: TimeSpan.FromMilliseconds(500)); + + actor.Tell(new RedundancyStateChanged( + Nodes: new[] + { + new NodeRedundancyState(local, RedundancyRole.Detached, + IsClusterLeader: false, IsDriverPrimary: false, DateTime.UtcNow), + }, + CorrelationId.NewId())); + + AwaitAssert(() => publisher.Levels.ShouldContain((byte)0), + duration: TimeSpan.FromMilliseconds(500)); + } + + /// Regression: a DB-BACKED node (probe wired + DbHealthStatus reachable, dbLess default + /// false) is unchanged — it still computes 240 via the calculator path. Proves the new DB-less + /// branch did not disturb the existing DB-backed seam. + [Fact] + public void DbBacked_node_unchanged_still_publishes_240() + { + var publisher = new RecordingPublisher(); + var local = NodeId.Parse("db-backed-node"); + var probe = Sys.ActorOf(Akka.Actor.Props.Create(() => + new StubDbHealth(new DbHealthProbeActor.DbHealthStatus(true, DateTime.UtcNow, null)))); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests( + serviceLevel: publisher, localNode: local, dbHealthProbe: probe)); + + actor.Tell(new DbHealthProbeActor.DbHealthStatus(true, DateTime.UtcNow, null)); + actor.Tell(new RedundancyStateChanged( + Nodes: new[] + { + new NodeRedundancyState(local, RedundancyRole.Secondary, + IsClusterLeader: false, IsDriverPrimary: false, DateTime.UtcNow), + }, + CorrelationId.NewId())); + + AwaitAssert(() => publisher.Levels.ShouldContain((byte)240), + duration: TimeSpan.FromMilliseconds(500)); + } + /// Stub DB-health probe actor that answers /// with a fixed status, so the calculator path is deterministic without a real timer. private sealed class StubDbHealth : Akka.Actor.ReceiveActor