From ec17a8797b58189b142c9edcf4cc0ae21095bdf4 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 11 Nov 2026 13:05:06 -0500 Subject: [PATCH] =?UTF-8?q?fix(opcua):=20#855=20review=204=20=E2=80=94=20p?= =?UTF-8?q?er-item=20"known"=20in=20the=20folder=20cut;=20pin=20the=20post?= =?UTF-8?q?-stall=20liveness=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - RefreshDirectItemsWithCut keys its "decided off Lock" set per (item, condition), so a condition one item's walk saw is still decided for another. - The deferral warning no longer blames slow folder subscribers (no folder of ours defers since #855). - FolderEventFloodWireTests' removal test now asserts the folder item got nothing and the Server object exactly the pre-stall event (it passed with the post-stall liveness re-check deleted). Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk --- .../OtOpcUaNodeManager.cs | 13 +++++++------ .../FolderEventFloodWireTests.cs | 5 ++++- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index f6af07d1a..0bed75dad 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -1806,9 +1806,9 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 { _lastDeferralWarningUtc = now; _logger.LogWarning( - "OtOpcUaNodeManager: condition events are being deferred — an event notifier's delivery channel is " - + "full (first: {NodeId}). Folder event subscribers are not keeping up, or Lock is held across a large " - + "burst; events stay ordered per condition, and past {PerCondition} waiting per condition the oldest " + "OtOpcUaNodeManager: condition events are being deferred — a notifier hop the node manager does not " + + "serve itself suspended (first: {NodeId}); since #855 no folder of ours does, so this is unexpected. " + + "Events stay ordered per condition, and past {PerCondition} waiting per condition the oldest " + "give way to the newest (Gitea #838)", condition.NodeId, MaxDeferredEventsPerCondition); } @@ -6948,7 +6948,8 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 // Verdicts per (item, condition) before Lock, from a live walk: they depend on the event type, source, notifier // wiring and grants, not on the condition's state. var allowed = new HashSet<(IEventMonitoredItem, NodeState)>(); - var known = new HashSet(System.Collections.Generic.ReferenceEqualityComparer.Instance); + // Per (item, condition): a condition one item's walk saw is still undecided for another item (#855 review 4). + var known = new HashSet<(IEventMonitoredItem, NodeState)>(); foreach (var item in items) { if (AttachedNotifierOf(item) is not { } notifier) continue; @@ -6957,7 +6958,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 foreach (var e in events) { if (ConditionOf(e) is not { } node) continue; - known.Add(node); + known.Add((item, node)); if (IsDirectRefreshEventAllowed(item, e, enforced)) allowed.Add((item, node)); } } @@ -7015,7 +7016,7 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2, INodeManager3 // A condition materialised since the verdicts were taken is decided here (rare). if (!allowed.Contains((item, instance))) { - if (known.Contains(instance)) continue; + if (known.Contains((item, instance))) continue; if (!IsDirectRefreshEventAllowed(item, target, enforced)) continue; } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderEventFloodWireTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderEventFloodWireTests.cs index 97f2271de..266637e48 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderEventFloodWireTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/FolderEventFloodWireTests.cs @@ -193,7 +193,10 @@ public sealed class FolderEventFloodWireTests Flooded(onServer).ShouldNotBeEmpty(); // None of the deferred tail — which was still waiting when the condition was removed — reached anyone. Flooded(onServer).Max().ShouldBeLessThan(FloodSize - 1, "no ghost reached the Server-object subscriber"); - Flooded(onA).DefaultIfEmpty(-1).Max().ShouldBeLessThan(FloodSize - 1, "no ghost reached the folder subscriber"); + // Only the in-flight f0 ever reached the stalled EqA hop, and the condition was gone when the stall released: the + // post-stall liveness re-check must have withheld it (#855 review 4 — "< FloodSize - 1" passed without that check). + Flooded(onA).ShouldBeEmpty("no ghost reached the folder subscriber"); + Flooded(onServer).ShouldBe([0], "only the event reported before the stall reached the Server object"); } private static AlarmConditionSnapshot State(string message) =>