From bdc81a13726f8389a40f55be0620cf1a86821ea9 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 20 Aug 2026 09:34:49 -0400 Subject: [PATCH] refactor(runtime): one desired-subscription payload authority (_desiredByDriver) --- ...R3-03-restart-resubscribe-watchdog-plan.md | 17 +++++ ...rt-resubscribe-watchdog-plan.md.tasks.json | 2 +- .../Drivers/DriverHostActor.cs | 68 ++++++++++++++++--- 3 files changed, 75 insertions(+), 12 deletions(-) diff --git a/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md b/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md index 4a9966cb..c74e839d 100644 --- a/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md +++ b/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md @@ -487,3 +487,20 @@ the reciprocal falsification-partner xmldoc on the existing anticipated, each of its trip cycles now costs a full `FastTimeout` (250 ms) instead of one 50 ms tick, and its `AwaitCondition(InitializeCount >= 2, 5 s)` still has ample headroom. No budget change was needed. + +### T2 — desired-subscription payload authority + +Behaviour-preserving refactor, so there is no red observation to record by design — the gate is that +**no test moves**. `--filter "FullyQualifiedName~DriverHostActor"` → **114/114 passed**, unchanged +from the base commit. + +⚠️ One run of that filter reported `DriverHostActorNativeAlarmAckRoutingTests.RouteNativeAlarmAck_on_non_primary_is_dropped` +failing on an `AwaitAssert`. Re-run in isolation: **3/3 passed**; re-run of the whole 114-test filter: +**114/114 passed**. Treated as the known #500 flake set for this assembly (~13 %/run), not a +regression — it touches none of the code this task edits (the alarm-ack Primary gate is a different +method entirely). + +Deviation from the plan's literal step text (cosmetic, recorded for the reviewer): the plan's Finding-1 +step 1 says `_desiredByDriver` *replaces* `_lastComposition` at `:347-349` while T3 step 2 says T3 +deletes `_lastComposition`. Keeping T2 compilable means the two fields coexist for one commit; +`_lastComposition` and its assignment are deleted in T3 as T3's own step text specifies. diff --git a/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md.tasks.json b/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md.tasks.json index 2464790a..582bcf35 100644 --- a/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md.tasks.json +++ b/archreview/plans/R3-03-restart-resubscribe-watchdog-plan.md.tasks.json @@ -11,7 +11,7 @@ { "id": "T2", "subject": "Desired-subscription payload authority: _desiredByDriver + SendDesiredSubscriptions (behaviour-preserving refactor)", - "status": "pending", + "status": "done", "blockedBy": [] }, { diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index e95ca092..ff212655 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -348,6 +348,22 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS /// ). Null until the first apply. private AddressSpaceComposition? _lastComposition; + /// One driver child's desired subscription payload — exactly the + /// the apply's bulk push sent it. + /// The non-alarm raw tags' RawPaths this driver should keep subscribed. + /// The alarm-bearing raw tags' RawPaths that un-gate this driver's + /// feed. + private readonly record struct DesiredSubscriptionSet( + IReadOnlyList ValueRefs, + IReadOnlyList AlarmRefs); + + /// Per-driver desired subscription sets from the most-recent apply. THE authority: the bulk + /// push, and the supervision-reprovision handler all send from here, so a + /// child obtained by ANY route carries the same set. Cleared and repopulated whole on every apply, + /// alongside the routing maps, so renames/removals can never leave a stale entry. Empty until the + /// first apply — a child spawned before any apply is handed nothing, which is what it should have. + private readonly Dictionary _desiredByDriver = new(StringComparer.Ordinal); + /// /// Cached local from the latest /// snapshot (null = unknown until the first snapshot arrives, or no local node match). The Primary @@ -2652,23 +2668,25 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS } } - // One authored-only push (value refs + alarm refs from the maps built above), shared by the bulk loop AND - // the dropped-driver fallback so the two CANNOT drift: the fallback's correctness depends on sending the - // SAME payload the bulk loop would have, so it's a shared helper (structural), not a comment-maintained - // invariant. An EMPTY set is valid — the child's Connected handler routes it to Unsubscribe (dropping a - // stale handle) rather than a spurious subscribe. Returns the value-ref count for the bulk-loop log total. - int SendAuthoredOnly(IActorRef actor, string driverId) + // Cache the per-driver payload as THE authority, in the same clear-and-repopulate pass as the routing + // maps above so a rename/removal can never leave a stale entry behind. Keyed over the UNION of the two + // ref maps AND the running children: a child with no authored tags must get an explicit EMPTY set (which + // it routes to Unsubscribe), not be absent from the authority — absence and empty must not differ here. + _desiredByDriver.Clear(); + foreach (var driverId in refsByDriver.Keys + .Concat(alarmRefsByDriver.Keys) + .Concat(_children.Keys) + .Distinct(StringComparer.Ordinal)) { - var refs = refsByDriver.TryGetValue(driverId, out var r) ? r : Array.Empty(); - var alarmRefs = alarmRefsByDriver.TryGetValue(driverId, out var ar) ? ar : Array.Empty(); - actor.Tell(new DriverInstanceActor.SetDesiredSubscriptions(refs, SubscriptionPublishingInterval, alarmRefs)); - return refs.Count; + _desiredByDriver[driverId] = new DesiredSubscriptionSet( + refsByDriver.TryGetValue(driverId, out var r) ? r : Array.Empty(), + alarmRefsByDriver.TryGetValue(driverId, out var ar) ? ar : Array.Empty()); } var total = 0; foreach (var (driverId, entry) in _children) { - total += SendAuthoredOnly(entry.Actor, driverId); + total += SendDesiredSubscriptions(entry.Actor, driverId); } if (total > 0) @@ -2710,6 +2728,34 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers, IWithUnboundedS } + /// + /// Send 's desired subscription payload to . + /// + /// + /// The ONE place a is built, so the apply's + /// bulk push, a respawn () and a supervision reprovision + /// (HandleReprovisionRequested) cannot drift. That is structural rather than + /// comment-maintained on purpose: a second sender that assembled its own payload is exactly how + /// 03/S16 happened — the restart path got a child without the set that makes a child useful. + /// + /// An EMPTY set is valid and MEANINGFUL — the child routes it to Unsubscribe (dropping a + /// stale handle) rather than to a spurious subscribe — so a driver missing from + /// is sent an empty set, never skipped. + /// + /// + /// The driver child to hand the set to. + /// The driver instance whose set to send. + /// The value-ref count, for the bulk loop's log total. + private int SendDesiredSubscriptions(IActorRef actor, string driverId) + { + var set = _desiredByDriver.TryGetValue(driverId, out var s) + ? s + : new DesiredSubscriptionSet(Array.Empty(), Array.Empty()); + actor.Tell(new DriverInstanceActor.SetDesiredSubscriptions( + set.ValueRefs, SubscriptionPublishingInterval, set.AlarmRefs)); + return set.ValueRefs.Count; + } + private void SpawnChild(DriverInstanceSpec spec) { var stub = DriverInstanceActor.ShouldStub(spec.DriverType, _localRoles);