docs(plan): perf follow-ups close-out — outcomes, live-gate evidence, register annotations
This commit is contained in:
Joseph Doherty
@@ -763,9 +763,10 @@ Two things to know before relying on any of this:
|
||||
`TriePermissionEvaluator`'s, so **any divergence between the two clocks denies every request** with a
|
||||
verdict indistinguishable from "no grant matched". Found by a test clock two hours ahead.
|
||||
- **Stage 6's hot seams (read/browse/subscribe) go through a per-`(session, node, operation, realm)`
|
||||
decision cache**, cleared wholesale when the generation advances. It is **sharded per session**, so a
|
||||
single client crawling a large tree past the per-session ceiling evicts only its own shard and not
|
||||
every other session's decisions. ⚠️ It caches the **grant walk only** — key revocation and expiry are
|
||||
decision cache**, cleared wholesale when the generation advances. It is **sharded per API key**
|
||||
(`ApiKeyId ?? Principal` — despite the type name, sessions sharing one key share one shard and one
|
||||
budget), so a client crawling a large tree past the per-key ceiling evicts only that key's shard and
|
||||
not every other key's decisions. ⚠️ It caches the **grant walk only** — key revocation and expiry are
|
||||
re-checked **before** the cache lookup on every call, because expiry moves with the clock rather than
|
||||
with a deployment.
|
||||
- **Browse needs ancestor visibility, and it must NOT be a synthesized grant on the ancestors.** A
|
||||
|
||||
@@ -1340,6 +1340,12 @@ whose message mentions it.
|
||||
|
||||
Open items the work surfaced but deliberately did not close. None blocks the branch.
|
||||
|
||||
> **Annotated 2026-08-17 by the `perf/followups` branch** (plan:
|
||||
> [`2026-08-17-perf-followups.md`](2026-08-17-perf-followups.md)). Items it closed carry a
|
||||
> ` — closed by <sha>, 2026-08-17` suffix; items with no suffix remain open. Annotations are
|
||||
> **append-only** — the original text of each item is untouched, including where the follow-up
|
||||
> work reached a different conclusion than the item predicted.
|
||||
|
||||
**Cross-repo**
|
||||
|
||||
- **HistorianGateway needs a multi-tag batch `WriteLiveValues` RPC.** Client `0.3.0` exposes
|
||||
@@ -1351,22 +1357,31 @@ Open items the work surfaced but deliberately did not close. None blocks the bra
|
||||
|
||||
- **S7: multi-variable PDUs for span leftovers.** The Task 13 planner groups contiguous
|
||||
reads into one PDU per span; tags that do not fall in a span are still one PDU each. A
|
||||
multi-var PDU would absorb them.
|
||||
multi-var PDU would absorb them. — closed by `20022083`, 2026-08-17 (per-chunk fallback:
|
||||
S7.Net throws for the whole multi-var request, so a bad item fails its chunk, not the pass;
|
||||
Timer/Counter areas are excluded on an int-vs-ushort box-parity difference).
|
||||
- ⚠️ **S7 span planner has no live gate.** The `16` (max gap) / `220` (max span bytes)
|
||||
defaults were chosen analytically and pinned only by unit tests. Run the planner against
|
||||
the `s7_1500` fixture (`lmxopcua-fix up s7 s7_1500`, `S7_SIM_ENDPOINT=10.100.0.35:1102`)
|
||||
before trusting them on real hardware.
|
||||
before trusting them on real hardware. — closed by `57c66416`, 2026-08-17 (**LIVE GATE
|
||||
PASSED** vs `10.100.0.35:1102`; 3 wire requests for 10 tags with spans on vs 10 with spans
|
||||
off).
|
||||
- **`SpanReadMaxSpanBytes` has no validation guard.** A value above the negotiated PDU size
|
||||
is accepted at config time and fails at read time.
|
||||
is accepted at config time and fails at read time. — closed by `acb37724`, 2026-08-17.
|
||||
- **MTConnect `Build` could stream through `XmlReader`.** Task 27 removed the per-chunk
|
||||
string copies; the document build still materialises.
|
||||
string copies; the document build still materialises. — closed by `cda82c07`, 2026-08-17
|
||||
(566/566 parity, ~19% allocation win; one disclosed error-ordering change on a
|
||||
double-broken document).
|
||||
- **No driver-level meter for the PollGroupEngine overrun counter.** The counter exists and
|
||||
is observable in-process but is not exported per driver.
|
||||
is observable in-process but is not exported per driver. — closed by `e22ecea0`, 2026-08-17
|
||||
(tag keys moved to snake_case consts in `97902d68` after review).
|
||||
- **`NoteOverrun` conflates a slow success with a slow failure** — a poll that overran
|
||||
because the device was slow and one that overran because it timed out increment the same
|
||||
counter. Worth a doc caveat rather than a code change.
|
||||
counter. Worth a doc caveat rather than a code change. — closed by `e22ecea0`, 2026-08-17
|
||||
(doc caveat, as proposed).
|
||||
- **`backoffCap` wants a constructor doc note** so a future caller does not have to read the
|
||||
loop to learn its units and its interaction with the poll interval.
|
||||
loop to learn its units and its interaction with the poll interval. — closed by `e22ecea0`,
|
||||
2026-08-17.
|
||||
|
||||
**Runtime / deploy path**
|
||||
|
||||
@@ -1377,11 +1392,14 @@ Open items the work surfaced but deliberately did not close. None blocks the bra
|
||||
review, Important). The read now runs off the mailbox so it cannot wedge the actor, but a
|
||||
hung SQL read still pins a thread-pool thread indefinitely unless the `ConfigDb`
|
||||
connection string's command timeout bounds it. **Confirm that it does** — the code relies
|
||||
on it and does not assert it.
|
||||
on it and does not assert it. — closed by `dba5f806`, 2026-08-17 (explicit 30 s
|
||||
`SetCommandTimeout` behind an `IsRelational()` guard; EF 10.0.7 verified empirically to
|
||||
reset it on return to the pool, so there is no pool pollution).
|
||||
- **The historian drain's `catch` swallows a shutdown `OperationCanceledException` as a
|
||||
per-tag failure** when other tags in the same pass succeeded, spuriously incrementing
|
||||
`_tagWriteFailures` (Task 3 review; pre-existing, inherited). Rethrowing a genuine
|
||||
`ct` cancellation is the fix.
|
||||
`ct` cancellation is the fix. — closed by `7870c91f`, 2026-08-17 (the red run proved the
|
||||
bug also discarded healthy tags' samples at shutdown, not just the counter).
|
||||
- **Host-side mux interest gating was skipped** — the batch envelope (Task 2) made the
|
||||
saving it would have bought immaterial. Recorded so it is not re-proposed as an oversight.
|
||||
|
||||
@@ -1389,30 +1407,42 @@ Open items the work surfaced but deliberately did not close. None blocks the bra
|
||||
|
||||
- **Task 6: `_globalCount` can drift under concurrent shard eviction**, and there is no
|
||||
concurrency stress test. Drift is bounded and only affects when trimming triggers, not
|
||||
correctness of any decision.
|
||||
correctness of any decision. — closed by `d8849345`, 2026-08-17 (the planned counter swap
|
||||
alone was insufficient: a seal sentinel plus a `TryBookInsert` compare-exchange were both
|
||||
needed, each proven necessary by a 6/6-red falsification run).
|
||||
- **Task 9: the empty-identifier pattern should be tightened** — the dedup key tolerates an
|
||||
empty node identifier rather than rejecting it.
|
||||
empty node identifier rather than rejecting it. — closed by `c4d8e4a2`, 2026-08-17
|
||||
(pattern-form guard; the bypass is pinned by a clock-call count).
|
||||
- **Task 17: the `GroupBy(_ => 1)` aggregate is unverified against real SQL Server.** It is
|
||||
covered only by EF InMemory, which does not exercise the provider's translation, so the
|
||||
single-round-trip claim is untested where it matters.
|
||||
single-round-trip claim is untested where it matters. — closed by `c149410b`, 2026-08-17
|
||||
(aggregate confirmed live against SQL Server; the suite skips cleanly offline via an
|
||||
xunit-v2 `SqlServerFactAttribute`).
|
||||
- **Task 21: the `CHARINDEX`/collation widening needs a SQL-Server-backed test**, and the
|
||||
worst-case **250 sequential round-trips** (a rename touching the chunking limit) should
|
||||
carry a one-line comment at the call site.
|
||||
carry a one-line comment at the call site. — closed by `c149410b`, 2026-08-17 (collation
|
||||
widening confirmed live; round-trips comment added).
|
||||
|
||||
**Alarms**
|
||||
|
||||
- **Task 15: `TrackBackgroundTask` has a registration-ordering gap** (pre-existing) — a task
|
||||
can complete before it is registered. Also `_disposed` is non-volatile.
|
||||
can complete before it is registered. Also `_disposed` is non-volatile. — closed by
|
||||
`5ae43023`, 2026-08-17 (a TCS-placeholder register-first factory closes the ordering gap;
|
||||
`_disposed` made volatile).
|
||||
|
||||
**Test-suite reliability** (affects how reliably a regression is *caught*, not shipped behaviour)
|
||||
|
||||
- **Task 1's second-reader durability test is fragile on macOS** — it depends on filesystem
|
||||
flush visibility to a second reader, which the platform does not guarantee on the same
|
||||
timescale as Windows/Linux.
|
||||
timescale as Windows/Linux. — closed by `6b46c485`, 2026-08-17 (a robust bounded wait
|
||||
replaced the fixed sleep; the anticipated macOS skip turned out to be unnecessary —
|
||||
verified empirically, the reader does become visible).
|
||||
- **The #491 falsification control is nondeterministic under load.** It relies on a
|
||||
`ThreadStatic` actor cell, so under a loaded sweep the intended-red mutation can
|
||||
occasionally pass. The test is still worth keeping; do not read a single green run of it
|
||||
as proof the regression is caught.
|
||||
as proof the regression is caught. — closed by `6b46c485`, 2026-08-17 (documented in the
|
||||
test file as an honest nondeterminism note; the nondeterminism itself is inherent and
|
||||
remains).
|
||||
|
||||
**Accepted-not-fixed at the final integration review (2026-08-15)** — carried knowingly, none
|
||||
blocking: the poll **pacing** behaviour change ships with no production consumer of the overrun
|
||||
@@ -1423,3 +1453,10 @@ branch (docker-dev recipe:
|
||||
`ScriptTagCatalogSnapshotCache` expires on **wall-clock TTL**; the S7 span reader **stamps a partial
|
||||
span** when the PDU comes back short; MTConnect's `IsFramingOnly` fast path is **ASCII-only**; and a
|
||||
minor Razor exposure where `StateHasChanged` runs inside a `finally`.
|
||||
|
||||
**Annotation, 2026-08-17 (append-only).** Two of the accepted-not-fixed items above have since been
|
||||
retired by `perf/followups`: the overrun counter **now has a production consumer** (the per-driver
|
||||
meter, `e22ecea0` + `97902d68`), and this branch's code state **has been live-gated** — S7 span reads
|
||||
against the `s7_1500` fixture (`57c66416`) and a full docker-dev regression on the merged remainder
|
||||
(2026-08-17, no commit). The `_globalCount` drift item is likewise closed (`d8849345`). The rest of
|
||||
the paragraph stands as written.
|
||||
|
||||
@@ -613,3 +613,168 @@ are glob noise, not failures). (3) Outcomes + register annotations + ledger. (4)
|
||||
update is the orchestrator's job, not this task's.
|
||||
|
||||
**Commit:** `docs(plan): perf follow-ups close-out — outcomes, register annotations, sweep record`
|
||||
|
||||
---
|
||||
|
||||
## Outcomes
|
||||
|
||||
*Written at close-out, 2026-08-17, on branch `perf/followups`. All 15 tasks complete; both
|
||||
release-condition live gates PASSED.*
|
||||
|
||||
### Sweep record
|
||||
|
||||
`dotnet build ZB.MOM.WW.OtOpcUa.slnx` → **0 errors**, 896 warnings (all pre-existing:
|
||||
`xUnit1051` cancellation-token advice and the repo's own `OTOPCUA0001` CapabilityInvoker
|
||||
analyzer firing on test call sites).
|
||||
|
||||
Full unit sweep — every `*.Tests` project in the solution, run per-project with `--no-build`
|
||||
against the tree at the close-out commit. `*.IntegrationTests` are build-only by convention
|
||||
and were not run in the sweep (the S7 suite ran separately as Task 13's live gate; the two
|
||||
empty `tests/Drivers/…Driver.Historian.Wonderware*.Tests` directories carry no csproj and are
|
||||
glob noise).
|
||||
|
||||
**43 / 43 test projects green (exit code 0 on every one).**
|
||||
|
||||
| | |
|
||||
|---|---|
|
||||
| Passed | **10 358** |
|
||||
| Failed | **0** |
|
||||
| Skipped | 19 |
|
||||
| **Total** | **10 377** |
|
||||
|
||||
No test failed, so no isolation re-run was needed. All 19 skips are **pre-existing, env-gated**
|
||||
suites, spread across five projects — `Core.Tests` (1), `Driver.Galaxy.Tests` (8),
|
||||
`Driver.Historian.Gateway.Tests` (6, the `LiveIntegration` category that needs the
|
||||
`wonder-sql-vd03` VPN), `OpcUaServer.Tests` (1), `Runtime.Tests` (3). **None of the skips are
|
||||
the new SQL-Server-backed tests**: `AdminUI.Tests` ran **1537 / 0 skipped** and
|
||||
`Configuration.Tests` **190 / 0 skipped**, which is the evidence that Task 7's pins actually
|
||||
executed against a real SQL Server rather than skipping past themselves.
|
||||
|
||||
The DB-backed suites (`Configuration.Tests`, `AdminUI.Tests` including Task 7's new
|
||||
SQL-Server-backed pins) ran **against a reachable** `10.100.0.35,14330` — they were executed,
|
||||
not skipped. None of the known load-flake candidates flaked
|
||||
(`DriverInstanceActorCommsLossWatchdogTests`, `RawTreeServiceRenameWarningTests`,
|
||||
`RouteNativeAlarmAck_on_non_primary_is_dropped`, plus this branch's new
|
||||
`SessionAclDecisionCache` concurrency stress test and the `ContinuousHistorizationRecorder`
|
||||
timing tests).
|
||||
|
||||
### Per-task outcomes
|
||||
|
||||
| # | Task | Commit(s) | Outcome + deviation |
|
||||
|---|---|---|---|
|
||||
| 1 | Per-driver poll-overrun meter + `PollGroupEngine` doc notes | `e22ecea0`, `97902d68` | Done. Review deviation: the tag keys were literals; moved to snake_case consts (`driver_type` / `driver_instance`) so they match the rest of the Meter and a dashboard's group-by cannot split across two spellings. |
|
||||
| 2 | `SpanReadMaxSpanBytes` validation guard | `acb37724` | Done, as planned — validated at parse time, with the over-PDU degradation documented. |
|
||||
| 3 | Historian drain rethrows a shutdown `OperationCanceledException` | `7870c91f` | Done. Deviation in *severity*, not shape: the red run showed the bug did not merely inflate `_tagWriteFailures` — a shutdown mid-drain **discarded healthy tags' samples**. A pass abort, not a per-tag failure. |
|
||||
| 4 | Bound the Direct-mode `LoadArtifactBlob` read | `dba5f806` | Done, with one addition the plan did not anticipate: an `IsRelational()` guard, because the extension throws on the InMemory provider the actor tests use. EF 10.0.7 verified **empirically** to reset the timeout on return to the pool, so the local `SetCommandTimeout` cannot pollute the pooled factory. ⚠️ The relational branch is consequently untested in-suite. |
|
||||
| 5 | ACL `_globalCount` drift + concurrency stress test | `d8849345` | Done — **beyond the planned scope, and provably so.** The planned counter swap alone was insufficient; a seal sentinel *and* a `TryBookInsert` compare-exchange were both required. Falsification: removing either went 6/6 red. |
|
||||
| 6 | Revalidation memo rejects empty node identifiers | `c4d8e4a2` | Done as a pattern-form guard mirroring the authorizer's; the bypass is pinned by a clock-call count rather than by observing the memo directly. |
|
||||
| 7 | SQL-Server-backed pins for the drift watermark + rename collation | `c149410b` | Done. Deviation: `AdminUI.Tests` is pinned to **xunit v2** by `Akka.TestKit.Xunit2`, so `Assert.Skip` is unavailable — an overriding `SqlServerFactAttribute` gives a genuine skip instead. Both the `GroupBy` watermark aggregate and the `CHARINDEX` collation widening confirmed **live** against SQL Server. |
|
||||
| 8 | `TrackBackgroundTask` registration ordering + volatile `_disposed` | `5ae43023` | Done via a TCS-placeholder **register-first** factory (registration precedes the work starting, closing the gap at its root rather than narrowing it); `_disposed` made volatile. |
|
||||
| 9 | Durability-test robustness + honest #491 nondeterminism notes | `6b46c485` | Done. Deviation: the planned macOS skip proved **unnecessary** — verified empirically that a bounded wait is sufficient, so the test runs on macOS rather than skipping. The #491 falsification control's nondeterminism is inherent and is now documented, not fixed. |
|
||||
| 10 | MTConnect sample-chunk parsing through `XmlReader` | `cda82c07` | Done. 566/566 parity against the pre-change parser, ~19% allocation win. One **disclosed** behaviour change: on a doubly-broken document the *ordering* of the reported errors differs. |
|
||||
| 11 | Expose `ReadMultipleVarsAsync` through the `IS7Plc` seam | `6ee553cf` | Done, as planned. |
|
||||
| 12 | S7 span leftovers batch through multi-var read PDUs | `20022083` | Done, with two forced deviations: S7.Net throws for the **whole** multi-var request rather than per item, so a bad item is handled by a **per-chunk fallback**; and Timer/Counter areas are **excluded** because S7.Net boxes them as `int` where the single-read path yields `ushort`, and box parity is load-bearing downstream. |
|
||||
| 13 | S7 live gate (span reads + multi-var leftovers) | `57c66416` | **LIVE GATE PASSED** vs `10.100.0.35:1102`. Evidence below. |
|
||||
| 14 | docker-dev live gate (regression of the merged remainder) | *(no commit — evidence below)* | **LIVE GATE PASSED.** Evidence below. |
|
||||
| 15 | Close-out — full sweep + Outcomes | `7c5a843a`, *this commit* | Done. `7c5a843a` carries the review round's comment-only fixes; this commit carries the Outcomes, the live-gate evidence, the register annotations, and the ledger. |
|
||||
|
||||
### Live gate — S7 span reads (Task 13)
|
||||
|
||||
**2026-08-17, against the `s7_1500` fixture at `10.100.0.35:1102`. PASSED.**
|
||||
|
||||
- **Results:** 3/3 span-read tests green; 7 passed / 1 skipped across the full
|
||||
`Device=S7_1500` suite. The single skip is the env-gated SYN-blackhole test, which needs a
|
||||
recipe the fixture does not provide — not a silent skip of the gated behaviour.
|
||||
- **Wire shape (the actual claim under test):** with spans **on**, 10 tags produced
|
||||
**3 requests** — 2 block reads + 1 multi-var read carrying 3 items. With spans **off**, the
|
||||
same 10 tags produced **10 requests**. Multi-variable and cross-area batching both confirmed
|
||||
on the wire, and a falsification bit was flipped to prove the counter was measuring the real
|
||||
traffic.
|
||||
- **Reconnect suite** ran **live** (not simulated) via `S7_RECONNECT_BOUNCE_CMD` and passed.
|
||||
- **Not live-pinned:** the Timer/Counter exclusion — the fixture has no TM/CT area, so the
|
||||
box-parity reasoning behind excluding them rests on unit tests only.
|
||||
- **Ops notes for whoever re-runs this** (both cost time on this run):
|
||||
- The fixture **profile is baked into the image**, so `--build` is mandatory after editing a
|
||||
profile. Confirm the seed actually took by looking for
|
||||
`seeded DB1 … seeds=<n>` in `docker logs`; a stale image seeds the old profile silently.
|
||||
- `/opt/otopcua-s7` on the docker host is **root-owned**, so a plain `rsync` fails with
|
||||
status 23 **while looking successful** in its summary output. Use
|
||||
`--rsync-path="sudo rsync"`.
|
||||
|
||||
### Live gate — docker-dev regression (Task 14)
|
||||
|
||||
**2026-08-17, local OrbStack rig, branch at `6b46c485`, ~14.5 min wall. PASSED.**
|
||||
|
||||
- **Boot:** clean-slate build + boot; 8 containers healthy, `RestartCount=0` across the board.
|
||||
- **Deploy:** `c50678b2` sealed in **306 ms** with **6/6 Applied**.
|
||||
- **Data plane:** browse / read / write end-to-end against the pymodbus fixture; a **180 s**
|
||||
subscription streamed with **0 Bad** values.
|
||||
- **Surgical apply (the headline regression target):** a **real** UNS-reference surgical apply
|
||||
— deploy `3df4b321`, logging
|
||||
`applied plan kind=PureAdd added=2 changed=1 surgicalRawTags=1 rebuild=False`. The open
|
||||
subscription streamed **Good across the apply**, and the newly-added UNS mirror served live
|
||||
immediately. This is the behaviour the 2026-08-15 surgical-deploys work exists to protect,
|
||||
exercised as a genuine surgical path rather than a rebuild in disguise.
|
||||
- **Deploy gate:** correctly rejected a malformed first attempt with **422**
|
||||
(`UnsSegmentInvalid`, `EquipmentIdNotDerived`) — the gate is live, not bypassed.
|
||||
- **AdminUI:** `/fleet`, `/alerts`, `/hosts`, `/uns`, `/raw`, `/deployments` all 200 with live
|
||||
data.
|
||||
- **Logs:** 0 ERR/FTL, excluding the by-design standby 503s.
|
||||
- **Teardown:** clean.
|
||||
- **Anomalies (non-blocking, all disclosed):**
|
||||
- A transient **404 on the first `POST /api/deployments`** ~40 s after boot; the retry
|
||||
succeeded. Scripted deploys should **poll** rather than fire once (see the follow-up
|
||||
register below).
|
||||
- Boot-time telemetry-dial **reconnect warnings**, settling within one cycle.
|
||||
- `ScriptLogEntry` **dead-letters on site nodes**, one per deploy — **pre-existing**, not
|
||||
introduced by this branch.
|
||||
- **NOT exercised (say so plainly, do not read the pass as wider than it is):** historian
|
||||
end-to-end (needs the VPN), the `FetchAndCache` flip, the ACL-**enabled** enforcement path,
|
||||
the alarm-bearing surgical **ratchet** to rebuild, load-scale throughput, and real-browser
|
||||
Blazor circuits.
|
||||
|
||||
### Follow-up register (opened by this round)
|
||||
|
||||
Open items this branch surfaced but deliberately did not close. **None blocks the branch.**
|
||||
|
||||
**ACL**
|
||||
|
||||
- ⚠️ **Pre-existing generation-rollover window in `SessionAclDecisionCache.InvalidateIfStale`.**
|
||||
While the CAS winner is running `ClearAll()`, a racing caller can still serve a
|
||||
stale-generation verdict out of a shard the winner has not purged yet. The window is
|
||||
microseconds and **key revocation/expiry are unaffected** (both are re-checked *above* the
|
||||
cache on every call), so this is a stale *grant walk*, not a stale key. Fix shape if it ever
|
||||
matters: stamp the generation on `Shard` itself, or make the racer wait on the winner.
|
||||
- **A fresh `Shard` could take an initial-capacity hint** for a session known to be crawling,
|
||||
to avoid the dictionary's growth rehashes on exactly the workload the per-key ceiling exists
|
||||
for.
|
||||
|
||||
**S7**
|
||||
|
||||
- **No cancellation-branch test on any read path.** Pre-existing, but this branch added a third
|
||||
path (multi-var), so the gap now spans three.
|
||||
- **No integration case for one bad decode inside an otherwise-good chunk** — the per-chunk
|
||||
fallback is unit-tested only.
|
||||
- **The Timer/Counter exclusion needs a live pin**, which needs a fixture with a TM/CT area.
|
||||
|
||||
**Historian**
|
||||
|
||||
- **Task 3's `Task.Delay(500)` is still a hope-wait** and could become a bounded poll, the same
|
||||
treatment Task 9 gave the durability test.
|
||||
|
||||
**AdminUI tests**
|
||||
|
||||
- **A mid-run SQL connectivity blip surfaces as `SqlException` failures, not skips** — the
|
||||
reachability probe is one-shot, taken before the run.
|
||||
- **Orphaned test databases have no reaper.** A killed test process leaves its uniquely-named
|
||||
database behind on `10.100.0.35,14330`.
|
||||
|
||||
**MTConnect**
|
||||
|
||||
- **`AsStream` wants a buffer-ownership comment** so a future caller does not have to infer the
|
||||
lifetime from the call site.
|
||||
|
||||
**docker-dev**
|
||||
|
||||
- **The first `POST /api/deployments` after boot can 404.** Worth a readiness gate on the
|
||||
deployments endpoint, so scripted deploys do not have to encode a retry.
|
||||
|
||||
@@ -1,21 +1,138 @@
|
||||
{
|
||||
"planPath": "docs/plans/2026-08-17-perf-followups.md",
|
||||
"tasks": [
|
||||
{"id": 1, "subject": "Task 1: Per-driver poll-overrun meter + PollGroupEngine doc notes", "status": "pending"},
|
||||
{"id": 2, "subject": "Task 2: S7 span-option validation guard", "status": "pending"},
|
||||
{"id": 3, "subject": "Task 3: Historian drain — shutdown cancellation is not a per-tag failure", "status": "pending"},
|
||||
{"id": 4, "subject": "Task 4: Direct-apply artifact read — explicit command timeout", "status": "pending"},
|
||||
{"id": 5, "subject": "Task 5: SessionAclDecisionCache eviction race + stress test", "status": "pending"},
|
||||
{"id": 6, "subject": "Task 6: Revalidate memo — reject empty node identifiers", "status": "pending"},
|
||||
{"id": 7, "subject": "Task 7: SQL-Server-backed tests for drift probe + rename scan", "status": "pending"},
|
||||
{"id": 8, "subject": "Task 8: ScriptedAlarmEngine TrackBackgroundTask ordering + volatile _disposed", "status": "pending"},
|
||||
{"id": 9, "subject": "Task 9: Test-reliability hardening (durability second-reader, #491 controls)", "status": "pending", "blockedBy": [3]},
|
||||
{"id": 10, "subject": "Task 10: MTConnect Build streams through XmlReader", "status": "pending"},
|
||||
{"id": 11, "subject": "Task 11: IS7Plc multi-variable read seam", "status": "pending"},
|
||||
{"id": 12, "subject": "Task 12: S7 multi-var reads for span leftovers", "status": "pending", "blockedBy": [1, 11]},
|
||||
{"id": 13, "subject": "Task 13: S7 live gate — span planner + multi-var vs s7_1500 fixture", "status": "pending", "blockedBy": [2, 12]},
|
||||
{"id": 14, "subject": "Task 14: docker-dev live gate (regression of the merged remainder)", "status": "pending", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12]},
|
||||
{"id": 15, "subject": "Task 15: Close-out — full sweep + Outcomes", "status": "pending", "blockedBy": [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14]}
|
||||
{
|
||||
"id": 1,
|
||||
"subject": "Task 1: Per-driver poll-overrun meter + PollGroupEngine doc notes",
|
||||
"status": "completed",
|
||||
"note": "e22ecea0 + fix 97902d68 (snake_case tag consts); spec ✅ + code APPROVED; close-out nit: stale doc comment on PollOverruns at OtOpcUaTelemetry.cs:50 still says driver.type"
|
||||
},
|
||||
{
|
||||
"id": 2,
|
||||
"subject": "Task 2: S7 span-option validation guard",
|
||||
"status": "completed",
|
||||
"note": "acb37724; code review APPROVED; deviations: new test file, 2 doc corrections (<= -1 wording)"
|
||||
},
|
||||
{
|
||||
"id": 3,
|
||||
"subject": "Task 3: Historian drain — shutdown cancellation is not a per-tag failure",
|
||||
"status": "completed",
|
||||
"note": "7870c91f; spec ✅ + code APPROVED (Minor: Task.Delay(500) hope-wait at test :590); red run proved healthy-tag sample discard at shutdown; falsification via when-guard weakening"
|
||||
},
|
||||
{
|
||||
"id": 4,
|
||||
"subject": "Task 4: Direct-apply artifact read — explicit command timeout",
|
||||
"status": "completed",
|
||||
"note": "dba5f806; spec ✅ + code APPROVED; IsRelational guard (InMemory tests); EF 10.0.7 pool-reset verified empirically; minor for close-out: comment could state the relational branch is untested in-suite"
|
||||
},
|
||||
{
|
||||
"id": 5,
|
||||
"subject": "Task 5: SessionAclDecisionCache eviction race + stress test",
|
||||
"status": "completed",
|
||||
"note": "d8849345; spec ✅ (interleavings traced) + code APPROVED; falsification 6/6 red reverted. FOLLOW-UP for register (pre-existing, Important, non-blocking): InvalidateIfStale generation-rollover window — a racer can serve a stale-generation verdict from a not-yet-purged shard during the winner's ClearAll (microseconds; revocation/expiry unaffected). Fix shape: generation stamp on Shard or wait-on-winner. Minors: fresh-Shard capacity hint, un-elected global ClearAll comment, CAS backoff, stress-test CI wall-time, 'every booked entry' wording"
|
||||
},
|
||||
{
|
||||
"id": 6,
|
||||
"subject": "Task 6: Revalidate memo — reject empty node identifiers",
|
||||
"status": "completed",
|
||||
"note": "c4d8e4a2; code review APPROVED (pattern-form deviation positive); bypass-count test shape (clock.Calls==2), clock-read-above-guard verified"
|
||||
},
|
||||
{
|
||||
"id": 7,
|
||||
"subject": "Task 7: SQL-Server-backed tests for drift probe + rename scan",
|
||||
"status": "completed",
|
||||
"note": "c149410b; spec ✅ + code APPROVED (5 Minors: mid-run blip not skipped, kill-orphaned DBs, duplicated 200 const in test, ~3s discovery probe offline, TryDropDatabase catches DbException only); xunit-v2 SqlServerFactAttribute skip; 10/10 ran live vs 10.100.0.35"
|
||||
},
|
||||
{
|
||||
"id": 8,
|
||||
"subject": "Task 8: ScriptedAlarmEngine TrackBackgroundTask ordering + volatile _disposed",
|
||||
"status": "completed",
|
||||
"note": "5ae43023; spec ✅ + code APPROVED (2 Minors: null-coalesce comment, remove-before-complete ordering comment); TCS placeholder register-first, volatile _disposed, handover Task.Run preserved"
|
||||
},
|
||||
{
|
||||
"id": 9,
|
||||
"subject": "Task 9: Test-reliability hardening (durability second-reader, #491 controls)",
|
||||
"status": "completed",
|
||||
"blockedBy": [
|
||||
3
|
||||
],
|
||||
"note": "6b46c485; code review APPROVED; robust-wait path (empirically verified fresh-reader re-open observes later commits; xunit v3 skip was available but unneeded); both #491 tests carry load-nondeterminism warnings"
|
||||
},
|
||||
{
|
||||
"id": 10,
|
||||
"subject": "Task 10: MTConnect Build streams through XmlReader",
|
||||
"status": "completed",
|
||||
"note": "cda82c07; spec ✅ + code APPROVED (2 Minors: pre-existing error-phrase asymmetry, AsStream ownership comment); disclosed diff: double-broken doc reports observation error first; ~19% alloc win, honest"
|
||||
},
|
||||
{
|
||||
"id": 11,
|
||||
"subject": "Task 11: IS7Plc multi-variable read seam",
|
||||
"status": "completed",
|
||||
"note": "6ee553cf; code review APPROVED; no shared delegate-hook fake exists — Task 12 gives ImageS7Plc a real multi-var impl"
|
||||
},
|
||||
{
|
||||
"id": 12,
|
||||
"subject": "Task 12: S7 multi-var reads for span leftovers",
|
||||
"status": "completed",
|
||||
"blockedBy": [
|
||||
1,
|
||||
11
|
||||
],
|
||||
"note": "20022083; spec ✅ + code APPROVED; Timer/Counter excluded (int vs ushort box parity); S7.Net throws wholesale → per-chunk fallback; 2 Minor coverage gaps for register: no cancellation-branch test on any read path (pre-existing), no one-bad-decode-in-chunk integration case"
|
||||
},
|
||||
{
|
||||
"id": 13,
|
||||
"subject": "Task 13: S7 live gate — span planner + multi-var vs s7_1500 fixture",
|
||||
"status": "completed",
|
||||
"blockedBy": [
|
||||
2,
|
||||
12
|
||||
],
|
||||
"note": "57c66416; GATE PASSED live vs 10.100.0.35:1102 (3 requests vs 10; multi-var leg + cross-area MW10 confirmed; falsification bit); code review APPROVED; ops findings: fixture profile baked into image (--build mandatory), /opt/otopcua-s7 root-owned (rsync needs sudo rsync-path); Timer/Counter not live-pinned (no TM/CT fixture area)"
|
||||
},
|
||||
{
|
||||
"id": 14,
|
||||
"subject": "Task 14: docker-dev live gate (regression of the merged remainder)",
|
||||
"status": "completed",
|
||||
"blockedBy": [
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
4,
|
||||
5,
|
||||
6,
|
||||
7,
|
||||
8,
|
||||
9,
|
||||
10,
|
||||
11,
|
||||
12
|
||||
],
|
||||
"note": "GATE PASSED (no commit): fresh rig @6b46c485, deploy sealed 306ms 6/6 Applied, 180s subscription 0 Bad across a REAL surgical UNS-ref apply (rebuild=False, surgicalRawTags=1), UNS mirror live, panels 200, 0 ERR/FTL, teardown clean. Anomalies non-blocking: first-POST 404 ~40s after boot (retry), boot telemetry reconnect warnings, ScriptLogEntry dead-letters on sites. Not exercised: historian E2E (VPN), FetchAndCache, ACL-enabled path, alarm ratchet"
|
||||
},
|
||||
{
|
||||
"id": 15,
|
||||
"subject": "Task 15: Close-out — full sweep + Outcomes",
|
||||
"status": "completed",
|
||||
"note": "PLACEHOLDER_SHA (docs) preceded by 7c5a843a (review-round comment fixes). Build 0 errors / 896 pre-existing warnings. Full unit sweep 43/43 projects green: 10358 passed, 0 failed, 19 skipped (all pre-existing env-gated; AdminUI.Tests 1537/0-skipped and Configuration.Tests 190/0-skipped confirm Task 7's SQL-backed pins RAN against 10.100.0.35,14330). No failures, so no isolation re-run. CLAUDE.md correction: the ACL decision cache is sharded per API key (ApiKeyId ?? Principal), not per session.",
|
||||
"blockedBy": [
|
||||
1,
|
||||
2,
|
||||
3,
|
||||
4,
|
||||
5,
|
||||
6,
|
||||
7,
|
||||
8,
|
||||
9,
|
||||
10,
|
||||
11,
|
||||
12,
|
||||
13,
|
||||
14
|
||||
]
|
||||
}
|
||||
],
|
||||
"lastUpdated": "2026-08-17T08:00:00Z"
|
||||
"closeOut": "All 15 tasks complete on branch perf/followups. Both release-condition LIVE GATES PASSED: S7 span reads vs the s7_1500 fixture at 10.100.0.35:1102 (57c66416 — 3 wire requests for 10 tags with spans on vs 10 with spans off; multi-var + cross-area confirmed; Timer/Counter exclusion NOT live-pinned, no TM/CT fixture area), and the docker-dev regression on the merged remainder (no commit — deploy sealed 306ms 6/6 Applied, a REAL surgical UNS-reference apply with rebuild=False streaming Good across an open 180s subscription, 0 ERR/FTL, clean teardown). Close-out sweep: build 0 errors, 43/43 test projects green, 10358 passed / 0 failed / 19 skipped (all pre-existing env-gated). 12 register items in docs/plans/2026-08-15-perf-roadmap-remainder.md §Follow-up register are annotated closed (append-only, with SHAs); HistorianGateway's cross-repo batch WriteLiveValues RPC and the synchronous NodeDeploymentState writes stay open. A NEW follow-up register for this round (ACL generation-rollover window, S7 cancellation/one-bad-decode coverage, historian hope-wait, AdminUI mid-run-blip + orphaned test DBs, MTConnect AsStream ownership, docker-dev first-POST 404) lives in the plan's ## Outcomes section — none blocking. Not exercised by either gate: historian E2E (needs VPN), FetchAndCache flip, ACL-enabled enforcement path, alarm-bearing surgical ratchet, load-scale throughput.",
|
||||
"lastUpdated": "2026-08-17T15:05:00Z"
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user