feat(mesh-phase4): retire DB-reachability from ServiceLevel on DB-less nodes
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -103,6 +103,11 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers
|
||||
private readonly IDbContextFactory<OtOpcUaConfigDbContext>? _dbFactory;
|
||||
private readonly AddressSpaceApplier? _applier;
|
||||
private readonly IActorRef? _dbHealthProbe;
|
||||
/// <summary>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 —
|
||||
/// <c>DbReachable</c> is fed constant <c>true</c> and only snapshot staleness can demote. See
|
||||
/// <see cref="RecomputeServiceLevel"/>.</summary>
|
||||
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
|
||||
/// <param name="healthTickInterval">The period between self-driven DB-health refresh ticks (each
|
||||
/// Asks <paramref name="dbHealthProbe"/> for its cached status); defaults to 5 seconds. No timer is
|
||||
/// started when <paramref name="dbHealthProbe"/> is null.</param>
|
||||
/// <param name="dbLess">Per-cluster mesh Phase 4: this is a driver-only node with no ConfigDb, so the
|
||||
/// ServiceLevel DB-reachability input is retired (<c>DbReachable</c> treated constant-true, only
|
||||
/// staleness demotes). No <paramref name="dbHealthProbe"/> is wired on such a node.</param>
|
||||
/// <returns>The configured <see cref="Props"/> for creating this actor, pinned to the
|
||||
/// <see cref="DispatcherId"/> dispatcher.</returns>
|
||||
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);
|
||||
|
||||
/// <summary>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.</summary>
|
||||
@@ -195,6 +205,9 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers
|
||||
/// <param name="healthTickInterval">The period between self-driven DB-health refresh ticks (each
|
||||
/// Asks <paramref name="dbHealthProbe"/> for its cached status); defaults to 5 seconds. No timer is
|
||||
/// started when <paramref name="dbHealthProbe"/> is null.</param>
|
||||
/// <param name="dbLess">Per-cluster mesh Phase 4: this is a driver-only node with no ConfigDb, so the
|
||||
/// ServiceLevel DB-reachability input is retired (<c>DbReachable</c> treated constant-true, only
|
||||
/// staleness demotes). No <paramref name="dbHealthProbe"/> is wired on such a node.</param>
|
||||
/// <returns>The configured <see cref="Props"/> for creating this actor in tests, without dispatcher
|
||||
/// pinning or the DPS subscribe.</returns>
|
||||
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));
|
||||
|
||||
/// <summary>Initializes a new instance of the <see cref="OpcUaPublishActor"/> class.</summary>
|
||||
/// <param name="sink">The OPC UA address space sink.</param>
|
||||
@@ -237,6 +252,9 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers
|
||||
/// <param name="healthTickInterval">The period between self-driven DB-health refresh ticks (each
|
||||
/// Asks <paramref name="dbHealthProbe"/> for its cached status); defaults to 5 seconds. No timer is
|
||||
/// started when <paramref name="dbHealthProbe"/> is null.</param>
|
||||
/// <param name="dbLess">Per-cluster mesh Phase 4: this is a driver-only node with no ConfigDb, so the
|
||||
/// ServiceLevel DB-reachability input is retired (<c>DbReachable</c> treated constant-true, only
|
||||
/// staleness demotes). No <paramref name="dbHealthProbe"/> is wired on such a node.</param>
|
||||
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.
|
||||
|
||||
@@ -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<IHistoryWriter>() ?? NullHistoryWriter.Instance;
|
||||
|
||||
var dbHealth = system.ActorOf(
|
||||
DbHealthProbeActor.Props(dbFactory),
|
||||
DbHealthProbeActorName);
|
||||
registry.Register<DbHealthProbeActorKey>(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<DbHealthProbeActorKey>(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<OpcUaPublishActorKey>(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,
|
||||
|
||||
@@ -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.
|
||||
|
||||
/// <summary>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.</summary>
|
||||
[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));
|
||||
}
|
||||
|
||||
/// <summary>DB-less, member Up, snapshot fresh, Primary → 250 (basis 240 + 10 Primary bonus).</summary>
|
||||
[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));
|
||||
}
|
||||
|
||||
/// <summary>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.</summary>
|
||||
[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));
|
||||
}
|
||||
|
||||
/// <summary>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).</summary>
|
||||
[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));
|
||||
}
|
||||
|
||||
/// <summary>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.</summary>
|
||||
[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));
|
||||
}
|
||||
|
||||
/// <summary>Stub DB-health probe actor that answers <see cref="DbHealthProbeActor.GetStatus"/>
|
||||
/// with a fixed status, so the calculator path is deterministic without a real timer.</summary>
|
||||
private sealed class StubDbHealth : Akka.Actor.ReceiveActor
|
||||
|
||||
Reference in New Issue
Block a user