Merge #855 folder event items: per-item delivery under the node manager, ConditionRefresh cut matches the Server object

Claude-Session: https://claude.ai/code/session_01GT67AD99zQQqpLCVm3p7xk
This commit is contained in:
Joseph Doherty
2026-11-11 13:07:21 -05:00
10 changed files with 1654 additions and 228 deletions
@@ -0,0 +1,102 @@
# Folder (notifier) event items: per-item delivery (Gitea #855)
Status: design for #855 (owner decision 2026-11-11: build per-item delivery). Background: #838 (deferral instead of
blocking under `Lock`), #854 (sequence cut on Server-object refresh), #826 (per-folder event ACL).
## Problem
A condition event reaches an event item on an equipment / raw device folder through the SDK's `MonitoredNode2`
for that folder: `OnReportEventAsync` writes it to a bounded channel (4096), and a per-node consumer task decides
each item's verdict (memoised per `(item, event type, SourceNode)`) and calls `QueueEvent`. That gives the node
manager no per-event, per-item decision point, so a `ConditionRefresh` of a folder item cannot withhold an older
event that is still in the channel or in our deferral queue: the refresh is overtaken (the #854 guarantee covers only
Server-object items). The channel's consumer also needs `Lock` on a verdict miss, which is why a burst under `Lock`
fills the channel and defers (#838).
## Design
### Where delivery hooks in
`DeliverConditionEventAsync` already walks the condition's notifier chain hop by hop (our copy of the SDK's
`ReportEventAsync`, #838). At a hop whose `OnReportEventAsync` is the SDK `MonitoredNode2` of **a node this manager
owns** (`handler.Target is MonitoredNode2 m && m.Node == node`), the walk no longer calls the handler. It delivers the
event itself to that node's attached items (`m.EventMonitoredItems`), one by one: the SDK's audit guard, the verdict
(`INodeManager3.ValidateEventRolePermissions` — the SDK's type/source `ReceiveEvents` check plus, under enforcement,
the #826 live-notifier + grant check), then the item's gate and cut, then `QueueEvent`.
This is exactly how Server-object items are served (`OnConditionReportEvent`, #586/#854): same verdict per event, same
gate, same cut, same `QueueEvent`.
### Bypass / coexistence with `MonitoredNode2`
- **Condition events bypass the channel** for our notifiers. The `MonitoredNode2` stays: the SDK still owns item
attachment (subscribe / unsubscribe / transfer / tombstone / revalidation stand-down detach), and its channel still
carries everything else that reaches the node — a condition's SDK-raised method **audit** events (only with auditing
on) and data-change notifications for attribute items on the folder. Those keep the SDK's cached verdict, so
`InvalidateFolderEventVerdicts` stays.
- A stood-down item is not in `EventMonitoredItems`, so it receives nothing, as before.
- A hop whose handler is anything else (none exists today: conditions are never event notifiers and variables cannot
host event items) still goes through the handler — the fallback that keeps the #838 deferral reachable.
### Consequence for overload
Every hop the node manager owns now completes synchronously: `QueueEvent` never blocks (the item's own queue discards
oldest at `QueueSize`), and the verdict runs on the reporter's thread, which already holds `Lock`. So a burst under one
`Lock` hold no longer defers at all, and the #838 deadlock shape (a consumer needing `Lock` while the reporter waits on
its channel) is gone for condition events. Folder items stay in report order per item, exactly like Server-object items.
The #838 deferral queue and the #854 cut machinery remain as the safety net for a hop that does suspend (the fallback
above, or a future SDK change). Folder items are brought under the same cut:
- An event whose delivery continues off `Lock` — queued in the deferral queue, **or** the in-flight event whose walk
suspended at a hop (new: marked when the walk suspends, so the hops after the suspension take the gate) — takes the
item's gate and is withheld when the item's latest refresh cut supersedes it (`RefreshCut.Supersedes`, unchanged).
- `ConditionRefresh` / `ConditionRefresh2` of folder items has the same two paths as the Server object, flag on and off:
- **Nothing deferred:** the SDK base walk of the item's notifier (off `Lock`, live condition nodes), per-item verdict
as before (#826 filter under enforcement). Kept only if no deferral episode existed at any moment of it; otherwise
the cut path runs too and supersedes it inside the same RefreshStart..RefreshEnd. Its closing check takes `Lock`
once: a report whose walk suspended (marked under `Lock` before suspending) has registered its deferral by then —
without it a folder refresh could pass the check in the instant between the suspension and the count.
- **Deferring — the cut:** verdicts per (item, condition) first, off `Lock`, from a live walk. Then one `Lock` hold:
watermark, episode, the Retain set, and per item the same walk producing frozen snapshots, with the cut + "replay
pending" marker installed under the item's gate. The replay is queued off `Lock`, one event per gate hold.
- State per item (`EventItemState`, previously Server-object only) is keyed by the item object for its lifetime, so it
survives unsubscribe + resubscribe; stale cuts are cleared when the deferral episode ends, for folder and Server items.
### Locking
Unchanged order, now for folder items too: `Lock` → item gate → the SDK item's own lock. Synchronous delivery runs
under the reporter's `Lock`; the verdict takes `Lock` re-entrantly and the core node manager's / authorizer's locks
(as the Server-object path already does). Off-`Lock` (deferred) delivery computes the verdict **before** taking the item
gate, so nothing under a gate takes `Lock` or the event-report gate. A reporter waits at most for one in-progress
`QueueEvent` on an item; a refresh waits at most for one deferred delivery already inside the gate, never for a drain.
No reporter ever waits on a channel.
### Bounds
No new queue. Per item: the SDK item queue (`QueueSize`, DiscardOldest). Deferral (only on the fallback): unchanged,
1024 per condition / 65 536 total, coalescing. Cuts: one per refreshed item, sets sized by the conditions, cleared at the
end of the deferral episode.
### ACL (#826)
Each event is decided per item at delivery against the current generation — no memoised verdict for condition events
any more, so a deploy / revocation / reference removal takes effect on the next event without waiting for an
invalidation. Refresh replays only conditions the item may see (the same check); a condition omitted for want of a grant
is never withheld afterwards (the cut's existing rule).
### Cost
The per-event verdict for folder items moves from the SDK's consumer task (cached) to the reporter thread under `Lock`
(uncached), as Server-object items already pay. Folder subscribers are few (ScadaBridge subscribes at `i=2253`).
## Tests
- `ConditionRefreshDeferralWireTests` / `FolderEventFloodWireTests` / `EventQueueOverflowAclWireTests`: deferral is no
longer produced by flooding a folder item; tests that need it stall the folder's hop through a test probe
(`NotifierDeliveryStall`, null in production) instead of the folder item's SDK lock (which would now block the reporter,
as it would on the Server object). Flood tests now pin that folder delivery never defers.
- New `FolderConditionRefreshWireTests`: folder-item refresh during deferral (flag on/off, retained / cleared,
ConditionRefresh2, resubscribe), the gate race, newer-than-cut replay, ACL omission not withheld, ACL revocation during
deferral, the in-flight suspended event, and two stress tests (production overload with no stall; stall + refresh +
reports concurrently).
+46 -22
View File
@@ -901,7 +901,7 @@ Behaviours worth knowing before enabling it:
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
- The deferral queues, every other Server-object item and every non-refreshing 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.
@@ -912,29 +912,53 @@ Behaviours worth knowing before enabling it:
- **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.
- **Folder (notifier) items are delivered per item (Gitea #855).** Before #855 a folder item was fed by the SDK's own
`MonitoredNode2` channel (up to 4096 events accepted before a refresh), with the per-item verdict memoised by (item,
event type, source), so a refresh of a folder item could still be followed by older events. Now, at every notifier
the node manager owns, the walk serves the attached items itself (`DeliverToNotifierItems`), exactly as it serves
Server-object items: the SDK's audit guard, the verdict through `INodeManager3.ValidateEventRolePermissions` (SDK
type/source `ReceiveEvents` + the #826 decision under enforcement — its item-independent part, the live condition,
the notifier check and the visibility scopes, resolved once per hop) **per event, uncached**, then the item's gate and
cut, then `QueueEvent`. The `MonitoredNode2` stays for item attachment and for what still uses its channel (a
condition's SDK method audit events, data-change items on the folder), so `InvalidateFolderEventVerdicts` stays.
- **No notifier of ours defers any more.** `QueueEvent` never waits for a consumer (the item queue discards oldest at
`QueueSize`) and the verdict runs on the reporter's thread, which already holds `Lock` — so a burst under one
`Lock` hold completes with nothing deferred, and the #838 deadlock shape is gone for condition events. Folder items
therefore see condition events in report order. The cost moves with it: the per-event verdict and filter for
folder items now run under `Lock` (as for Server-object items), where the SDK used to cache the verdict off it —
including the SDK item queue's own per-event costs (its duplicate scan, and at a full queue a list shift), so many
folder items on one notifier whose clients stop publishing make each report proportionally slower under `Lock`.
With auditing on (off in this repo), a condition's SDK method **audit** event still takes the folder's channel, so
at a folder item it may now arrive after the condition event that method produced (they were one FIFO before).
- **The deferral queue and the cut stay as the safety net**, for a hop that is not ours (none exists today:
conditions are never event notifiers and variables cannot host event items) — and folder items share the cut:
a `ConditionRefresh`/`ConditionRefresh2` of a folder item takes the same two paths as the Server object (plain walk
off `Lock` when nothing is deferred; otherwise verdicts off `Lock`, then one `Lock` hold for the watermark, the
Retain set and, per item, its notifier walk frozen as snapshots, with the cut installed under the item gate and the
replay queued off `Lock`). An event whose walk suspended at a hop is marked deferred **before** it suspends, so
every later hop judges it against the cut; the plain refresh path's closing check takes `Lock` once, so a report
that suspended under it has registered its deferral by then. Lock order is unchanged (`Lock` → item gate → item
lock); off `Lock` the verdict is computed before the gate.
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).
complete **with nothing deferred** (fails if the per-item delivery stops recognising the SDK fan-out), and the item
delivers at most its revised `QueueSize` in increasing order ending with the last event; and a condition **removed**
while its events wait (delivery stalled at the folder) has none of them delivered to either the folder or the
Server-object item. Tests that need a deferral stall delivery at a folder through a test probe
(`NotifierStall` → `OtOpcUaNodeManager.NotifierDeliveryStall`, null in production), which suspends the walk exactly
as a full channel did — holding a folder item's SDK lock, as the pre-#855 tests did, would now block the reporter.
`ConditionRefreshDeferralWireTests` (#854) refreshes the Server-object item while the folder is stalled
(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; 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 a stress test runs reports,
refreshes and an intermittent stall concurrently. `FolderConditionRefreshWireTests` (#855) is the folder-item twin:
the same matrix on a refreshed folder item (a second subscriber on the same folder keeps the whole tail; a
ModifyMonitoredItems re-subscribe keeps the cut), the gate race, a newer-than-cut event on the raw device folder, a
condition omitted by the ACL (reference dropped) not withheld once it returns, a revocation after the refresh, a
production-shape overload (bursts under `Lock` + repeated refreshes, no stall: nothing deferred, the stream never
goes backwards, no replay queued under `Lock`) and a stress test with an intermittent stall.
**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