Merge #854 ConditionRefresh not overtaken by deferred condition events (Server-object items)
v2-ci / unit-tests (push) Failing after 1h12m23s

This commit is contained in:
Joseph Doherty
2026-11-11 07:39:25 -05:00
5 changed files with 1278 additions and 55 deletions
+48 -4
View File
@@ -882,15 +882,59 @@ Behaviours worth knowing before enabling it:
changes which folders a later hop reaches — and the liveness check covers removal); `MonitoredNode2.Dispose` completes
its channel writer, which fails a pending `WriteAsync` with `ChannelClosedException` (swallowed by the SDK), so a
disposed node cannot stall a condition's queue; and the delivery task catches per-event exceptions, so it keeps draining.
- ⚠️ **Residual (documented, not fixed): `ConditionRefresh` does not flush deferrals.** While a notifier channel is
overloaded, a client that refreshes may receive older deferred events of a condition after `RefreshEndEvent`. They
are still in that condition's order and its newest event arrives last, so the state a client ends up holding is
correct; making refresh wait for the queues would block a service call on the very drain the overload is stalling.
- **`ConditionRefresh` is not overtaken by deferred events on Server-object items (Gitea #854).** A refresh of an
`i=2253` item replays each condition's current state between RefreshStart and RefreshEnd; the condition's deferred
events — all older — used to follow RefreshEnd, briefly showing a stale state or re-adding an alarm the refresh had
just dropped (ScadaBridge's snapshot swap drops absent conditions, then a stale active event re-added it). Every
condition event now carries its report sequence. Two paths, chosen per refresh:
- **Nothing deferred (the common case): unchanged and off `Lock`.** The SDK-shaped notifier walk, queued as live
condition nodes; `Lock` is taken only to copy the root-notifier list. It stands only if no deferral episode existed
at any moment of it (deferral count zero before and after, episode number unchanged); otherwise the cut path runs
too, and its replay — later in the same RefreshStart..RefreshEnd — supersedes anything that slipped in between.
- **Deferring: the cut.** ACL/role verdicts first, off `Lock`. Then ONE short `Lock` hold takes the sequence
watermark and a frozen snapshot of each Retain condition straight from the condition map (O(conditions), no tree
walk), and installs per item, under that item's gate, the **cut** plus a "replay pending" marker. The replay is
queued **off `Lock`**, one event per hold of the item gate. While the marker is set, every condition event to the
item takes the gate: one newer than the cut marks its condition so the older frozen entry is dropped instead of
following it; a deferred one at or below the cut is withheld **from that item only** when the refresh re-sent its
condition, or when it is `Retain=true` for a condition the refresh omitted **because it was not retained**. An
older `Retain=false` event of such a condition still arrives (it agrees with the omission), and a condition that was
retained but omitted for the item's **grants** is never withheld (a grant added before the drain lets its events
through; the ACL decides them at delivery as ever).
- The deferral queues, every other Server-object item and every folder item are untouched. A refresh waits for the
drain at most for one in-progress `QueueEvent` on that item (the gate), never for the drain itself; a reporter
holding `Lock` likewise waits at most for one. Each item keeps one gate/cut state for its lifetime (keyed by the
item, so an unsubscribe + resubscribe keeps it), and cuts are cleared when the deferral count returns to zero.
The custom refresh is refused (`ObjectDisposedException`) once the node manager is disposing, and disposal waits
for refreshes in flight, as the base's private operation guard does. Meter:
`otopcua.opcua.condition_events.superseded` (per item and event). ConditionRefresh and ConditionRefresh2, flag
on and off.
- **Measured** (3000 retained conditions, longest `Lock` wait another thread saw during one refresh): the first
#854 cut (`1622d07f`, everything under `Lock`) 95–106 ms idle / 102–127 ms deferring; now 0–3 ms idle /
23–32 ms deferring (the frozen snapshots).
- ⚠️ **Residual: folder (notifier) items.** A refresh of an item on an equipment/device folder can still be followed
by older events of a condition. Two sources, neither reachable per item: the SDK's own `MonitoredNode2` channel
(up to 4096 events already accepted before the refresh — this exists in the stock SDK too, and in normal operation
for the few events in transit), and our deferral queue, whose folder hop goes through that channel, where the SDK
memoises the per-item permission verdict by (item, event type, source) so there is no per-event, per-item hook. The
events stay in the condition's order and its newest arrives last, so the client converges. Discarding the queue
globally on refresh would drop transitions for every non-refreshing subscriber while leaving the channel backlog;
making refresh wait would block a service call on the stalled drain. ScadaBridge also ignores a transition older
than the one it holds for a condition.
Pinned by `FolderEventFloodWireTests`: 6000 events under one `Lock` hold with publishing disabled (flag on and off)
complete, drain to zero, coalesce, and the item delivers at most its revised `QueueSize` in increasing order ending
with the last event; and a condition **removed** in the same `Lock` hold has none of its deferred events delivered to
either the folder or the Server-object item (the flag-off variant fails without the liveness checks).
`ConditionRefreshDeferralWireTests` (#854) stalls a folder item's SDK lock so its channel fills without `Lock` held,
refreshes the Server-object item (ConditionRefresh and ConditionRefresh2, flag on/off, condition retained or cleared)
and pins that nothing older than the replay follows RefreshEnd while the non-refreshing folder item still receives the
whole tail (also across an unsubscribe + resubscribe of the item, and the cut is cleared after the drain); a
probe-driven test pins that a refresh waits for a delivery already past its check (fails without the item gate); an
ACL-on test pins that a condition omitted for want of a grant is not withheld once the grant lands; a 3000-condition
test measures the `Lock` hold and pins that neither path queues its replay under `Lock`; and a stress test runs
reports, refreshes and an intermittent stall concurrently (its stream never goes backwards outside a refresh, and
every refresh ends on the newest state delivered).
**Model-change and audit events on the Server object are gated too** (Gitea #588). The node manager's
own server-wide events — `GeneralModelChangeEvent` (NodeAdded / NodeDeleted / DataTypeChanged) and the