From dc459633e01273b39c07a5fa5ab7091526c9150c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 15 Aug 2026 21:33:18 -0400 Subject: [PATCH] =?UTF-8?q?docs(plan):=20perf=20roadmap=20remainder=20?= =?UTF-8?q?=E2=80=94=20historian=20batching,=20ACL=20cache,=20driver=20rou?= =?UTF-8?q?nd-trips,=20alarm=20coalescing,=20AdminUI=20hot=20spots,=20resi?= =?UTF-8?q?duals?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../2026-08-15-perf-roadmap-remainder.md | 1266 +++++++++++++++++ ...08-15-perf-roadmap-remainder.md.tasks.json | 34 + 2 files changed, 1300 insertions(+) create mode 100644 docs/plans/2026-08-15-perf-roadmap-remainder.md create mode 100644 docs/plans/2026-08-15-perf-roadmap-remainder.md.tasks.json diff --git a/docs/plans/2026-08-15-perf-roadmap-remainder.md b/docs/plans/2026-08-15-perf-roadmap-remainder.md new file mode 100644 index 00000000..2a374d70 --- /dev/null +++ b/docs/plans/2026-08-15-perf-roadmap-remainder.md @@ -0,0 +1,1266 @@ +# Perf Roadmap Remainder Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:executing-plans (or +> superpowers-extended-cc:subagent-driven-development in-session) to implement this plan +> task-by-task. + +**Goal:** Close the remaining items of the 2026-08-15 architectural performance review — +roadmap items 3 (historian group commit + parallel drain), 4 (ACL cache re-shape), +5 (S7 span grouping + AbCip bounded parallelism), 6 (alarm-engine coalescing), +7 (AdminUI hot spots) — plus the residual finding-07 blocking-I/O sites and finding-08 +per-value waste multipliers. + +**Architecture:** Six independent work streams on one branch (`perf/roadmap-remainder`), +each stream a small set of TDD tasks. The historian stream extends the item-1 batching +pattern one hop further (mux → subscribers) so the outbox can group-commit; the ACL +stream re-shapes the decision cache and hot path without touching the trie; the driver +stream copies Modbus's coalescing shape into S7 and adds a bounded window to AbCip; the +alarm stream adds dirty-set coalescing while preserving the pinned crash-consistency +semantics; AdminUI gets projections + watermark/TTL caches; the residuals batch the +comms-loss fan and move the last hot-mailbox blob read off-thread. + +**Tech Stack:** .NET 10, Akka.NET, EF Core (SQL Server + InMemory tests), FasterLog, +S7netplus, libplctag, xUnit + Shouldly, Akka.TestKit. + +**Verified baseline:** master `a8c93a00`. All line numbers below were re-verified against +that commit by six parallel exploration passes (2026-08-15); where the review's numbers +had drifted, the corrected ones are used here. + +**Execution conventions (all tasks):** +- Work in this checkout on branch `perf/roadmap-remainder`. Stage **explicit paths only** + (`git add `), never `git add -A`. +- TDD: write the failing test first, watch it fail, implement, watch it pass, run the + project's full unit suite, commit. +- Test commands: `dotnet test tests//` for a project, + `dotnet test --filter "FullyQualifiedName~."` for one test. +- Do not touch `*.IntegrationTests` projects (Docker fixtures) — build-only. +- If a task needs a file not in its `Files:` block, STOP and surface it as a plan defect. + +--- + +## Stream A — Historian group commit + parallel drain (review finding 03, Critical) + +Current facts: `IHistorizationOutbox` has only single-entry `AppendAsync` +(`src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/Historian/IHistorizationOutbox.cs:11–44`); +`FasterLogHistorizationOutbox.AppendAsync` (`src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/FasterLogHistorizationOutbox.cs:97–137`) +does one `EnqueueAsync` + one `CommitAsync` (fsync) per entry in `PerEntry` mode, plus a +second `CommitAsync` per capacity truncation. Values arrive at the recorder as one +`VirtualTagActor.DependencyValueChanged` message per value because the item-1 batch +envelope stops at `DependencyMuxActor.OnAttributeValueBatchPublished` +(`src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs:124–139`), which +fans one message per (item, subscriber). The drain +(`ContinuousHistorizationRecorder.DrainOnceAsync`, `.../Runtime/Historian/ContinuousHistorizationRecorder.cs:500–626`) +awaits one `WriteLiveValuesAsync` per tag group sequentially. The gateway client (0.3.0) +has **no multi-tag write RPC** — a batch RPC on the HistorianGateway contract is a +**cross-repo follow-up, out of scope here**; the client-side fix is bounded parallelism. + +### Task 1: Outbox batch append with group commit + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/Historian/IHistorizationOutbox.cs` +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway/Recorder/FasterLogHistorizationOutbox.cs` +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Historian.Gateway.Tests/Recorder/FasterLogHistorizationOutboxTests.cs` +- Modify (fakes only): `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs` (the nested `InMemoryOutbox`), `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/ServiceCollectionExtensionsTests.cs` (the nested `FakeOutbox`) + +**Spec:** +1. Add to `IHistorizationOutbox`: + ```csharp + /// + /// Appends a burst of entries with ONE durability commit for the whole batch (group + /// commit). Under PerEntry commit mode this is the per-batch fsync that replaces N + /// per-entry fsyncs; under Periodic it is indistinguishable from N AppendAsync calls. + /// An empty list is a no-op. + /// + ValueTask AppendBatchAsync(IReadOnlyList entries, CancellationToken ct); + ``` +2. `FasterLogHistorizationOutbox.AppendBatchAsync`: loop `_log.EnqueueAsync(payload, ct)` + for every entry, **then** one `await _log.CommitAsync(ct)` when + `_commitMode == PerEntry` (zero commits when `Periodic`). Then do the `_state`-locked + bookkeeping for all entries in one lock acquisition (add all to `_live`/`_index`, + run the drop-oldest loop once), and — if anything was dropped — one + `_log.TruncateUntil(...)` + **one** `CommitAsync`. Net worst case: **two** commits per + batch (vs `2×N` today at capacity). +3. Re-implement single `AppendAsync` as `AppendBatchAsync([entry], ct)`-equivalent (may + delegate; keep the public single-entry signature and its existing behaviour). +4. Update the two test fakes (`InMemoryOutbox`, `FakeOutbox`) to implement the new member + (loop their existing `AppendCore`; `InMemoryOutbox` keeps its yielding-on-pool-thread + variant applied to the **batch** method too — same + `await Task.Delay(1).ConfigureAwait(false)` idiom, see the comment at + `ContinuousHistorizationRecorderTests.cs:695–748` for why `Task.Yield()` is vacuous). + +**Tests (write first, in `FasterLogHistorizationOutboxTests.cs`):** +- `AppendBatch_then_peek_returns_fifo_in_batch_order` — batch of 3, `PeekBatchAsync(10)` + returns the 3 in order; restart-recovery (dispose + reopen, mirroring + `Remove_truncates_and_survives_restart` at `:41`) still sees all 3 (proves the batch + was committed durably). +- `AppendBatch_at_capacity_drops_oldest_across_the_whole_batch` — capacity 2, batch of 4: + only the last 2 survive, `DroppedCount == 2`. +- `AppendBatch_empty_is_noop` — no throw, count unchanged. + +**Commit:** `perf(historian): group-commit batch append on the historization outbox` + +--- + +### Task 2: Batched mux→subscriber delivery, single-eval virtual tags + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 1, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyMuxActor.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/VirtualTagActor.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/VirtualTags/DependencyConsumerMuxAdapter.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationRecorder.cs` (receive registration only — the batch handler loops the existing per-value logic; the group-commit rewrite is Task 4) +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ScriptedAlarms/ScriptedAlarmHostActor.cs` (receive registration only — loop `_upstream.Push` per item) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/DependencyMuxActorTests.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/VirtualTags/VirtualTagActorTests.cs` + +**Spec:** +1. New message, colocated with `DependencyValueChanged` in `VirtualTagActor`: + ```csharp + /// + /// One subscriber's slice of one upstream burst: every item in this batch matched + /// this subscriber's registered interest set. Never empty. List ownership transfers + /// to the receiver (the mux allocates a fresh list per subscriber per burst). + /// + public sealed record DependencyValueBatch(IReadOnlyList Items); + ``` +2. `DependencyMuxActor.OnAttributeValueBatchPublished` (`:124–139`): instead of + `sub.Tell(dep)` per (item, subscriber), accumulate per-subscriber lists + (`Dictionary>`, allocated lazily — nothing when + no item matches), then one `sub.Tell(new DependencyValueBatch(list))` per subscriber. + The per-value comms-loss path (`OnAttributeValuePublished`, `:102–115`) also emits a + single-item `DependencyValueBatch` — after this task the mux emits **only** batches. +3. Subscribers gain a `Receive` that loops their + existing per-item handler — mechanical for `DependencyConsumerMuxAdapter` (whatever its + current dispatch is, applied per item), `ContinuousHistorizationRecorder` + (`ReceiveAsync` looping `OnValueChangedAsync` per item — Task 4 replaces this), and + `ScriptedAlarmHostActor` (`OnDependencyChanged` per item). **Keep** each subscriber's + existing `DependencyValueChanged` receive registration (tests and any direct senders + still use it — e.g. `ContinuousHistorizationRecorderTests` Tells the per-value message + directly). +4. `VirtualTagActor` is the one subscriber whose batch handler is NOT a loop: apply + **all** items to `_dependencies` first, then run `Evaluate` **once** (the existing + `OnDependencyChanged` body from `:179–224` split into apply-phase and evaluate-phase). + A K-input script hit by a K-item burst evaluates once instead of K times, and that one + evaluation sees the complete input set. + +**Tests (write first):** +- `DependencyMuxActorTests`: `A_burst_reaches_each_subscriber_as_one_batch_message` — + register subscriber for `["a","b"]`, send a 3-item `AttributeValueBatchPublished` + (`a`, `b`, `unmatched`), `ExpectMsg` with exactly + 2 items and **no further message** (`ExpectNoMsg`). Update the existing + `AttributeValueBatchPublished_is_filtered_per_item_to_interested_subscribers` (`:158`) + to the batch shape. +- `VirtualTagActorTests`: `A_batch_touching_K_inputs_evaluates_once` — counting evaluator + (a `IVirtualTagEvaluator` fake with an eval counter), 3-input tag, one 3-item batch ⇒ + counter == 1, one `EvaluationResult` to parent carrying the value computed from all 3 + inputs. Falsification control: send the same 3 values as 3 separate + `DependencyValueChanged` messages ⇒ counter == 3 (proves the batch path, not value + dedup, is what collapsed the count). + +**Commit:** `perf(runtime): mux delivers per-subscriber batches; virtual tags evaluate once per burst` + +--- + +### Task 3: Parallel drain with bounded concurrency + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 1, Task 2, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationRecorder.cs` (`DrainOnceAsync` `:500–626`, `Props` `:178–191`, ctor) +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationOptions.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs` (spawn site `:463–481` threads the new option) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs` + +**Spec:** +1. New option `DrainConcurrency` (int, default `8`, `Validate()` warns when < 1) on + `ContinuousHistorizationOptions`; thread through `Props`/ctor and the spawn site. +2. `DrainOnceAsync`: replace the sequential `foreach` over tag groups (`:517–545`) with a + bounded-parallel execution — materialise the groups, then: + ```csharp + using var window = new SemaphoreSlim(_drainConcurrency); + var tasks = groups.Select(async g => + { + await window.WaitAsync(ct).ConfigureAwait(false); + try + { + bool ok; + try { ok = await _writer.WriteLiveValuesAsync(g.Key, g.Values, ct).ConfigureAwait(false); } + catch { ok = false; } + return (g.Key, g.Values.Count, ok); + } + finally { window.Release(); } + }).ToList(); + var outcomes = await Task.WhenAll(tasks).ConfigureAwait(false); + ``` + then compute `succeeded` / `failedGroups` from `outcomes` and run the **unchanged** + #538 decision block (`:554–611`) against them. `_tagWriteFailures` mutation stays + post-`WhenAll` on the single logical drain flow (never inside the parallel lambdas) — + the dictionary is actor-state. +3. The all-or-nothing ack (`RemoveAsync(batch[^1].Id)`) and the #538 semantics are + **behaviour-pinned**: `A_backend_that_fails_every_tag_abandons_nothing` (`:455–486`), + `A_permanently_failing_tag_is_abandoned_so_healthy_tags_stop_being_rewritten` (`:408`), + and `A_tag_that_recovers_within_its_budget_is_not_abandoned` (`:490`) must pass + unmodified. Do not change what counts as "another tag wrote this pass" — it is now + "any other group's outcome in this pass was ok". + +**Tests (write first):** +- `Drain_writes_distinct_tags_concurrently_within_the_window` — a gating `FakeValueWriter` + variant that blocks the first call on a TCS and counts concurrently-in-flight calls: + with 3 tags queued and concurrency 8, at least 2 calls are in flight before the gate + releases. (Add `MaxObservedConcurrency` to the fake under its existing lock.) +- `Drain_concurrency_of_one_is_sequential` — same fixture, concurrency 1 ⇒ + `MaxObservedConcurrency == 1` (falsification control for the window). +- All existing recorder tests green unmodified. + +**Commit:** `perf(historian): bounded-parallel per-tag drain (DrainConcurrency, default 8)` + +--- + +### Task 4: Recorder batched ingest — one fsync per burst + +**Classification:** high-risk +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 7, Task 9, Task 13, Task 17, Task 21, Task 25 (blocked by Tasks 1, 2, 3 — same file as Task 3) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Historian/ContinuousHistorizationRecorder.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs` + +**Spec:** +1. Rewrite the `DependencyValueBatch` handler (registered in Task 2 as a loop) as the real + batched path: run the existing per-value gates (ref filter `:285–288`, capture gate + `:297–315` — **re-read per value**, the pinned semantic of + `The_capture_gate_is_re_read_per_value_so_promotion_resumes_capture`, numeric coercion + `:317–324`) per item, accumulate the surviving `HistorizationOutboxEntry` list (one per + historian name per item via `_refMap`), then **one** + `await _outbox.AppendBatchAsync(entries, _lifetime.Token).ConfigureAwait(false)` and + one trailing `self.Tell(DrainTick.Instance)`. Capture `IActorRef self = Self` before + the await — same `ConfigureAwait(false)`-destroys-`Self` hazard as the live-gate defect + (`:341` comment). +2. Keep the single-value `DependencyValueChanged` handler delegating to the same core + (a 1-item batch) so existing tests and any direct senders behave identically. +3. On append failure: count `_outboxAppendFailures` once per failed batch append, log, + drop the batch's values (same policy as the per-value path, applied batch-wide). + +**Tests (write first):** +- `A_burst_appends_once_and_drains_all_values` — instrument `InMemoryOutbox` with an + `AppendBatchCallCount`; send one 5-item `DependencyValueBatch` (all refs historized) ⇒ + `AppendBatchCallCount == 1`, all 5 values eventually reach the writer. +- `A_yielding_batch_append_neither_faults_the_actor_nor_loses_values` — the pool-thread + yielding outbox + a batch message; actor stays alive (send `GetStatus` after), values + drain. (Mirrors `A_yielding_append_neither_faults_the_actor_nor_loses_the_value` `:359`.) +- `The_capture_gate_suppresses_items_within_a_batch` — gate closed for the batch ⇒ nothing + appended (extends `A_closed_capture_gate_suppresses_the_value` `:543` to the batch path). + +**Commit:** `perf(historian): batch ingest — one group commit per value burst` + +--- + +### Task 5: Historian defaults, config drift, docs + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** none (blocked by Tasks 3, 4) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Host/appsettings.json` (`:40–51`) +- Modify: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationOptionsTests.cs` +- Modify: `docs/Historian.md`, `docs/Configuration.md` (ContinuousHistorization key tables) +- Modify: `CLAUDE.md` (the ContinuousHistorization key table: add `DrainConcurrency`, correct `DrainBatchSize` default) + +**Spec:** +1. Raise `DrainBatchSize` default `64 → 1024` (options class + appsettings). Rationale to + record in the XML doc: the drain is now bounded-parallel and the peek is an in-memory + scan; 64 made the outbox fill during any gateway hiccup. +2. Fix the shipped-appsettings drift found in exploration: `appsettings.json` sets + `Capacity: 0` (unbounded) while the options default and `docs/Historian.md:176` say + `500_000` — set appsettings to `500000` and add the missing `MaxWriteAttempts: 5` and + new `DrainConcurrency: 8` keys, so the shipped file matches the documented posture. +3. Add a defaults-pinning test to `ContinuousHistorizationOptionsTests` (none exists — + exploration confirmed changing defaults is currently uncaught): assert `CommitMode == + "PerEntry"`, `DrainBatchSize == 1024`, `DrainConcurrency == 8`, `Capacity == 500_000`, + `MaxWriteAttempts == 5`. +4. Docs: document `AppendBatchAsync` group commit ("PerEntry now means one fsync per + ingest burst, not per value"), `DrainConcurrency`, the new `DrainBatchSize`, and record + the **cross-repo follow-up**: a multi-tag batch `WriteLiveValues` RPC on the + HistorianGateway contract (client 0.3.0 has none — bounded parallelism is the ceiling + until then). + +**Commit:** `chore(historian): batch-era defaults + appsettings/doc alignment` + +--- + +## Stream B — ACL cache re-shape (review finding 04, High) + +Current facts: `SessionAclDecisionCache` +(`src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/SessionAclDecisionCache.cs`) is one +flat `ConcurrentDictionary` with `MaxEntries = 100_000` (`:48`) and a **wholesale +`Clear()`** at cap (`:92`), reading `ConcurrentDictionary.Count` (all-locks snapshot) on +every store. `DataPlaneAuthorizer.Evaluate` (`Security/DataPlaneAuthorizer.cs:176–233`) +runs `TryBuildScope` — the `Split('/')` at `:305` — **before** the cache lookup, allocates +a 5-capture closure per call even on hits, and builds interpolated deny reasons eagerly on +the Debug-logged hot deny path. Browse filtering uses a backwards `RemoveAt` loop at +`OtOpcUaNodeManager.cs:2947–2960` (twin at `TranslateBrowsePath` `:2998–3005` the review +missed). `RevalidateMonitoredItems` (`:3173–3221`) walks every live item with no +(session, node) dedup, two `lock (Lock)` takes per changed item, on the deploy thread. +No session-close eviction hook exists anywhere (verified) — the shard cap is the bound. + +### Task 6: Per-session decision-cache shards + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/SessionAclDecisionCache.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SessionAclDecisionCacheTests.cs` + +**Spec:** +1. Re-shape storage to `ConcurrentDictionary` where + ```csharp + private sealed class Shard + { + public readonly ConcurrentDictionary Entries = new(); + public int ApproxCount; // Interlocked; approximate by design — never call Entries.Count on the hot path + } + private readonly record struct ShardKey(OpcUaOperation Operation, AddressSpaceRealm Realm, string NodeIdentifier); + ``` +2. `public const int MaxEntriesPerSession = 20_000;` — at the per-shard cap, clear **that + shard only** (`shard.Entries.Clear(); Interlocked.Exchange(ref shard.ApproxCount, 0);`). + One machine consumer crawling a 50k-node tree now cycles only its own shard; other + sessions' hits survive. Keep `MaxEntries = 100_000` as the **global** approximate bound: + track a global `Interlocked` counter; when it trips, clear all shards (the today + behaviour, now a rare fallback). Both caps use the approximate counters — no + `ConcurrentDictionary.Count` anywhere on the store path. +3. Preserve the public surface the tests and `NodeManagerStage6EnforcementTests:191–197` + pin: `Count` (now the exact sum over shards — off-hot-path property, may enumerate), + `Generation`, `Clear()`, and + `GetOrAdd(long generation, string sessionKey, OpcUaOperation op, AddressSpaceRealm realm, string nodeIdentifier, Func compute)`. + Keep the two pinned semantics verbatim: generation-flush clears everything (single + CAS-winner clears, `:100–108` comment logic), and a decision overtaken by a deploy + mid-compute is **not stored** (`A_decision_overtaken_by_a_deploy_is_not_stored`). +4. XML-doc the design shift: the old "clearing beats evicting" argument (`:37–47`) now + applies **per shard**. + +**Tests (write first):** +- `A_full_session_shard_clears_only_that_session` — fill session "A" past + `MaxEntriesPerSession`, one entry for session "B" ⇒ B's entry still hits (compute not + re-invoked), A's shard restarted. +- `The_global_bound_still_holds` — adapt the existing `The_cache_is_bounded` (`:117`) to + many sessions; `Count` stays ≤ `MaxEntries` + one shard's worth. +- All 9 existing facts pass (adjust only constructions, never assertions). + +**Commit:** `perf(acl): per-session decision-cache shards — a crawl no longer evicts the fleet` + +--- + +### Task 7: Authorizer hot path — lookup first, no closures, lazy deny reasons + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 4, Task 9, Task 13, Task 17, Task 21, Task 25 (blocked by Task 6) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/DataPlaneAuthorizer.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/SessionAclDecisionCache.cs` (add the TryGet/Set pair) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/DataPlaneAuthorizerTests.cs` + +**Spec:** +1. Add an allocation-free fast path to the cache: + `bool TryGet(long generation, string sessionKey, OpcUaOperation op, AddressSpaceRealm realm, string nodeIdentifier, out bool decision)` + and `void Set(long generation, ...same..., bool decision)` (Set carries the + overtaken-generation drop check). Keep `GetOrAdd` delegating to them. +2. Reorder `Evaluate` (`:176–233`): identity/principal/cluster checks → key-revocation + re-check (**must stay outside the cache** — the `:203–208` comment and + `NodeManagerStage6EnforcementTests` clock test pin it) → extract + `nodeId.Identifier as string` (null/empty ⇒ deny, absorbing `TryBuildScope`'s guard) → + `Decisions.TryGet(...)` — **hit returns here, no scope build, no closure** → on miss: + `TryBuildScope` + `Walk` + `Decisions.Set(...)`. +3. Deny-reason laziness: `Deny` currently interpolates at every call site before deciding + the level (`:310–325`, cached denials log Debug). Change the hot-path call sites + (`:214`, `:232`) to pass the format pieces and let `Deny` build the string only when + `_log.IsEnabled(...)` for the chosen level. Verify whether the SDK's `ServiceResult` + is immutable; if so return a cached + `static readonly ServiceResult DeniedResult = new(StatusCodes.BadUserAccessDenied);` + from the deny path — if anything mutates `ServiceResult` (check SDK source via + DeepWiki `OPCFoundation/UA-.NETStandard` if unsure), keep the per-call allocation and + note why in a comment. +4. The load-bearing ALLOW test + (`NodeManagerAclEnforcementTests.An_acl_grant_authorizes_a_write_from_a_session_carrying_no_roles`) + and all 17 `DataPlaneAuthorizerTests` facts must pass unmodified. + +**Tests (write first, in `DataPlaneAuthorizerTests.cs`):** +- `A_cache_hit_does_not_rebuild_the_scope` — a counting evaluator fake (existing test + fakes count `Authorize` calls): two identical `AuthorizeCached` calls ⇒ evaluator called + once; assert via the cache `Count == 1` that the second call was a hit. (The scope-build + itself is not directly observable — the evaluator count is the proxy: hit ⇒ no Walk ⇒ no + scope consumption.) +- `Key_revocation_is_rechecked_on_a_cache_hit` — prime an allow, revoke the key in the + snapshot, same call ⇒ denied (pins the ordering of step 2). + +**Commit:** `perf(acl): authorizer hot path — cache lookup before scope build, lazy deny reasons` + +--- + +### Task 8: Browse and TranslateBrowsePath compaction + +**Classification:** standard +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs` (`Browse` `:2919–2963`, `TranslateBrowsePath` `:2998–3005`) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs` + +**Spec:** +1. Replace both backwards `RemoveAt(ii)` loops with forward two-pointer compaction over + the appended range `[before, Count)`: walk forward, copy each permitted reference down + to a `write` cursor, then remove the tail once + (`for (var ii = references.Count - 1; ii >= write; ii--) references.RemoveAt(ii);` — + or `RemoveRange` if the concrete `IList` supports it; it is typed `IList`, so the + tail loop of RemoveAt-at-end is O(removed) with no element moves). Semantics + unchanged: same `Permits` gate per reference, same `filtered` count, same + `LogAclFiltering` call. +2. Apply identically to the `targetIds` loop in `TranslateBrowsePath`. + +**Tests (write first):** extend the existing browse-filtering coverage in +`NodeManagerStage6EnforcementTests` with an order-preservation case: grant on nodes +`{1st, 3rd, 5th}` of a 6-child folder ⇒ result is exactly those three **in original +order** (the compaction bug this guards against is duplicated/reordered survivors). +The over-the-wire suite (`AclSubscriptionEnforcementTests.Browse_returns_only_the_granted_subtree`) +stays green untouched. + +**Commit:** `perf(acl): O(n) browse/translate filtering compaction` + +--- + +### Task 9: RevalidateMonitoredItems dedup + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 4, Task 7, Task 13, Task 17, Task 21, Task 25 (blocked by Task 8 — same file) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs` (`RevalidateMonitoredItems` `:3173–3221`) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/NodeManagerStage6EnforcementTests.cs` + +**Spec:** +1. Inside the walk, memoize the verdict per distinct + `(sessionKey, nodeIdentifier)` in a local + `Dictionary<(string, string), bool>` — sessionKey = + `identity.ApiKeyId ?? identity.Principal` (same derivation the authorizer uses), node + identifier = `(string)tracked.Handle.NodeId.Identifier`. N monitored items on the same + node from the same session now cost one `IsAllowedCached` evaluation instead of N. + Items whose identity is not a `RoleCarryingUserIdentity` keep today's per-item path. +2. Leave the per-item `lock (Lock)` takes as-is (attach/detach must hold the SDK lock; + batching them under one hold would extend the lock across `QueueValue` calls — out of + scope, note as considered-and-rejected in a comment). + +**Tests (write first):** a unit test in `NodeManagerStage6EnforcementTests` (the rig +there constructs the node manager with a fake authorizer): create 3 monitored items on +the same node for the same identity via `OnMonitoredItemCreated`, flip authority, call +`RevalidateMonitoredItems()`, assert the counting authorizer evaluated that (session, +node) **once** — falsification control: 3 items on 3 distinct nodes ⇒ 3 evaluations. + +**Commit:** `perf(acl): dedup subscription revalidation by (session, node)` + +--- + +## Stream C — Driver round-trips (review finding 05, High) + +Current facts (all line-verified): S7 reads one tag per wire request under a single gate +(`S7Driver.ReadAsync` `:503–599`, per-tag `ReadOneAsync` `:601–636`), with every address +pre-parsed at init into `S7ParsedAddress(Area, DbNumber, Size, ByteOffset, BitOffset)` +(`:260–267`) and a contiguous bulk primitive already on the seam +(`IS7Plc.ReadBytesAsync`, `IS7Plc.cs:44`; no multi-var read — **multi-var PDUs are a +recorded follow-up, not in scope**). AbCip awaits each fallback tag strictly sequentially +(`AbCipDriver.ReadAsync` `:576–600`); the libplctag handle is per-tag async-capable but +NOT concurrency-safe per handle (per-tag `GetRuntimeLock` exists, `:1398–1414`), and +results are index-addressed. `PollGroupEngine` paces delay-after-completion with zero +instrumentation (`PollGroupEngine.cs:142–195`) and `Unsubscribe` blocks up to 5 s +(`:114–137`); the engine's project (`Core.Abstractions`) has **no references at all**, so +metrics are exposed as a counter + callback, not a Meter. The Modbus `InitializeAsync` +transport overwrite-without-dispose is confirmed still present (`ModbusDriver.cs:265`), +reachable via `DriverInstanceActor.AdoptConfigDuringInit` (`:752–764`); MQTT's +teardown-first pattern (`MqttDriver.cs:946–1033`) is the template, and Modbus's own +`ReadCoalescedAsync` (`:787+`) is the in-repo prior art for span planning. + +### Task 10: Modbus InitializeAsync transport leak + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs` (`InitializeAsync` `:252–307`) +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus.Tests/ModbusLifecycleHygieneTests.cs` + +**Spec:** +1. At the top of `InitializeAsync` (before `_transport = _transportFactory(_options)` at + `:265`), dispose what a prior initialize published: cancel+dispose `_probeCts` and + `_reprobeCts` (null-tolerant, the `TeardownAsync` `:1675–1702` idiom), and + `if (_transport is not null) await _transport.DisposeAsync().ConfigureAwait(false); + _transport = null;`. Do **NOT** call full `TeardownAsync()` — it disposes `_poll` + (the ctor-created engine, never rebuilt) and clears the tag maps; scope the fix to the + three leaked resources, with a comment naming `AdoptConfigDuringInit` as the reachable + caller (mirror the MQTT comment at `MqttDriver.cs:952–961`). +2. First initialize sees three nulls — no behaviour change. + +**Tests (write first, in `ModbusLifecycleHygieneTests.cs` using the existing +`DisposeCount` fake transport at `:23`):** +- `A_second_InitializeAsync_disposes_the_first_transport` — init twice with a factory + returning distinct fakes ⇒ first fake `DisposeCount == 1`, second `== 0`, driver + healthy on the second config. + +**Commit:** `fix(modbus): dispose the previous transport and probe loops on re-initialize` + +--- + +### Task 11: PollGroupEngine fixed-rate pacing, overrun counter, async unsubscribe + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/PollGroupEngine.cs` +- Modify (mechanical call-site swap to the async unsubscribe): `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Modbus/ModbusDriver.cs:1250–1252`, `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Focas/FocasDriver.cs:841–843`, `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCat/TwinCatDriver.cs:579–588`, `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbLegacy/AbLegacyDriver.cs:492–494`, `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs:1442–1444`, plus the AbCip and Sql engine call sites (locate `\.Unsubscribe\(` in those two drivers — same one-line swap) +- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions.Tests/PollGroupEngineTests.cs` + +**Spec:** +1. **Fixed-rate pacing:** in `PollLoopAsync` (`:142–195`), stamp + `Stopwatch.GetTimestamp()` before `PollOnceAsync`, compute + `elapsed = Stopwatch.GetElapsedTime(start)` after it, and delay + `max(interval - elapsed, TimeSpan.Zero)` instead of the full interval. When + `elapsed > interval` increment a `long _overrunCount` (Interlocked; expose as + `public long OverrunCount`) and invoke the new optional ctor callback + `Action? onOverrun = null` (wrapped in try/catch like `ReportError`). The + failure/backoff branch is untouched: when `consecutiveFailures > 0` and a backoff cap + is configured, `ConnectionBackoff.ComputeDelay` still wins (a failing device must not + be hammered at fixed rate). +2. **Async unsubscribe:** add + `public async ValueTask UnsubscribeAsync(ISubscriptionHandle handle)` — same + TryRemove, then cancel + `await task.WaitAsync(TimeSpan.FromSeconds(5))` under + try/catch (the `DisposeAsync` `:272–295` shape) + CTS dispose. Keep the sync + `Unsubscribe` for compatibility (tests use it), sharing the teardown via a small + private core. Swap every driver's `Task UnsubscribeAsync(...)` body from the blocking + `_poll.Unsubscribe(h)` to `await _poll.UnsubscribeAsync(h)` — this unblocks the + `DriverInstanceActor` mailbox turn that awaits it (`DriverInstanceActor.cs:482`, + `:894`). +3. No Meter here — `Core.Abstractions` deliberately references nothing; the counter + + callback is the instrumentation surface (record in the class doc; wiring a meter at + driver level is a follow-up). + +**Tests (write first, in `PollGroupEngineTests.cs`, real-delay style like the existing +suite):** +- `A_slow_reader_overrun_is_counted_and_the_next_poll_starts_promptly` — reader that + delays 150 ms on interval 50 ms: after ~4 polls `OverrunCount >= 2` and the observed + poll count over 1 s is ≥ 5 (fixed-rate; the old delay-after-completion shape would give + ≤ 5 in 1 s at 200 ms/cycle — assert with slack, this suite tolerates CI jitter with + `WaitForAsync`). +- `A_fast_reader_never_overruns` — instant reader, `OverrunCount == 0` after several + cycles (falsification control). +- `UnsubscribeAsync_stops_callbacks_like_the_sync_path` — mirror the existing + `Unsubscribe` assertions through the async API. + +**Commit:** `perf(drivers): fixed-rate poll pacing with overrun counter; non-blocking unsubscribe` + +--- + +### Task 12: S7 narrow-scalar span codec + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs` (new `internal static` helpers next to `DecodeScalarBlock` `:753+`) +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanCodecTests.cs` (new) + +**Spec:** +1. `internal static object DecodeNarrowScalar(S7TagDefinition tag, S7ParsedAddress addr, ReadOnlySpan slice)` + — decodes the narrow scalars that today go through `plc.ReadAsync(string)` + + `ReinterpretRawValue` (`:632`, `:1000`) from a big-endian byte slice positioned at the + tag's own offset: `Bool` from `slice[0]` bit `addr.BitOffset`; `Byte`/`SByte`; + `Int16`/`UInt16` from the BE word; `Int32`/`UInt32`/`Float32` from the BE dword. + **Semantic parity with `ReinterpretRawValue` is the requirement** — read that method + first and mirror its sign/width handling exactly (it is the reference behaviour the + integration parity test in Task 13 will compare against). +2. `internal static int NarrowScalarWidth(S7ParsedAddress addr)` — `Bit`/`Byte` ⇒ 1, + `Word` ⇒ 2, `DWord` ⇒ 4 (align with `ElementByteSize` `:868`). + +**Tests (write first, table-driven `[Theory]` over the S7DataType × slice matrix):** +- known BE byte patterns decode to the exact expected CLR values, including sign edges + (`0x8000` ⇒ Int16 -32768 / UInt16 32768), float bit patterns, and bit extraction for + `BitOffset` 0 and 7. + +**Commit:** `feat(s7): pure narrow-scalar decode from byte spans` + +--- + +### Task 13: S7 span planner + grouped ReadAsync + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 4, Task 7, Task 9, Task 17, Task 21, Task 25 (blocked by Task 12 — same file) + +**Files:** +- Create: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7ReadSpanPlanner.cs` +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7/S7Driver.cs` (`ReadAsync` `:503–599`) +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Contracts/S7DriverOptions.cs` (or wherever `S7DriverOptions` lives — locate it; the options type the ctor takes) +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7ReadSpanPlannerTests.cs` (new) +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs` (new) + +**Spec:** +1. Options: `SpanReadMaxGapBytes` (int, default `16`; `<= -1` disables span grouping) and + `SpanReadMaxSpanBytes` (int, default `220` — under one 240-byte PDU's payload; + `ReadBytesAsync` would transparently chunk bigger spans, the cap bounds wasted gap + bytes, not correctness). +2. `S7ReadSpanPlanner.Build(IReadOnlyList<(int OriginalIndex, S7TagDefinition Tag, S7ParsedAddress Addr)> resolved, int maxGapBytes, int maxSpanBytes)` + — pure function, Modbus-`ReadCoalescedAsync`-shaped (`ModbusDriver.cs:787+` is the + reference): **eligible** = everything except `Area is Timer or Counter` (their + `ByteOffset` is a device number, not an address) and except array tags with + `ArrayCount >= 1` only if mixing them complicates width math — arrays ARE eligible + (width = `ArrayCount * ElementByteSize`), strings are eligible (width = + `StringLength + 2` via `ScalarByteWidth`). Group by `(Area, DbNumber)`, sort by + `ByteOffset`, merge while `nextStart - currentEnd <= maxGapBytes` and + `mergedSpan <= maxSpanBytes`. **A span never extends past the last member's end** — + gap bytes between members are read (they exist inside the DB between two real tags), + but nothing beyond the highest member end is requested, so a span can never fail where + the individual reads would have succeeded. Output: spans (area, db, start, length, + members with relative offsets) + leftovers (ineligible / disabled ⇒ per-tag path). +3. `ReadAsync`: after resolving references (keep the exact unresolved/`BadNodeIdUnknown` + handling), when grouping is enabled build the plan and execute one + `ReadBytesAsync(ToS7NetArea(area), db, start, length, ct)` per span under the existing + `_gate`, then decode each member by slicing (`DecodeNarrowScalar` from Task 12 for + narrow scalars, `DecodeScalarBlock`/`DecodeArrayBlock` for buffer types — both take a + block starting at the tag's offset, which the slice provides). Per-span failure maps + through the **existing** exception taxonomy (`:552–595`: OCE ⇒ `_plcDead`+throw, + access-denied ⇒ `BadNotSupported`+Faulted, PlcException ⇒ `BadDeviceFailure`+Degraded, + else ⇒ `BadCommunicationError`+Degraded) applied to every member of the failed span — + extract that catch ladder into a private helper so the span path and the per-tag path + share it verbatim. Leftover tags keep today's `ReadOneAsync` path. Health writes stay + as today (whole method already runs under `_gate`, single-threaded — no new races). +4. `minInterval`/subscription flow untouched — this is inside the reader delegate. + +**Tests (write first):** +- `S7ReadSpanPlannerTests`: contiguous tags merge; gap > max splits; span cap splits; + distinct DBs never merge; Timer/Counter excluded; span end == last member end (assert + no span length overshoots). +- `S7SpanReadParityTests` — **the falsification gate**: a fake `IS7Plc` holding a byte + image per (area, db) serving both `ReadBytesAsync` (slice of the image) and `ReadAsync` + (decode from the same image at the parsed address — implement in the fake); a mixed + tag set (bits at both bit offsets, words, dwords, a float, a string, an array, with + gaps) read through a grouping driver and a `SpanReadMaxGapBytes = -1` driver ⇒ + **snapshot-for-snapshot identical values and statuses**. Plus + `A_span_read_failure_degrades_every_member_like_the_per_tag_path` (fake throws from + `ReadBytesAsync` ⇒ all members `BadDeviceFailure`/`BadCommunicationError` per the + exception type, health Degraded). + +**Commit:** `perf(s7): span-grouped reads — one PDU per contiguous block instead of one per tag` + +--- + +### Task 14: AbCip bounded-parallel reads + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs` (`ReadAsync` `:576–600`) +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs` (+ the factory DTO mapping in `AbCipDriverFactoryExtensions.cs`) +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipParallelReadTests.cs` (new) + +**Spec:** +1. Option `MaxConcurrentReads` (int, default `8`, clamp ≥ 1) on `AbCipDriverOptions` + + DTO + factory mapping (follow the `EnableDeclarationOnlyUdtGrouping` plumbing at + `AbCipDriverFactoryExtensions.cs:87`, `:148`). +2. `ReadAsync`: replace the two sequential `foreach`-await loops with one bounded-window + execution over `plan.Groups ∪ plan.Fallbacks` (a `SemaphoreSlim(maxConcurrent)` + + `Task.WhenAll`, the Task 3 shape). Safety rails, all already in the code and to be + preserved: the per-tag `GetRuntimeLock` serialises Read→GetStatus→Decode against the + shared handle (05/STAB-4, `:629–630` comment) — the window parallelises **across** + tags, never within one; results are index-addressed into `results[OriginalIndex]` + (`:576–600`), so completion order is irrelevant; `EnsureTagRuntimeAsync`'s + duplicate-create race is already handled (`:1013–1018`). +3. **Health writes must not race**: inspect how `ReadSingleAsync`/`ReadGroupAsync` write + driver health today; hoist to a post-`WhenAll` aggregation — collect each task's + outcome (`Healthy`/`Degraded`/`Faulted` + message) and write the health field **once** + after the batch with worst-outcome-wins (Faulted > Degraded > Healthy), + `LastSuccessfulRead` advanced if any tag succeeded. This preserves the observable + contract of `GetHealth()` that the comms-loss watchdog depends on + (CLAUDE.md §"Comms-loss detection") — `LastSuccessfulRead` still advances on any + success, still freezes on total failure. +4. Caller cancellation: propagate OCE like today (a cancelled batch may leave some tasks + unstarted — fine; `Task.WhenAll` rethrow after ensuring the window can't deadlock — + release in `finally`). + +**Tests (write first, `FakeAbCipTag`-based like `AbCipDriverReadTests.cs:12–35`):** +- `Reads_overlap_up_to_the_window` — a `FakeAbCipTag` subclass whose `ReadAsync` blocks + on a shared TCS and records max-concurrent (Interlocked): 4 tags, window 8 ⇒ observed + concurrency ≥ 2; window 1 ⇒ exactly 1 (falsification control). +- `Same_tag_requested_twice_is_still_serialised` — two references to the same tag name in + one batch ⇒ the fake's per-instance concurrent-read count never exceeds 1 (the runtime + lock holds). +- `Health_after_a_mixed_batch_is_worst_outcome_with_advanced_LastSuccessfulRead` — one + succeeding + one throwing tag ⇒ `Degraded` and `LastSuccessfulRead != null`. + +**Commit:** `perf(abcip): bounded-parallel tag reads (MaxConcurrentReads, default 8)` + +--- + +## Stream D — Alarm-engine coalescing (review finding 06, High) + +Current facts (zero drift from the review): every upstream push spawns an unthrottled +fire-and-forget `ReevaluateAsync` that queues on the single `_evalGate` +(`src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs:507–515`, gate +`:146`), with `_store.SaveAsync` inside the gate (`:539`) and a 250 ms +`Task.Run`+`WaitAsync` per predicate (`TimedScriptEvaluator.cs:78–81`) — N pushes during +one slow evaluation ⇒ N serialized re-evaluations. Crucially `RefillReadCache` (`:651–657`) +always reads the **latest** `_valueCache` values, so coalescing loses nothing by design. + +**Design decision (differs from the review's write-behind suggestion, deliberately):** +`SaveAsync` stays inside the gate, before the in-memory update. The persist-before-update +ordering is pinned by `Store_save_failure_leaves_in_memory_state_unchanged` (`:602–628`) +and is a real crash-consistency property (a store failure must leave both sides at the +prior state). A write-behind queue would trade that away for latency the dirty-set +coalescing already removes: post-coalescing, saves happen only on genuine transitions +(rare), not per push. Record this rationale in the engine's class doc. + +### Task 15: Dirty-set coalescing in ScriptedAlarmEngine + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms/ScriptedAlarmEngine.cs` +- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Core.ScriptedAlarms.Tests/ScriptedAlarmEngineTests.cs` + +**Spec:** +1. Add engine state: + ```csharp + private readonly HashSet _dirtyAlarmIds = new(StringComparer.Ordinal); + private readonly object _dirtyLock = new(); + private bool _drainScheduled; // guarded by _dirtyLock + ``` +2. `OnUpstreamChange` (`:507–515`): keep the `_valueCache` write; replace the per-push + `TrackBackgroundTask(ReevaluateAsync(alarmIds.ToArray(), ...))` with: under + `_dirtyLock`, union the referencing alarm ids into `_dirtyAlarmIds` (no `ToArray` + allocation); if `_drainScheduled` is false, set it and + `TrackBackgroundTask(DrainDirtyAsync(CancellationToken.None))`. A push landing while a + drain is running just grows the set — the running drain's loop picks it up. +3. `DrainDirtyAsync`: loop — under `_dirtyLock` swap the set out + (`var batch = _dirtyAlarmIds.ToArray(); _dirtyAlarmIds.Clear();` — when empty, clear + `_drainScheduled` and exit); then run the **existing** `ReevaluateAsync(batch, ct)` + body per swap (gate → per-alarm evaluate-latest → persist-before-update → release → + fire emissions). Reuse `ReevaluateAsync` as-is by calling it — its shape is already + correct for a batch; only the spawn-per-push is being replaced. **Keep the field name + `_evalGate` and its acquire/release shape** — + `OnEvent_emission_happens_outside_evalGate` (`:754–807`) reflects on that private + field by name. +4. `RunShelvingCheck` (`:831–836`) can keep its direct spawn (5 s cadence, not a storm + path) — but route it through the dirty set if trivially possible (union all ids + + schedule) for one code path. +5. `Dispose` drain semantics unchanged: `DrainDirtyAsync` is tracked via + `TrackBackgroundTask`, so `Dispose_drains_in_flight_reevaluation_tasks` (`:816–852`) + holds. The `_disposed` re-check inside the gate (`:528`) stays. + +**Tests (write first):** +- `A_storm_of_pushes_coalesces_to_one_evaluation_per_alarm` — a save-and-eval-counting + setup: block the gate via `BlockingSaveAlarmStateStore` (`:1231–1271`) mid-first-eval, + push the same tag 10 more times, release ⇒ the alarm's predicate evaluated **at most + 2** more times (the in-flight one + one drain pass over the coalesced set), not 10. + Count evaluations via a script that increments a static/captured counter through + `ctx` — or simpler: a real script `return (int)ctx.GetTag("Temp").Value > 100;` and a + save-counting store decorator (add one: `CountingAlarmStateStore` wrapping + `InMemoryAlarmStateStore`) asserting transitions, plus assert via the counting store + that no redundant saves happened. Include the falsification control: 10 pushes with + **alternating** values that each cross the threshold ⇒ the final state reflects the + **latest** value (coalescing is latest-wins, never lost-update). +- `A_push_during_a_drain_is_not_lost` — push while the drain is inside the gate ⇒ a + second drain pass runs and evaluates it. +- All existing engine facts pass unmodified — pay attention to the `Task.Delay(100/150)` + quality tests (`:667–694` etc.): the drain must start immediately when the gate is + free (no timer/delay in the scheduling path — the coalescing window is "whatever + queued while the gate was busy", not a wall-clock batch window). + +**Commit:** `perf(alarms): dirty-set coalescing — one evaluation per alarm per drain, latest values win` + +--- + +## Stream E — AdminUI hot spots (review finding 09, Medium) + +Current facts: `/deployments` materialises 50 full `Deployment` entities including +`ArtifactBlob` and runs a full 18-table `ConfigComposer.SnapshotAndFlattenAsync` compose +per visit (`Deployments.razor:80–99`); the Monaco catalog rebuilds six fleet-wide table +scans per completion/hover call with **zero caching anywhere in the solution** +(`ScriptTagCatalog.BuildEntriesAsync`, `IScriptTagCatalog.cs:159–219`; `/completions` +fires per keystroke inside string literals — `monaco-init.js:33,198`); bulk add-references +runs a fresh DbContext + 4 queries per tag (`EffectiveNameGuard.cs:21–88`, loop at +`UnsTreeService.cs:214–242`); the raw tree loads full `Tag` entities (with `TagConfig` +nvarchar(max)) to render three fields (`RawTreeService.cs:163–203`); every raw rename +loads every script body (`:1415–1436`); the context factory is **not** pooled despite +comments claiming it is (`Configuration/ServiceCollectionExtensions.cs:25`); RoleGrants +pins a scoped DbContext for the circuit via `LdapGroupRoleMappingService`; `/fleet` and +`/hosts` poll on 10 s / 5 s timers with no overlap guard. AdminUI tests are pure EF +InMemory (no SQL Server needed). + +### Task 16: Deployments list projection + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Deployments.razor` + +**Spec:** +1. Replace the full-entity load (`:83–87`) with a blob-free projection: + ```csharp + private sealed record DeploymentRow( + Guid DeploymentId, string RevisionHash, DeploymentStatus Status, + string CreatedBy, DateTime CreatedAtUtc, DateTime? SealedAtUtc); + ``` + `.Select(d => new DeploymentRow(...))` before `ToListAsync()`. The table (`:54–64`) + renders exactly these six columns; the drift check needs only + `Status`/`RevisionHash` — adjust `latestSealed` accordingly. No test project covers + this page (verified); the compile + the Task 17 tests cover the drift seam. + +**Commit:** `perf(adminui): /deployments lists a projection, not 50 artifact blobs` + +--- + +### Task 17: Drift watermark probe + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** Task 4, Task 7, Task 9, Task 13, Task 21, Task 25 (blocked by Task 16 — same razor file) + +**Files:** +- Create: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Deployments/DeploymentDriftProbe.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Deployments.razor` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs` (register singleton) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Deployments/DeploymentDriftProbeTests.cs` (new) + +**Spec:** +1. `DeploymentDriftProbe` — singleton over `IDbContextFactory`: + `Task GetDriftAsync(string latestSealedRevisionHash, CancellationToken ct)`. + Watermark = one cheap aggregate query over `ConfigEdit` + (`MAX(EditedAtUtc)`, `COUNT(*)` — a single `GroupBy(_ => 1).Select(...)` or two + scalars; `ConfigEdit` is the append-only audit row every mutating live-edit writes, + so any topology change moves it). Cache + `(watermark, sealedHash) → bool drift` under a `SemaphoreSlim(1,1)`; recompute + `ConfigComposer.SnapshotAndFlattenAsync` **only** when the watermark or the sealed + hash changed. Comment the accepted staleness: an edit path that bypasses + `ConfigEdit` would not invalidate — acceptable for a UI chip, and today every + AdminOperationsActor mutation writes one. +2. `Deployments.razor.ReloadAsync` drift branch (`:89–98`) delegates to the probe. + +**Tests (InMemory factory pattern from `UnsTreeTestDb.cs:144–150`):** +- `An_unchanged_watermark_reuses_the_cached_verdict` — counting decorator impossible + here, so assert indirectly: seed, call twice, mutate nothing ⇒ second call returns + without recompose. Make the compose seam injectable + (`Func> composeRevisionHash` ctor parameter + defaulting to the real `SnapshotAndFlattenAsync(...).RevisionHash`) so the test injects + a counting fake ⇒ compose count == 1 after two calls. +- `A_new_config_edit_recomposes` — add a `ConfigEdit` row between calls ⇒ compose count + == 2, and drift flips when the fake returns a different hash. + +**Commit:** `perf(adminui): drift chip recomposes only when the config-edit watermark moves` + +--- + +### Task 18: ScriptTagCatalog TTL snapshot cache + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 19, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/ScriptAnalysis/IScriptTagCatalog.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/EndpointRouteBuilderExtensions.cs` (`:98` area — register the cache singleton) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/ScriptAnalysis/ScriptTagCatalogTests.cs` + +**Spec:** +1. New `ScriptTagCatalogSnapshotCache` (singleton, in the same file or adjacent): + holds `(IReadOnlyList Entries, long StampTicks)`; `TimeProvider` ctor + dependency; TTL const `5 s`. `GetOrBuildAsync(Func>> build, ct)` + — double-checked under a `SemaphoreSlim(1,1)` so concurrent circuits share one + rebuild. Staleness bound is deliberate and documented: completions may lag a + just-saved tag by ≤ 5 s. +2. `ScriptTagCatalog` (stays scoped) takes the singleton cache; `GetPathsAsync` and + `GetTagInfoAsync` source entries from `cache.GetOrBuildAsync(BuildEntriesAsync, ct)`. + `GetTagInfoAsync`'s linear scan can stay (list is in memory now). + `GetEquipmentReferenceNamesAsync` (2 scoped queries) is untouched. + +**Tests (extend `ScriptTagCatalogTests.cs`, which already builds the catalog over an +InMemory factory):** +- `Two_calls_within_ttl_scan_the_db_once` — a counting `IDbContextFactory` decorator + (wrap `TestFactory`, count `CreateDbContextAsync`) ⇒ second `GetPathsAsync` creates no + context. +- `An_expired_ttl_rebuilds` — `Microsoft.Extensions.Time.Testing.FakeTimeProvider` + advanced 6 s ⇒ context created again, and a tag added between calls appears. + +**Commit:** `perf(adminui): 5s snapshot cache in front of the script tag catalog` + +--- + +### Task 19: EffectiveNameGuard batch check + bulk add-references + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 20, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/IEffectiveNameGuard.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/EffectiveNameGuard.cs` +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/UnsTreeService.cs` (`AddReferencesAsync` `:171–257`) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/EffectiveNameGuardTests.cs`, `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/UnsTreeServiceReferenceTests.cs` + +**Spec:** +1. Add to the interface (default-implemented in terms of `CheckAsync` so the + `NoOpEffectiveNameGuard` and the test `FakeGuard` keep compiling — or implement on + both, whichever is cleaner): + ```csharp + /// Checks many proposed names against ONE load of the equipment's name space. + /// Returns the first collision message, or null when all are free. + Task CheckManyAsync(string equipmentId, IReadOnlyList proposedEffectiveNames, + EffectiveNameSourceKind kind, CancellationToken ct = default); + ``` +2. `EffectiveNameGuard.CheckManyAsync`: one DbContext, the same 4 per-equipment queries + (`:34`, `:41`, `:59`, `:73`) loaded **once**, then all names checked in memory with the + same collision rules/messages as `CheckAsync` (extract the shared in-memory check). +3. `UnsTreeService.AddReferencesAsync`: hoist the guard out of the loop — collect the + effective names during the (kept) per-tag validation loop, then one + `CheckManyAsync(equipmentId, names, Reference, ct)` before building `pending`. + N-tag bulk add: N+ contexts + 4N queries → 1 context + 4 queries. + +**Tests:** batch-vs-loop parity in `EffectiveNameGuardTests` (same collisions detected, +same messages, first-collision-wins ordering); update `UnsTreeServiceReferenceTests`' +`FakeGuard` and assert a 3-tag add calls the guard **once**. + +**Commit:** `perf(adminui): one guard round-trip per bulk reference add` + +--- + +### Task 20: Raw tree tag-name projection + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 22, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs` (`LoadDeviceChildrenAsync` `:163–182`, `LoadTagGroupChildrenAsync` `:184–203`, `BuildTagNode` `:302–313`) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTreeServiceTests.cs` + +**Spec:** project the two tag queries to exactly what `BuildTagNode` consumes — read +`BuildTagNode` first and enumerate its `Tag` member accesses (exploration saw `TagId`, +`Name`, `RowVersion` + whatever else the method touches); introduce a private record for +the projection and a `BuildTagNode` overload taking it. `TagConfig` (nvarchar(max) JSON) +must not be in the SELECT. Existing `RawTreeServiceTests` green. + +**Commit:** `perf(adminui): raw tree renders tag rows from a projection without TagConfig` + +--- + +### Task 21: Rename script-scan filters in SQL + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 4, Task 7, Task 9, Task 13, Task 17, Task 25 (blocked by Task 20 — same file) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawTreeService.cs` (`BuildScriptReferenceWarningAsync` `:1415–1436`) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawTreeServiceRenameWarningTests.cs` + +**Spec:** push the substring match into the query so unmatched script **bodies** never +leave the database: build a disjunction predicate over `oldPaths` by composing +`Expression.OrElse` around `s.SourceCode.Contains(path)` (EF translates +`string.Contains` to `CHARINDEX` on SQL Server and evaluates in-memory on InMemory — +both fine; write a small private +`static Expression> AnyPathContained(IReadOnlyList paths)` +helper with a shared parameter). Project `{ Name }` only. Guard the degenerate case +(`oldPaths.Count == 0` returns null — already there). Keep the ordinal semantics note: +SQL Server `CHARINDEX` under the column's collation may be case-insensitive where +`StringComparison.Ordinal` was not — RawPaths are operator-authored identifiers; accept +the (strictly wider) match and say so in a comment, since a false-positive warning is +harmless and a false negative was the old in-memory scan's real risk profile anyway. +Existing `RawTreeServiceRenameWarningTests` must pass unmodified (they pin match +behaviour on InMemory, where `Contains` stays ordinal). + +**Commit:** `perf(adminui): rename script scan filters server-side, stops shipping every script body` + +--- + +### Task 22: Pooled context factory + RoleGrants off the circuit-scoped context + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 23, Task 24, Task 27 + +**Files:** +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/ServiceCollectionExtensions.cs` (`:19–37`) +- Modify: `src/Core/ZB.MOM.WW.OtOpcUa.Configuration/Services/LdapGroupRoleMappingService.cs` +- Test: `tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/` (locate the existing `LdapGroupRoleMappingService` tests, adjust construction) + +**Spec:** +1. `AddDbContextFactory` → `AddPooledDbContextFactory` (`:25`). Precondition to verify + before the swap: `OtOpcUaConfigDbContext` has a single public + `DbContextOptions`-taking ctor and no injected scoped services (exploration saw the + standard shape at `UnsTreeTestDb.cs` construction) — if anything else shows up, stop + and surface it. +2. Refactor `LdapGroupRoleMappingService` from a ctor-injected scoped + `OtOpcUaConfigDbContext` to `IDbContextFactory` — create + + await-dispose a context per method (`ListAllAsync`/`CreateAsync`/`DeleteAsync` and any + siblings). This unpins the RoleGrants circuit's long-lived context AND makes the + service safe under Blazor's concurrent renders. Then delete the scoped + `CreateDbContext()` bridge at `:30` **if** `LdapGroupRoleMappingService` was its only + consumer — grep for other ctor-injected `OtOpcUaConfigDbContext` takers first + (`AuthEndpoints.LoginAsync` reaches the service, not the context, per the comment); + if another consumer exists, keep the bridge and note it. +3. Fix the three stale "pooled factory" comments exploration found + (`IScriptTagCatalog.cs:82–84`, `EffectiveNameGuard.cs:15`, `UnsTreeTestDb.cs:142–143`) + — they become true; reword to state pooling is now real. + +**Commit:** `perf(adminui): pooled DbContext factory; role-grants reads per-call, not per-circuit` + +--- + +### Task 23: Fleet/Hosts poll overlap guards + +**Classification:** small +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 24, Task 27 + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Fleet.razor` (`:102–171`) +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Pages/Hosts.razor` (`:316–400`) + +**Spec:** in both timer callbacks add a `int _tickRunning` overlap guard +(`if (Interlocked.Exchange(ref _tickRunning, 1) == 1) return;` … `finally +Volatile.Write(ref _tickRunning, 0);`) so a slow DB tick never stacks behind the next, +and pass a page-lifetime `CancellationTokenSource` token into the EF calls, cancelled in +`Dispose`. No behaviour change otherwise; no page tests exist (verified) — compile + +manual `/run` sanity is the gate. + +**Commit:** `fix(adminui): fleet/hosts poll ticks no longer overlap; cancel on dispose` + +--- + +## Stream F — Residual fan-out waste + blocking I/O (review findings 07/08) + +Current facts: the mux `Tell` from `DriverHostActor.ForwardBatchToMux` (`:1137–1170`) is +one envelope per burst — with Task 2 in place, host-side interest gating is near-zero +value (**deliberately skipped**; the mux already early-outs and now emits one message per +interested subscriber). MQTT raises per message with no unchanged-value suppression +(`MqttSubscriptionManager.Deliver` `:703–753` → `Raise` `:762–784`) while +`LastValueCache.Update` runs immediately before the raise. The comms-loss fan +(`FanCommsLossToDependencyMux` `:1293–1312`) scans the whole `(driverId, rawPath)` map +and Tells one **per-value** message per ref; `OnDriverConnectivityChanged` (`:1219–1250`) +scans the alarm map the same way. The one remaining hot-mailbox blob read is the Direct +apply path (`LoadArtifactBlob` `:2034–2059` called from `ReconcileDriversForApply` +`:2080–2084`; second copy at `PushDesiredSubscriptions` `:2380–2384` on the restore +path); `UpsertNodeDeploymentState`'s sync `SaveChanges` (≤3 small row writes per apply) +and the PreStart-only bootstrap reads are **recorded follow-ups, not in scope** — they +are bounded per-deploy costs, not per-value costs. MTConnect's `/sample` chunks go +byte[] → string → trimmed string → XDocument (`MultipartStreamReader.ReadNextPartAsync` +`:655`, `MTConnectAgentClient.cs:254`) while a `Stream` overload already exists +(`MTConnectStreamsParser.cs:127–142`); serving on-demand reads from the live index was +**considered and rejected in-code** (`MTConnectDriver.cs:652–667` — a second writer +racing the pump) — honor that. + +### Task 24: MQTT unchanged-value suppression + +**Classification:** standard +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 27 + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttSubscriptionManager.cs` (`Deliver` `:703–753`, `Raise` `:762–784`, `DegradeTopic` `:679–683`) +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttSubscriptionManagerTests.cs` + +**Spec:** before `Values.Update(name, snapshot)` in `Deliver`, read the prior snapshot +(`Values.Read(name)` — it returns `BadWaitingForInitialData` for unseen, which never +equals a real value+status, so the **first** message always raises); when +`prior.Value` equals `snapshot.Value` (use the same structural comparison +`PollGroupEngine.ValuesAreDifferent` uses — replicate the array-aware compare locally or +extract; MQTT values are scalars/strings/byte[] from JSON) **and** +`prior.StatusCode == snapshot.StatusCode`, still `Values.Update` (freshen timestamps for +reads) but **skip `Raise`**. `DegradeTopic`'s Bad raise survives (status differs). +Timer-republishing brokers stop re-triggering the whole downstream fan per identical +message. + +**Tests (write first, `NewManager` harness `:29`):** +- `An_identical_republish_raises_nothing` — same topic/payload twice ⇒ one + `OnDataChange`; read still returns the value with the newer timestamp. +- `A_changed_value_after_a_suppressed_republish_raises` (falsification control). +- `A_status_transition_still_raises` — degrade after a good value ⇒ Bad raised. +- Sparkplug path: confirm `SparkplugIngestor` raises through its own seam (`MqttDriver.cs:1198`) + — if it bypasses `Deliver`, it is out of scope; note it either way in the commit body. + +**Commit:** `perf(mqtt): suppress unchanged-value republish fan-out` + +--- + +### Task 25: DriverHostActor per-driver index + batched comms-loss fan + +**Classification:** high-risk +**Estimated implement time:** ~4 min +**Parallelizable with:** Task 4, Task 7, Task 9, Task 13, Task 17, Task 21 (blocked by Task 2 — the mux batch message must exist) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs` (`ApplyDesiredSubscriptions` `:2506–2549`, `OnDriverConnectivityChanged` `:1219–1250`, `FanCommsLossToDependencyMux` `:1293–1312`) +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorLiveValueTests.cs` (or a new `DriverHostActorCommsLossTests.cs` beside it, using the `RuntimeActorTestBase` + `SeedV3Deployment` harness) + +**Spec:** +1. During `ApplyDesiredSubscriptions` (where `_nodeIdByDriverRef` / + `_alarmNodeIdByDriverRef` are rebuilt), also build + `Dictionary>` secondary indexes for both maps (same + value tuples, grouped). Rebuilt whole per apply like the primaries — no incremental + maintenance. +2. `OnDriverConnectivityChanged` and `FanCommsLossToDependencyMux` iterate the indexed + driver's list instead of scanning every entry. +3. Batch the comms-loss mux fan: instead of one `AttributeValuePublished` Tell per ref, + send one `DriverInstanceActor.AttributeValueBatchPublished` carrying every ref as a + `AttributeValueItem(rawPath, null, BadNoCommunication, ts)` — the mux batch path + (Task 2) fans it per interest. One driver dying with 5k tags: 5k Tells → 1. + +**Tests (write first):** `Comms_loss_reaches_the_mux_as_one_batch_of_bad_items` — seeded +deployment with ≥2 tags on the driver, trigger the connectivity-lost path, probe +registered for those refs expects one `DependencyValueBatch` whose items are all +`BadNoCommunication`; a second driver's tags are untouched (index correctness — +falsification control with two drivers). + +**Commit:** `perf(runtime): per-driver index for connectivity fans; comms-loss is one batch` + +--- + +### Task 26: Direct-apply artifact read off the mailbox + +**Classification:** high-risk +**Estimated implement time:** ~5 min +**Parallelizable with:** none (blocked by Task 25 — same file) + +**Files:** +- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs` +- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DriverHostActorApplyingStashTests.cs` (or a sibling file, same harness) + +**Spec:** +1. The Direct-mode dispatch path currently calls the sync `LoadArtifactBlob` + (`:2034–2059`, EF read of a potentially multi-MB gzip blob) inline on the mailbox + inside `ReconcileDriversForApply`/`ApplyAndAck`. Restructure Direct dispatch to the + **FetchAndCache shape that already exists in this file**: on dispatch, pipe + `Task.Run(() => LoadArtifactBlob(deploymentId))` via `.PipeTo(Self, success: bytes => + new , failure: ...)`, enter the `Applying` + state (which already stashes concurrent dispatches — Task history: item-2's stash + + wedge watchdog `:1063–1089`, `:1868–1895`), arm the same wedge watchdog, and run the + existing `HandleFetchedForApply`-style continuation when the bytes arrive. Reuse the + existing messages/continuations wherever they fit (`HandleFetchedForApply` `:1789–1862` + already handles in-hand bytes + parse + reconcile + acks); the goal is ONE apply + continuation for both modes, with only the byte-source differing. A failed load fails + the apply exactly like a failed fetch (#485 semantics: no bytes is no answer — never + tear down). +2. `RestoreApplied`'s boot-path read (`:2330`) and `PushDesiredSubscriptions`' restore + read (`:2380–2384`) may stay sync — they run at PreStart/restore, not in live traffic; + add a comment saying so. `TryRecoverFromStale`/`UpsertNodeDeploymentState` sync writes + are recorded follow-ups (bounded per-apply row writes), also commented. +3. Behavioural pins that must stay green: the whole + `DriverHostActorApplyingStashTests` suite (stash/dedup/wedge), `DriverHostActorReconcileTests`, + `DriverHostActorBatchFanOutTests`, and the Fleet ack flow tests. + +**Tests (write first):** `A_slow_direct_artifact_read_does_not_block_the_host_mailbox` +— gate the blob read (inject a gated `IDbContextFactory` or an artifact-load seam — +follow the `GatedArtifactCache` pattern from `DriverHostActorAsyncCacheWriteTests.cs:44`), +dispatch a Direct apply, and while the read is gated assert the host still answers an +unrelated message (e.g. forwards a value batch to the publish probe); release ⇒ apply +completes and acks Applied. + +**Commit:** `perf(runtime): direct-mode artifact read piped off the host mailbox` + +--- + +### Task 27: MTConnect sample chunks parse from bytes + +**Classification:** standard +**Estimated implement time:** ~3 min +**Parallelizable with:** Task 1, Task 2, Task 3, Task 6, Task 8, Task 10, Task 11, Task 12, Task 14, Task 15, Task 16, Task 18, Task 19, Task 20, Task 22, Task 23, Task 24 + +**Files:** +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectAgentClient.cs` (`:254`, `MultipartStreamReader.ReadNextPartAsync` `:616–655`) +- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs` (byte-span entry point) +- Test: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectStreamsParseTests.cs` + +**Spec:** stop materialising each streamed chunk as a string: +`ReadNextPartAsync` returns the raw `byte[]` (rename/overload; the string conversion at +`:655` moves to the one caller that still needs it, if any); add +`MTConnectStreamsParser.Parse(ReadOnlyMemory xml)` that wraps the bytes in a +`MemoryStream` and reuses the existing `Stream` overload (`:127–142`), handling the BOM +the string path trimmed (`XDocument.Load` handles a UTF-8 BOM natively — verify with a +BOM fixture). The `/sample` loop at `:254` uses the byte path. The DOM walk itself stays +(a full `XmlReader` streaming rewrite of `Build` is a recorded follow-up — this task +removes the double string copy, which is the allocation bulk). The on-demand `/current` +read design (`:652–667` throwaway index) is **deliberately untouched**. + +**Tests:** extend `MTConnectStreamsParseTests` with byte-input theories over the same +fixtures (`File.ReadAllBytes`), incl. a BOM-prefixed and a trailing-newline-suffixed +variant (the exact framing residue the string path trimmed at `:110`). + +**Commit:** `perf(mtconnect): sample chunks parse from bytes, no per-chunk string copies` + +--- + +### Task 28: Full sweep + plan close-out docs + +**Classification:** standard +**Estimated implement time:** ~5 min +**Parallelizable with:** none (blocked by all prior tasks) + +**Files:** +- Modify: `docs/plans/2026-08-15-perf-roadmap-remainder.md` (this file — outcomes section) +- Modify: `CLAUDE.md` (only where behaviour documented there changed: the + ContinuousHistorization table already updated in Task 5; check the drivers/`GetHealth` + and ACL sections for statements the work invalidated) +- Modify: `docs/security.md` (decision-cache sharding note), `docs/DriverLifecycle.md` + (fixed-rate pacing + overrun counter, async unsubscribe), `docs/ScriptedAlarms.md` + (dirty-set coalescing + the write-behind-rejected rationale) + +**Spec:** +1. `dotnet build ZB.MOM.WW.OtOpcUa.slnx` — zero errors/warnings-as-errors. +2. `dotnet test ZB.MOM.WW.OtOpcUa.slnx` — full unit sweep green (integration projects + build-only per repo convention; DB-backed suites need the central SQL Server at + `10.100.0.35,14330` reachable — if it is not, run every non-DB project individually + and record which were skipped). +3. Append an "## Outcomes" section here: per task, one line — done/deviation. Record the + follow-up register explicitly: HistorianGateway multi-tag batch RPC (cross-repo); + S7 multi-var PDUs for span leftovers; `UpsertNodeDeploymentState`/`TryRecoverFromStale` + sync writes; MTConnect `XmlReader` streaming `Build`; driver-level meter for the + engine overrun counter; host-side mux interest gating (skipped — batch envelope made + it moot). +4. Docs edits per Files list. + +**Commit:** `docs: perf roadmap remainder — outcomes + follow-up register` + +--- + +## Dependency graph (execution order) + +Wave 1 (parallel, disjoint files): 1, 2, 3, 6, 8, 10, 11, 12, 14, 15, 16, 18, 19, 20, 22, 23, 24, 27 +Wave 2 (parallel, after their blockers): 4 (←1,2,3), 7 (←6), 9 (←8), 13 (←12), 17 (←16), 21 (←20), 25 (←2) +Wave 3: 5 (←3,4), 26 (←25) +Wave 4: 28 (←all) + +**Not covered by this plan (explicit):** live docker-dev gating of the historian and +apply-path changes (run the existing live-gate recipes afterwards, as items 1–2 did); +the HistorianGateway contract change; any change to `redundancy-state`, the deploy +classifier, or the ACL trie itself. diff --git a/docs/plans/2026-08-15-perf-roadmap-remainder.md.tasks.json b/docs/plans/2026-08-15-perf-roadmap-remainder.md.tasks.json new file mode 100644 index 00000000..b73776b3 --- /dev/null +++ b/docs/plans/2026-08-15-perf-roadmap-remainder.md.tasks.json @@ -0,0 +1,34 @@ +{ + "planPath": "docs/plans/2026-08-15-perf-roadmap-remainder.md", + "tasks": [ + {"id": 1, "subject": "Task 1: Outbox batch append with group commit", "status": "pending"}, + {"id": 2, "subject": "Task 2: Batched mux→subscriber delivery, single-eval virtual tags", "status": "pending"}, + {"id": 3, "subject": "Task 3: Parallel drain with bounded concurrency", "status": "pending"}, + {"id": 4, "subject": "Task 4: Recorder batched ingest — one fsync per burst", "status": "pending", "blockedBy": [1, 2, 3]}, + {"id": 5, "subject": "Task 5: Historian defaults, config drift, docs", "status": "pending", "blockedBy": [3, 4]}, + {"id": 6, "subject": "Task 6: Per-session decision-cache shards", "status": "pending"}, + {"id": 7, "subject": "Task 7: Authorizer hot path — lookup first, no closures, lazy deny reasons", "status": "pending", "blockedBy": [6]}, + {"id": 8, "subject": "Task 8: Browse and TranslateBrowsePath compaction", "status": "pending"}, + {"id": 9, "subject": "Task 9: RevalidateMonitoredItems dedup", "status": "pending", "blockedBy": [8]}, + {"id": 10, "subject": "Task 10: Modbus InitializeAsync transport leak", "status": "pending"}, + {"id": 11, "subject": "Task 11: PollGroupEngine fixed-rate pacing, overrun counter, async unsubscribe", "status": "pending"}, + {"id": 12, "subject": "Task 12: S7 narrow-scalar span codec", "status": "pending"}, + {"id": 13, "subject": "Task 13: S7 span planner + grouped ReadAsync", "status": "pending", "blockedBy": [12]}, + {"id": 14, "subject": "Task 14: AbCip bounded-parallel reads", "status": "pending"}, + {"id": 15, "subject": "Task 15: Dirty-set coalescing in ScriptedAlarmEngine", "status": "pending"}, + {"id": 16, "subject": "Task 16: Deployments list projection", "status": "pending"}, + {"id": 17, "subject": "Task 17: Drift watermark probe", "status": "pending", "blockedBy": [16]}, + {"id": 18, "subject": "Task 18: ScriptTagCatalog TTL snapshot cache", "status": "pending"}, + {"id": 19, "subject": "Task 19: EffectiveNameGuard batch check + bulk add-references", "status": "pending"}, + {"id": 20, "subject": "Task 20: Raw tree tag-name projection", "status": "pending"}, + {"id": 21, "subject": "Task 21: Rename script-scan filters in SQL", "status": "pending", "blockedBy": [20]}, + {"id": 22, "subject": "Task 22: Pooled context factory + RoleGrants off the circuit-scoped context", "status": "pending"}, + {"id": 23, "subject": "Task 23: Fleet/Hosts poll overlap guards", "status": "pending"}, + {"id": 24, "subject": "Task 24: MQTT unchanged-value suppression", "status": "pending"}, + {"id": 25, "subject": "Task 25: DriverHostActor per-driver index + batched comms-loss fan", "status": "pending", "blockedBy": [2]}, + {"id": 26, "subject": "Task 26: Direct-apply artifact read off the mailbox", "status": "pending", "blockedBy": [25]}, + {"id": 27, "subject": "Task 27: MTConnect sample chunks parse from bytes", "status": "pending"}, + {"id": 28, "subject": "Task 28: Full sweep + plan close-out docs", "status": "pending", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27]} + ], + "lastUpdated": "2026-08-15T23:59:00Z" +}