fix(opcuaclient): #670 F2 re-gate — hold the replay guard for the whole window

The F2 live gate failed (10 value regressions over 4 blips): IsLateReplay
disarmed a ref on its first in-order notification, which arrives ~1 s after
recovery, while the SDK's late replay lands 4-6 s later. Equal source times
also passed, so a replay equal to the recovery read got through.

While the window is open each ref now carries a high-water mark (seeded at
arming, raised by the recovery read, advanced by every accepted notification);
a notification at or below it is dropped (equal included — OPC UA moves the
source timestamp on every value/status change). Null source time passes
without moving the mark. Window expiry still disarms entirely, so an upstream
clock step-back is never frozen outside the window. Arm/expiry serialised by a
lock so an expiry racing a re-arm cannot wipe fresh seeds.
This commit is contained in:
Joseph Doherty
2026-10-11 09:42:39 -04:00
parent 57deaeba16
commit 099bc1b6e0
5 changed files with 146 additions and 33 deletions
@@ -14,7 +14,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient.Tests;
[Trait("Category", "Unit")]
public sealed class OpcUaClientDiagnosticsTests
{
private sealed class Capture : IDisposable
internal sealed class Capture : IDisposable
{
private readonly MeterListener _listener = new();
private readonly object _gate = new();
@@ -46,9 +46,10 @@ public sealed class OpcUaClientSessionLossTests
return mock;
}
private static (OpcUaClientDriver Driver, Mock<ISession> Session, ManualClock Clock) NewConnectedDriver()
private static (OpcUaClientDriver Driver, Mock<ISession> Session, ManualClock Clock) NewConnectedDriver(
string driverInstanceId = "opcua-670")
{
var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions { RawTags = Tags }, "opcua-670");
var drv = new OpcUaClientDriver(new OpcUaClientDriverOptions { RawTags = Tags }, driverInstanceId);
var session = NewSession();
var clock = new ManualClock(new DateTimeOffset(2026, 10, 11, 12, 0, 0, TimeSpan.Zero));
drv.Clock = clock;
@@ -518,11 +519,11 @@ public sealed class OpcUaClientSessionLossTests
/// <summary>
/// #670 follow-up F2 (live gate): after a same-session recovery the SDK delivered one notification late with
/// an OLDER source time, and the node regressed to it. Inside the post-recovery replay window an older
/// notification is dropped; an equal one (a status-only change) passes and disarms the ref, after which the
/// ref is back to accepting anything.
/// notification is dropped, and so is one carrying the exact source time already accepted (OPC UA moves the
/// source timestamp on every value OR status change, so an equal one is a replay, not a status-only change).
/// </summary>
[Fact]
public async Task After_a_recovery_a_late_older_notification_is_dropped_until_the_ref_is_back_in_order()
public async Task After_a_recovery_a_late_older_or_equal_notification_is_dropped()
{
var (drv, session, clock) = NewConnectedDriver();
using var _ = drv;
@@ -536,12 +537,93 @@ public sealed class OpcUaClientSessionLossTests
drv.DeliverNotificationForTest(handle, SpeedRef, Good(667, t0.AddSeconds(1))); // late replay
drv.DeliverNotificationForTest(handle, SpeedRef,
new DataValue(new Variant(668), StatusCodes.UncertainLastUsableValue, t0.AddSeconds(2), t0.AddSeconds(2))); // same ts, status-only
new DataValue(new Variant(668), StatusCodes.UncertainLastUsableValue, t0.AddSeconds(2), t0.AddSeconds(2))); // same ts
drv.DeliverNotificationForTest(handle, SpeedRef, Good(669, t0.AddSeconds(3)));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(665, t0)); // disarmed: accepted
drv.DeliverNotificationForTest(handle, SpeedRef, Good(665, t0)); // still guarded
seen.Select(e => e.Snapshot.Value).ShouldBe(new object[] { 668, 669, 665 });
seen[0].Snapshot.StatusCode.ShouldBe((uint)StatusCodes.UncertainLastUsableValue);
seen.Select(e => e.Snapshot.Value).ShouldBe(new object[] { 669 });
}
/// <summary>
/// #670 F2 re-gate (live gate FAILED on the per-ref disarm): fresh notifications arrived ~1 s after recovery
/// and disarmed each ref, then the SDK's late replay arrived 4–6 s later and regressed the node (value 670 with
/// a pre-outage source time delivered after newer values). The guard must hold for the WHOLE window against the
/// newest ACCEPTED source time per ref — older, equal-to-the-recovery-read and equal-to-the-last-accepted
/// replays are all dropped, and counted.
/// </summary>
[Fact]
public async Task A_late_replay_after_a_fresh_in_order_notification_is_still_dropped_inside_the_window()
{
var instance = $"opcua-670-regate-{Guid.NewGuid():N}";
using var drops = new OpcUaClientDiagnosticsTests.Capture(instance);
var (drv, session, clock) = NewConnectedDriver(instance);
using var _ = drv;
var handle = drv.AddSubscriptionForTest([SpeedRef]);
var t0 = clock.Now.UtcDateTime;
drv.DeliverNotificationForTest(handle, SpeedRef, Good(100, t0.AddSeconds(-5)));
SetupReadAt(session, 105, t0); // recovery read seeds T0
await RecoverInPlaceAsync(drv);
var seen = new List<DataChangeEventArgs>();
drv.OnDataChange += (_, e) => seen.Add(e);
clock.Advance(TimeSpan.FromSeconds(1));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(116, t0.AddSeconds(6))); // fresh, in order: T1
clock.Advance(TimeSpan.FromSeconds(5));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(109, t0.AddSeconds(3))); // late replay, older than T1
drv.DeliverNotificationForTest(handle, SpeedRef, Good(105, t0)); // replay == T0 (recovery read)
drv.DeliverNotificationForTest(handle, SpeedRef, Good(116, t0.AddSeconds(6))); // replay == T1
drv.DeliverNotificationForTest(handle, SpeedRef, Good(670, t0.AddSeconds(-10))); // pre-outage replay
drv.DeliverNotificationForTest(handle, SpeedRef, Good(117, t0.AddSeconds(7))); // fresh again
seen.Select(e => e.Snapshot.Value).ShouldBe(new object[] { 116, 117 });
drops[OpcUaClientDiagnostics.StaleNotificationDropped].ShouldBe(4);
}
/// <summary>
/// The re-gated guard still expires: after the window an older source time (an upstream clock step-back) is
/// accepted, even though the ref accepted newer notifications inside the window.
/// </summary>
[Fact]
public async Task After_the_window_a_clock_step_back_is_accepted_even_after_in_window_notifications()
{
var (drv, session, clock) = NewConnectedDriver();
using var _ = drv;
var handle = drv.AddSubscriptionForTest([SpeedRef]);
var t0 = clock.Now.UtcDateTime;
drv.DeliverNotificationForTest(handle, SpeedRef, Good(1, t0.AddSeconds(-5)));
SetupReadAt(session, 2, t0);
await RecoverInPlaceAsync(drv);
var seen = new List<DataChangeEventArgs>();
drv.OnDataChange += (_, e) => seen.Add(e);
drv.DeliverNotificationForTest(handle, SpeedRef, Good(3, t0.AddSeconds(20)));
clock.Advance(TimeSpan.FromSeconds(31));
drv.DeliverNotificationForTest(handle, SpeedRef, Good(4, t0.AddSeconds(1))); // clock stepped back
drv.DeliverNotificationForTest(handle, SpeedRef, Good(5, t0)); // and again
seen.Select(e => e.Snapshot.Value).ShouldBe(new object[] { 3, 4, 5 });
}
/// <summary>A notification with no source timestamp passes through the open guard and does not move it.</summary>
[Fact]
public async Task A_notification_without_a_source_time_passes_the_guard_and_does_not_advance_it()
{
var (drv, session, clock) = NewConnectedDriver();
using var _ = drv;
var handle = drv.AddSubscriptionForTest([SpeedRef]);
var t0 = clock.Now.UtcDateTime;
drv.DeliverNotificationForTest(handle, SpeedRef, Good(1, t0.AddSeconds(-5)));
SetupReadAt(session, 2, t0);
await RecoverInPlaceAsync(drv);
var seen = new List<DataChangeEventArgs>();
drv.OnDataChange += (_, e) => seen.Add(e);
drv.DeliverNotificationForTest(handle, SpeedRef,
new DataValue(new Variant(3), StatusCodes.Good, DateTime.MinValue, t0)); // no source time
drv.DeliverNotificationForTest(handle, SpeedRef, Good(4, t0.AddSeconds(-1))); // still guarded at T0
drv.DeliverNotificationForTest(handle, SpeedRef, Good(5, t0.AddSeconds(1)));
seen.Select(e => e.Snapshot.Value).ShouldBe(new object[] { 3, 5 });
}
/// <summary>