From 77872b1d1c91eee82bc48cbb124dbb778c9dafeb Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 13:11:49 -0500 Subject: [PATCH] =?UTF-8?q?fix(runtime,galaxy):=20#869=20review=20round=20?= =?UTF-8?q?1=20=E2=80=94=20confirmation=20off=20the=20actor=20scheduler,?= =?UTF-8?q?=20stop/async-fault=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Start the ack confirmation with Task.Run + ConfigureAwait(false) so it never depends on the ReceiveAsync handler's ActorTaskScheduler (the PostStop abandon-and-revert report must run after the actor stopped). - DriverInstanceActorAckConfirmationTests: a confirmation that faults after the handler returned is reported; one pending at actor stop is reported; a confirmed ack reports nothing (positive control). - Deterministic tracker matching test (Raise/Clear/Retrigger/other alarm do not confirm; case-insensitive match does). - AdminUI min/max on the confirm-timeout field; Galaxy.md documents the reconnect, UNACK_RTN-as-Clear and RPC-budget limits. Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk --- docs/drivers/Galaxy.md | 10 ++ .../Drivers/Forms/GalaxyDriverForm.razor | 2 +- .../Drivers/DriverInstanceActor.cs | 14 +- .../Runtime/GalaxyAckConfirmationTests.cs | 33 +++- ...DriverInstanceActorAckConfirmationTests.cs | 152 ++++++++++++++++++ 5 files changed, 204 insertions(+), 7 deletions(-) create mode 100644 tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs diff --git a/docs/drivers/Galaxy.md b/docs/drivers/Galaxy.md index fb2b4734e..d7f70391b 100644 --- a/docs/drivers/Galaxy.md +++ b/docs/drivers/Galaxy.md @@ -162,6 +162,16 @@ e.g. `AckMsg write to '…' sent; OnWriteComplete not observed within 5000 ms.`) condition event with `AckedState = false`, and the `BadTimeout` verdict is recorded in the driver and host Warning logs. A confirming transition that arrives after the revert simply re-acknowledges the condition. A confirmation in flight when the driver actor stops is abandoned and reverted the same way. +- **Limits to know:** + - A gateway **reconnect** detaches the feed; an ack in flight across it is confirmed only if the new feed's + snapshot re-statement (`ActiveAcknowledged`) arrives inside the window, otherwise it reverts (fails safe). + - Acking an alarm that already **returned to normal** (`UNACK_RTN`) is confirmed by the worker's + `Acknowledge` (`ACK_RTN`) transition; if the gateway reconciles it as a `Clear` instead, nothing confirms + and the correctly-applied ack reverts to unacked. Unverified on hardware — include it in the next live gate. + - The RPC itself still runs under the actor's 5 s ack bound and the Tier-A `AlarmAcknowledge` timeout (5 s). + The gateway worker's write-completion window (`DefaultWriteCompletionTimeout`, 1.5 s) plus `AuthenticateUser` must stay + well under that, or the unconfirmed reply is never seen and the ack reverts as a timeout that also counts + toward the breaker. - `ConfirmTimeoutMs = 0` restores the pre-#869 behaviour (an unconfirmed ack counts as acknowledged, with a Warning) — an escape hatch only. Why the gateway-side ack did not stick on VD03 is tracked upstream (mxaccessgw#226). diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor index cdfe37893..9f41fbd84 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/GalaxyDriverForm.razor @@ -111,7 +111,7 @@
- +
When the gateway sends an ack but cannot confirm it, wait this long for Galaxy's acknowledged transition before reverting the condition to unacknowledged (#869). Default 10000; 0 = do not wait.
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs index 362d709b4..7be56023b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverInstanceActor.cs @@ -1376,8 +1376,16 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers // "sent but unconfirmed" OK). The confirmation wait is seconds long, so it runs OFF this mailbox and // OUTSIDE the resilience pipeline (a wait on an upstream event — it must neither be cut by the // AlarmAcknowledge capability timeout nor count toward the breaker). Failure reverts like any other. + // Task.Run, not a bare call: this handler is a ReceiveAsync, so TaskScheduler.Current is the actor's + // scheduler; the confirmation must not depend on this actor's context at all — in particular not after + // PostStop, when it still has to report the abandoned ack (pinned by DriverInstanceActorAckConfirmationTests). if (src is IAlarmAcknowledgeConfirmation confirmer) - _ = RunAckConfirmationAsync(confirmer, request, msg, replyTo, _ackConfirmCts.Token, _driverInstanceId, _log); + { + var stopToken = _ackConfirmCts.Token; + var driverInstanceId = _driverInstanceId; + var log = _log; + _ = Task.Run(() => RunAckConfirmationAsync(confirmer, request, msg, replyTo, stopToken, driverInstanceId, log)); + } } catch (DriverOperationRefusedException ex) { @@ -1412,10 +1420,10 @@ public sealed class DriverInstanceActor : ReceiveActor, IWithTimers IAlarmAcknowledgeConfirmation confirmer, IReadOnlyList request, RouteAlarmAck msg, IActorRef replyTo, CancellationToken stopToken, string driverInstanceId, ILoggingAdapter log) { - await Task.Yield(); try { - await confirmer.ConfirmAcknowledgedAsync(request, stopToken).WaitAsync(AckConfirmationCeiling, stopToken); + await confirmer.ConfirmAcknowledgedAsync(request, stopToken) + .WaitAsync(AckConfirmationCeiling, stopToken).ConfigureAwait(false); } catch (DriverOperationRefusedException ex) { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs index 2c0e1c34f..453f81dca 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Galaxy.Tests/Runtime/GalaxyAckConfirmationTests.cs @@ -111,9 +111,9 @@ public sealed class GalaxyAckConfirmationTests } /// - /// Unconfirmed OK + Galaxy's acknowledged transition on the feed → acknowledged. Positive control: the - /// confirmation is still pending after transitions that must NOT confirm (another alarm's ack, this - /// alarm's raise) — it only completes on the matching one. + /// Unconfirmed OK + Galaxy's acknowledged transition on the feed → acknowledged, end to end through the + /// driver's feed hook (non-matching transitions are emitted first; that they do NOT confirm is pinned + /// synchronously by ). /// [Fact] public async Task Unconfirmed_ack_is_confirmed_by_the_acknowledged_transition() @@ -246,6 +246,33 @@ public sealed class GalaxyAckConfirmationTests } } + /// + /// Tracker matching rules, checked synchronously: only an Acknowledge / ActiveAcknowledged transition of the + /// SAME alarm (case-insensitive) confirms. Positive control: the matching transition does flip it. + /// + [Fact] + public void Tracker_confirms_only_on_this_alarms_acknowledged_transition() + { + var tracker = new GalaxyAckConfirmationTracker(TimeSpan.FromSeconds(30), new CapturingLogger(), "drv"); + var watch = tracker.Begin(Ack(AlarmRef)[0], AlarmRef); + tracker.Arm(watch, UnconfirmedDiagnostic); + + foreach (var kind in new[] + { + GalaxyAlarmTransitionKind.Raise, GalaxyAlarmTransitionKind.Clear, + GalaxyAlarmTransitionKind.Retrigger, GalaxyAlarmTransitionKind.Unspecified, + }) + { + tracker.Observe(Transition(AlarmRef, kind)); + } + tracker.Observe(Transition("Other01.Level.Hi", GalaxyAlarmTransitionKind.Acknowledge)); + tracker.Observe(Transition("Other01.Level.Hi", GalaxyAlarmTransitionKind.ActiveAcknowledged)); + watch.IsConfirmed.ShouldBeFalse("no transition so far says THIS alarm is acknowledged"); + + tracker.Observe(Transition(AlarmRef.ToLowerInvariant(), GalaxyAlarmTransitionKind.Acknowledge)); + watch.IsConfirmed.ShouldBeTrue(); + } + [Fact] public void Options_default_to_ten_seconds_and_map_negative_to_the_default() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs new file mode 100644 index 000000000..8ec27749d --- /dev/null +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverInstanceActorAckConfirmationTests.cs @@ -0,0 +1,152 @@ +using System.Collections.Concurrent; +using Akka.Actor; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; +using ZB.MOM.WW.OtOpcUa.Runtime.Tests.Harness; + +namespace ZB.MOM.WW.OtOpcUa.Runtime.Tests.Drivers; + +/// +/// lmxopcua#869 — asks an driver to +/// confirm an ack AFTER returned, off its mailbox, and reports +/// to the router when the confirmation fails — including one that +/// fails long after the ack handler returned, and one abandoned because the actor stopped. +/// +public sealed class DriverInstanceActorAckConfirmationTests : RuntimeActorTestBase +{ + private static readonly TimeSpan Timeout = PresenceBudget; + + /// A confirmation that faults asynchronously, after the ack handler returned, is reported failed. + [Fact] + public void A_confirmation_that_fails_later_is_reported_as_a_failed_ack() + { + var driver = new ConfirmingDriver(); + var actor = SpawnConnected(driver); + var gate = new TaskCompletionSource(); + driver.Confirmation = () => gate.Task; + var router = CreateTestProbe(); + + actor.Tell(Ack("n1"), router.Ref); + AwaitCondition(() => driver.ConfirmCalls == 1, Timeout, TestContext.Current.CancellationToken); + gate.SetException(new DriverOperationRefusedException("not confirmed within 10 s (BadTimeout)")); + + var failed = router.ExpectMsg(Timeout, cancellationToken: TestContext.Current.CancellationToken); + failed.ConditionNodeId.ShouldBe("n1"); + failed.Reason.ShouldContain("BadTimeout"); + } + + /// + /// A confirmation still pending when the actor stops is abandoned AND reported failed, so the host reverts the + /// ack this incarnation can no longer confirm. (A confirmation continuation posted back to the stopped actor + /// would dead-letter and never report.) + /// + [Fact] + public void A_confirmation_pending_when_the_actor_stops_is_reported_as_a_failed_ack() + { + var driver = new ConfirmingDriver(); + var actor = SpawnConnected(driver); + driver.Confirmation = () => new TaskCompletionSource().Task; // never completes on its own + var router = CreateTestProbe(); + + actor.Tell(Ack("n-stop"), router.Ref); + AwaitCondition(() => driver.ConfirmCalls == 1, Timeout, TestContext.Current.CancellationToken); + Sys.Stop(actor); + + var failed = router.ExpectMsg(Timeout, cancellationToken: TestContext.Current.CancellationToken); + failed.ConditionNodeId.ShouldBe("n-stop"); + failed.Reason.ShouldContain("stopped"); + } + + /// + /// A confirmation that succeeds reports nothing. Positive control: a later ack whose confirmation fails is + /// reported, and it is the FIRST report the router receives — so none came for the confirmed one. + /// + [Fact] + public void A_confirmed_ack_reports_nothing() + { + var driver = new ConfirmingDriver(); + var actor = SpawnConnected(driver); + var router = CreateTestProbe(); + + driver.Confirmation = () => Task.Delay(20); + actor.Tell(Ack("n-ok"), router.Ref); + AwaitCondition(() => driver.ConfirmCalls == 1, Timeout, TestContext.Current.CancellationToken); + + driver.Confirmation = () => Task.FromException(new DriverOperationRefusedException("unconfirmed")); + actor.Tell(Ack("n-bad"), router.Ref); + + router.ExpectMsg(Timeout, cancellationToken: TestContext.Current.CancellationToken).ConditionNodeId.ShouldBe("n-bad"); + } + + private static DriverInstanceActor.RouteAlarmAck Ack(string conditionNodeId) => + new("Tank.Hi", "c", "alice", ConditionNodeId: conditionNodeId); + + /// Spawns the actor as a child of the test actor and waits for its Connected announcement. + private IActorRef SpawnConnected(ConfirmingDriver driver) + { + var actor = ChildActorOf(DriverInstanceActor.Props(driver)); + actor.Tell(new DriverInstanceActor.InitializeRequested("{}")); + FishForMessage(c => c.Connected, Timeout); + return actor; + } + + private sealed class ConfirmingDriver : IDriver, IAlarmSource, IAlarmAcknowledgeConfirmation + { + private int _ackCalls; + private int _confirmCalls; + private readonly ConcurrentQueue _acks = new(); + + public Func? Confirmation { get; set; } + + public int AckCalls => Volatile.Read(ref _ackCalls); + + public int ConfirmCalls => Volatile.Read(ref _confirmCalls); + + public string DriverInstanceId => "drv-confirm"; + + public string DriverType => "Test"; + + public Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask; + + public Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken) => Task.CompletedTask; + + public Task ShutdownAsync(CancellationToken cancellationToken) => Task.CompletedTask; + + public DriverHealth GetHealth() => new(DriverState.Healthy, null, null); + + public Task SubscribeAlarmsAsync( + IReadOnlyList sourceNodeIds, CancellationToken cancellationToken) => + Task.FromResult(new Handle()); + + public Task UnsubscribeAlarmsAsync(IAlarmSubscriptionHandle handle, CancellationToken cancellationToken) => + Task.CompletedTask; + + public Task AcknowledgeAsync( + IReadOnlyList acknowledgements, CancellationToken cancellationToken) + { + foreach (var a in acknowledgements) _acks.Enqueue(a); + Interlocked.Increment(ref _ackCalls); + return Task.CompletedTask; + } + + public Task ConfirmAcknowledgedAsync( + IReadOnlyList acknowledgements, CancellationToken cancellationToken) + { + Interlocked.Increment(ref _confirmCalls); + return (Confirmation?.Invoke() ?? Task.CompletedTask).WaitAsync(cancellationToken); + } + + public event EventHandler? OnAlarmEvent + { + add { } + remove { } + } + + private sealed class Handle : IAlarmSubscriptionHandle + { + public string DiagnosticId => "h"; + } + } +}