fix(opcua): scope the deferred-write replay to nodes the apply created (Lane B fix round 2)
N1: a full alarm-state snapshot also sets the condition's Quality, so it now supersedes an earlier quality-only write for the same id in the deferred buffer; a quality arriving after the state is kept and replays after it. N2 / M1: the replay is not inert on a node the apply KEPT - an SDK-applied Acknowledge/Confirm/Shelve or an optimistic client Write in the window would be rolled back (with spurious Part 9 events). The replay is now scoped to nodes the apply created: variables the applier ensured during this apply (AddressSpaceApplier.BeginRecordingEnsuredVariables) or every variable after a full rebuild; conditions in the applied composition but not the previous one (also on a rebuild, which carries a kept condition's state over). Option (b), a sink that reports whether a live write landed, needs the Commons DeferredAddressSpaceSink seam outside this lane (follow-up F7). M2: the benchmark remark now matches its Category=Benchmark trait. M3: TestHooks and the seam-carrying constructor are internal; PropsForTests produces the actor through an IIndirectActorProducer. M4: pinned replay-nothing-and-drop-the-buffer after a failed plan. M5: PreRestart answers every queued rebuild requester (degraded). Refs #732 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
This commit is contained in:
Joseph Doherty
+2
-1
@@ -20,7 +20,8 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests;
|
||||
/// </list>
|
||||
/// The wall-clock budgets are generous for CI hardware; the before/after numbers are in the commit
|
||||
/// message. P3 is pinned by the bookkeeping's own visit counter, which does not depend on the machine.
|
||||
/// Kept in the default run on purpose (not a Benchmark category the CI filter would have to learn to exclude):
|
||||
/// Tagged <c>Category=Benchmark</c>, but no filter excludes that category (CI drops only E2E + LiveIntegration),
|
||||
/// so it runs by default — on purpose:
|
||||
/// measured after the fix at 5–6 ms (P1) and 58–64 ms (P2) on a loaded dev machine against the 1 s budget —
|
||||
/// ≥ 15× headroom, so it does not flake on slow CI. The flip side: the pre-fix code (75–150 ms / 470–680 ms) also
|
||||
/// fits 1 s at these sizes, so the wall-clock legs guard against gross (order-of-magnitude) regressions only; the
|
||||
|
||||
+229
-6
@@ -235,6 +235,196 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
|
||||
sink.Exists(NewTag).ShouldBeTrue();
|
||||
}
|
||||
|
||||
// ───────────────────── fix round 2: N1 / N2 / M1 / M4 / M5 ─────────────────────
|
||||
|
||||
/// <summary>N1 — a full state snapshot carries quality, so it supersedes an EARLIER quality-only write for the
|
||||
/// same condition (the engine sends a transition with the recovered worst quality and no QualityChanged).</summary>
|
||||
[Fact]
|
||||
public void A_state_after_a_quality_only_write_supersedes_it_on_replay()
|
||||
{
|
||||
var sink = new ExistenceSink();
|
||||
using var gate = new ManualResetEventSlim(false);
|
||||
var actor = Spawn(sink, gate);
|
||||
|
||||
actor.Tell(Rebuild(AlarmArtifact()));
|
||||
actor.Tell(new OpcUaPublishActor.AlarmQualityUpdate("al-1", OpcUaStatus.BadNoCommunication, DateTime.UtcNow, AddressSpaceRealm.Uns));
|
||||
actor.Tell(new OpcUaPublishActor.AlarmStateUpdate("al-1", ActiveUnacked(OpcUaStatus.Good), DateTime.UtcNow, AddressSpaceRealm.Uns));
|
||||
AwaitAssert(() => sink.SkippedAlarms.Count.ShouldBe(2), PlanBudget);
|
||||
gate.Set();
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
|
||||
|
||||
sink.AlarmQualityOf("al-1").ShouldBe(OpcUaStatus.Good, "the later snapshot's quality is what live would leave");
|
||||
sink.AlarmStateOf("al-1")!.Active.ShouldBeTrue();
|
||||
}
|
||||
|
||||
/// <summary>N1 positive control — the reverse order: a quality-only write AFTER the state still lands last.</summary>
|
||||
[Fact]
|
||||
public void A_quality_only_write_after_a_state_still_lands_after_it_on_replay()
|
||||
{
|
||||
var sink = new ExistenceSink();
|
||||
using var gate = new ManualResetEventSlim(false);
|
||||
var actor = Spawn(sink, gate);
|
||||
|
||||
actor.Tell(Rebuild(AlarmArtifact()));
|
||||
actor.Tell(new OpcUaPublishActor.AlarmStateUpdate("al-1", ActiveUnacked(OpcUaStatus.Good), DateTime.UtcNow, AddressSpaceRealm.Uns));
|
||||
actor.Tell(new OpcUaPublishActor.AlarmQualityUpdate("al-1", OpcUaStatus.BadNoCommunication, DateTime.UtcNow, AddressSpaceRealm.Uns));
|
||||
AwaitAssert(() => sink.SkippedAlarms.Count.ShouldBe(2), PlanBudget);
|
||||
gate.Set();
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
|
||||
|
||||
sink.AlarmQualityOf("al-1").ShouldBe(OpcUaStatus.BadNoCommunication);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// N2 — a condition the plan KEEPS took the live write already; replaying it would roll back an operator's
|
||||
/// Acknowledge applied in the window (the SDK mutates the node directly) and fire spurious events. Driven over
|
||||
/// both apply shapes: a surgical pure-add, and a full rebuild (which carries the condition's state over).
|
||||
/// </summary>
|
||||
[Theory]
|
||||
[InlineData(false)]
|
||||
[InlineData(true)]
|
||||
public void An_out_of_band_ack_on_a_kept_condition_is_not_reverted_by_the_replay(bool rebuild)
|
||||
{
|
||||
var sink = new ExistenceSink();
|
||||
using var gate = new ManualResetEventSlim(true);
|
||||
var actor = Spawn(sink, gate);
|
||||
actor.Tell(Rebuild(AlarmArtifact()));
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
|
||||
sink.ConditionExists("al-1").ShouldBeTrue();
|
||||
|
||||
gate.Reset();
|
||||
// A surgical pure-add (new raw tags) or a full rebuild (an equipment rename) that keeps al-1.
|
||||
actor.Tell(Rebuild(rebuild ? AlarmArtifact(equipmentName: "Eq-renamed") : AlarmArtifact(rawTags: 2)));
|
||||
actor.Tell(new OpcUaPublishActor.AlarmStateUpdate("al-1", ActiveUnacked(OpcUaStatus.Good), DateTime.UtcNow, AddressSpaceRealm.Uns));
|
||||
AwaitAssert(() => sink.AlarmWritesTo("al-1").ShouldBe(1), PlanBudget); // the live write landed
|
||||
sink.AcknowledgeOutOfBand("al-1"); // an operator acks in the window
|
||||
|
||||
gate.Set();
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
|
||||
|
||||
sink.SawRebuild.ShouldBe(rebuild, "non-vacuity: the intended apply shape ran");
|
||||
sink.AlarmStateOf("al-1")!.Acknowledged.ShouldBeTrue("the operator's ack must survive the apply");
|
||||
sink.AlarmWritesTo("al-1").ShouldBe(1, "no replayed write (and so no extra Part 9 event) on a kept condition");
|
||||
}
|
||||
|
||||
/// <summary>M1 — a variable the surgical plan keeps took the live write; replaying it would revert a client
|
||||
/// Write applied in the window. Positive control in the same apply: the node the plan ADDS gets its value.</summary>
|
||||
[Fact]
|
||||
public void A_surgical_apply_replays_values_only_onto_the_variables_it_added()
|
||||
{
|
||||
var sink = new ExistenceSink();
|
||||
using var gate = new ManualResetEventSlim(true);
|
||||
var actor = Spawn(sink, gate);
|
||||
actor.Tell(Rebuild(RawTagArtifact(1)));
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
|
||||
const string kept = "Plant/Modbus/dev1/T0";
|
||||
|
||||
gate.Reset();
|
||||
actor.Tell(Rebuild(RawTagArtifact(3)));
|
||||
var at = DateTime.UtcNow;
|
||||
actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate(
|
||||
[
|
||||
new ValueWrite(kept, 5, OpcUaStatus.Good, at, AddressSpaceRealm.Raw),
|
||||
new ValueWrite(NewTag, 7, OpcUaStatus.Good, at, AddressSpaceRealm.Raw),
|
||||
]));
|
||||
AwaitAssert(() => sink.ValueOf(kept)!.Value.Value.ShouldBe(5), PlanBudget);
|
||||
sink.SetValueOutOfBand(kept, 9); // a client Write in the window
|
||||
|
||||
gate.Set();
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
|
||||
|
||||
sink.SawRebuild.ShouldBeFalse("non-vacuity: a surgical apply");
|
||||
sink.ValueOf(kept)!.Value.Value.ShouldBe(9, "the kept node's client write is not reverted");
|
||||
sink.ValueOf(NewTag)!.Value.Value.ShouldBe(7, "positive control: the added node gets its deferred value");
|
||||
}
|
||||
|
||||
/// <summary>M1 — a FULL rebuild recreates every variable, so every deferred value replays (the live write landed
|
||||
/// on a node the rebuild discarded).</summary>
|
||||
[Fact]
|
||||
public void A_rebuild_replays_every_deferred_value()
|
||||
{
|
||||
var sink = new ExistenceSink();
|
||||
using var gate = new ManualResetEventSlim(true);
|
||||
var actor = Spawn(sink, gate);
|
||||
actor.Tell(Rebuild(AlarmArtifact(rawTags: 1)));
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
|
||||
const string kept = "Plant/Modbus/dev1/T0";
|
||||
sink.Exists(kept).ShouldBeTrue();
|
||||
|
||||
gate.Reset();
|
||||
// An equipment rename is a ChangedEquipment delta ⇒ full rebuild; the raw tag keeps its path.
|
||||
actor.Tell(Rebuild(AlarmArtifact(equipmentName: "Eq-renamed", rawTags: 1)));
|
||||
var at = DateTime.UtcNow;
|
||||
actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate([new ValueWrite(kept, 5, OpcUaStatus.Good, at, AddressSpaceRealm.Raw)]));
|
||||
AwaitAssert(() => sink.ValueOf(kept)!.Value.Value.ShouldBe(5), PlanBudget);
|
||||
|
||||
gate.Set();
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
|
||||
|
||||
sink.SawRebuild.ShouldBeTrue("non-vacuity: a full rebuild ran");
|
||||
sink.ValueOf(kept)!.Value.Value.ShouldBe(5, "the recreated node gets the value its predecessor took");
|
||||
}
|
||||
|
||||
/// <summary>M4 — a FAILED plan applies nothing, so nothing is replayed, and the buffer is dropped once the
|
||||
/// pipeline is idle: a later plan that adds the node does not receive a write from the failed window.</summary>
|
||||
[Fact]
|
||||
public void A_failed_plan_replays_nothing_and_drops_the_buffer()
|
||||
{
|
||||
var sink = new ExistenceSink();
|
||||
using var gate = new ManualResetEventSlim(true);
|
||||
var actor = Spawn(sink, gate);
|
||||
actor.Tell(Rebuild(RawTagArtifact(2)));
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
|
||||
|
||||
gate.Reset();
|
||||
actor.Tell(Rebuild("not json"u8.ToArray()));
|
||||
actor.Tell(new OpcUaPublishActor.AttributeValueBatchUpdate(
|
||||
[new ValueWrite("Plant/Modbus/dev1/T2", 42, OpcUaStatus.Good, DateTime.UtcNow, AddressSpaceRealm.Raw)]));
|
||||
AwaitAssert(() => sink.SkippedWrites.ShouldContain("Plant/Modbus/dev1/T2"), PlanBudget);
|
||||
// Snapshot AFTER the live write (the sink's counter cannot tell a live write after an apply from a replay).
|
||||
var replayedBefore = sink.ReplayedItems;
|
||||
gate.Set();
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeTrue();
|
||||
sink.ReplayedItems.ShouldBe(replayedBefore, "nothing was applied, so nothing is replayed");
|
||||
|
||||
actor.Tell(Rebuild(RawTagArtifact(3)));
|
||||
ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget).Degraded.ShouldBeFalse();
|
||||
sink.Exists("Plant/Modbus/dev1/T2").ShouldBeTrue("positive control: the later plan adds it");
|
||||
sink.ValueOf("Plant/Modbus/dev1/T2").ShouldBeNull("the failed window's buffer was dropped");
|
||||
}
|
||||
|
||||
/// <summary>M5 — a restart answers every QUEUED requester (degraded) instead of leaving it to the deploy timeout;
|
||||
/// the in-flight one is answered by the incarnation check.</summary>
|
||||
[Fact]
|
||||
public void A_restart_answers_queued_rebuild_requesters()
|
||||
{
|
||||
var sink = new ExistenceSink();
|
||||
using var gate = new ManualResetEventSlim(false);
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
var childProps = OpcUaPublishActor.PropsForTests(sink, applier, new OpcUaPublishActor.TestHooks { BeforePlan = () => gate.Wait() });
|
||||
var parent = Sys.ActorOf(Props.Create(() => new RestartingParent(childProps)));
|
||||
parent.Tell(RestartingParent.GetChild.Instance);
|
||||
var child = ExpectMsg<IActorRef>();
|
||||
|
||||
var inFlight = Rebuild(RawTagArtifact(2));
|
||||
var queued = Rebuild(RawTagArtifact(3));
|
||||
child.Tell(inFlight);
|
||||
child.Tell(queued);
|
||||
child.Tell(Kill.Instance);
|
||||
|
||||
var first = ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
|
||||
first.Correlation.ShouldBe(queued.Correlation, "the queued requester is answered at the restart");
|
||||
first.Degraded.ShouldBeTrue();
|
||||
|
||||
gate.Set();
|
||||
var second = ExpectMsg<OpcUaPublishActor.RebuildCompleted>(PlanBudget);
|
||||
second.Correlation.ShouldBe(inFlight.Correlation);
|
||||
second.Degraded.ShouldBeTrue();
|
||||
}
|
||||
|
||||
private static AlarmConditionSnapshot ActiveUnacked(uint quality) =>
|
||||
new(Active: true, Acknowledged: false, Confirmed: true, Enabled: true, AlarmShelvingKind.Unshelved, 500, "high", quality);
|
||||
|
||||
// ───────────────────────────── helpers ─────────────────────────────
|
||||
|
||||
private IActorRef Spawn(ExistenceSink sink, ManualResetEventSlim gate) =>
|
||||
@@ -264,17 +454,30 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
|
||||
ScriptedAlarms = Array.Empty<object>(),
|
||||
});
|
||||
|
||||
private static byte[] AlarmArtifact() =>
|
||||
private static byte[] AlarmArtifact(string equipmentName = "Eq", int rawTags = 0) =>
|
||||
JsonSerializer.SerializeToUtf8Bytes(new
|
||||
{
|
||||
UnsAreas = new[] { new { UnsAreaId = "area-1", ClusterId = "c1", Name = "Area" } },
|
||||
UnsLines = new[] { new { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "Line" } },
|
||||
Equipment = new[]
|
||||
{
|
||||
new { EquipmentId = "eq-1", DriverInstanceId = (string?)null, UnsLineId = "line-1", Name = "Eq", MachineCode = "EQ-1" },
|
||||
new { EquipmentId = "eq-1", DriverInstanceId = (string?)null, UnsLineId = "line-1", Name = equipmentName, MachineCode = "EQ-1" },
|
||||
},
|
||||
DriverInstances = Array.Empty<object>(),
|
||||
Tags = Array.Empty<object>(),
|
||||
RawFolders = rawTags == 0
|
||||
? Array.Empty<object>()
|
||||
: new object[] { new { RawFolderId = "rf-1", ParentRawFolderId = (string?)null, Name = "Plant", ClusterId = "c1" } },
|
||||
DriverInstances = rawTags == 0
|
||||
? Array.Empty<object>()
|
||||
: new object[] { new { DriverInstanceId = "drv-1", DriverType = "Modbus", DriverConfig = "{}", Name = "Modbus", RawFolderId = "rf-1", ClusterId = "c1" } },
|
||||
Devices = rawTags == 0
|
||||
? Array.Empty<object>()
|
||||
: new object[] { new { DeviceId = "dev-1", DriverInstanceId = "drv-1", Name = "dev1", DeviceConfig = "{}" } },
|
||||
TagGroups = Array.Empty<object>(),
|
||||
Tags = Enumerable.Range(0, rawTags).Select(i => (object)new
|
||||
{
|
||||
TagId = $"t-{i}", DeviceId = "dev-1", TagGroupId = (string?)null, Name = $"T{i}",
|
||||
DataType = "Float", AccessLevel = 0, TagConfig = "{}",
|
||||
}).ToArray(),
|
||||
ScriptedAlarms = new[]
|
||||
{
|
||||
new
|
||||
@@ -334,6 +537,22 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
|
||||
private readonly Dictionary<string, uint> _alarmQualities = new();
|
||||
|
||||
public ConcurrentQueue<string> SkippedWrites { get; } = new();
|
||||
private readonly Dictionary<string, int> _alarmWrites = new();
|
||||
private volatile bool _sawRebuild;
|
||||
|
||||
/// <summary>Whether any apply called <see cref="RebuildAddressSpace"/> (the full-rebuild shape).</summary>
|
||||
public bool SawRebuild => _sawRebuild;
|
||||
public int AlarmWritesTo(string id) { lock (_gate) return _alarmWrites.GetValueOrDefault(id); }
|
||||
public void SetValueOutOfBand(string id, object? value)
|
||||
{
|
||||
lock (_gate) { var c = _values.GetValueOrDefault(id); _values[id] = (value, c.Status, c.At); }
|
||||
}
|
||||
|
||||
/// <summary>What an SDK-applied Part 9 Acknowledge does: mutates the condition directly, no sink write.</summary>
|
||||
public void AcknowledgeOutOfBand(string id)
|
||||
{
|
||||
lock (_gate) _alarmStates[id] = _alarmStates[id] with { Acknowledged = true };
|
||||
}
|
||||
public ConcurrentQueue<string> SkippedAlarms { get; } = new();
|
||||
public int ReplayedItems;
|
||||
|
||||
@@ -378,7 +597,10 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
|
||||
lock (_gate)
|
||||
{
|
||||
if (!_conditions.Contains(alarmNodeId)) { SkippedAlarms.Enqueue(alarmNodeId); return; }
|
||||
_alarmWrites[alarmNodeId] = _alarmWrites.GetValueOrDefault(alarmNodeId) + 1;
|
||||
// The node manager projects the snapshot's StatusCode onto the condition's Quality.
|
||||
_alarmStates[alarmNodeId] = state;
|
||||
_alarmQualities[alarmNodeId] = state.StatusCode;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -410,12 +632,13 @@ public sealed class OpcUaPublishActorDeferredWriteTests : RuntimeActorTestBase
|
||||
{
|
||||
lock (_gate)
|
||||
{
|
||||
// Variables are recreated empty; a condition's state is captured and carried over to its
|
||||
// re-materialised instance (the node manager's rebuild carry-over).
|
||||
_applyClosed = false;
|
||||
_sawRebuild = true;
|
||||
_variables.Clear();
|
||||
_conditions.Clear();
|
||||
_values.Clear();
|
||||
_alarmStates.Clear();
|
||||
_alarmQualities.Clear();
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user