docs(plan): perf follow-ups round 2 — ACL rollover window, S7 coverage + TM/CT live pin, historian bounded watches, test-DB reaper, docker-dev readiness gate
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,396 @@
|
||||
# Perf Follow-ups Round 2 Implementation Plan
|
||||
|
||||
> **For Claude:** REQUIRED SUB-SKILL: Use superpowers-extended-cc:subagent-driven-development to
|
||||
> implement this plan task-by-task. Opus implementers; classification-driven review chains.
|
||||
|
||||
**Goal:** Close the follow-up register opened by the 2026-08-17 perf follow-ups round
|
||||
(`docs/plans/2026-08-17-perf-followups.md` §"Follow-up register") — the ACL
|
||||
generation-rollover window, the S7 coverage gaps (cancellation branches, one-bad-decode,
|
||||
Timer/Counter live pin), the historian test hope-waits, the orphaned-test-DB reaper, the
|
||||
MTConnect ownership comment, and the docker-dev deploy readiness gate.
|
||||
|
||||
**Architecture:** No new subsystems. One production concurrency fix (generation stamp on the
|
||||
ACL cache's `Shard`), one production doc comment (MTConnect), one rig hardening
|
||||
(`docker-dev/deploy.sh` + Traefik access log); everything else is test/fixture work. Branch
|
||||
`perf/followups-r2` off master (`7bb888be`).
|
||||
|
||||
**Tech Stack:** .NET 10, xUnit + Shouldly, Akka.NET TestKit, python-snap7 fixture on
|
||||
`10.100.0.35`, SQL Server at `10.100.0.35,14330`, Traefik v3.1 on the local OrbStack rig.
|
||||
|
||||
**Out of scope (say so once, here):** the cross-repo HistorianGateway batch `WriteLiveValues`
|
||||
RPC (sister repo); the VPN-gated historian E2E; the FetchAndCache / ACL-enabled / alarm-ratchet
|
||||
/ load-scale live exercises (separate live-gate effort, on request).
|
||||
|
||||
---
|
||||
|
||||
## SHARED-CHECKOUT RULES (every implementer, verbatim from round 1)
|
||||
|
||||
All implementers share ONE checkout on branch `perf/followups-r2`. Therefore:
|
||||
|
||||
- Stage and commit with **explicit pathspecs only**: `git add -- <files>` then
|
||||
`git commit -m "<msg>" -- <files>`. NEVER bare `git commit`, `git add .`, `git add -A`.
|
||||
- NEVER run `git checkout`, `git switch`, `git reset`, `git stash`, `git clean`, or touch
|
||||
files outside your task's `Files:` contract.
|
||||
- On `index.lock` contention or transient build races (CS2012, MSB3030): wait 5–15 s and retry.
|
||||
- Falsification mutations (temporarily reverting a fix to prove a test bites) are made with
|
||||
Edit and reverted with Edit — never with git.
|
||||
- TDD red-first where the plan says so; when a test pins existing behaviour instead, label it
|
||||
honestly ("pin, not red-first") in your report.
|
||||
- Build/test only your own projects (`dotnet build <csproj>` / `dotnet test <csproj>
|
||||
[--filter ...]`), not the whole solution.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Close the ACL generation-rollover window; capacity hint on eviction
|
||||
|
||||
**Classification:** high-risk
|
||||
**Estimated implement time:** ~5 min
|
||||
**Parallelizable with:** Task 2, Task 3, Task 4, Task 5, Task 6, Task 7
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Security/SessionAclDecisionCache.cs`
|
||||
- Test: `tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/SessionAclDecisionCacheTests.cs`
|
||||
|
||||
**The defect (pre-existing, recorded by round 1).** `InvalidateIfStale` (lines 342–350)
|
||||
publishes the new `_generation` in the CAS at line 349 *before* the winner's `ClearAll()`
|
||||
(365–370) has purged the shards. A racer entering `TryGet` (167–183) in that window reads
|
||||
`seen == generation`, skips clearing, and serves an entry written under the previous
|
||||
generation from a shard the winner has not removed yet. Key revocation/expiry are unaffected
|
||||
(re-checked above the cache), so this is a stale grant walk only — but it is real, and the fix
|
||||
shape was already chosen in round 1's register: **stamp the generation on `Shard` itself**.
|
||||
|
||||
**Spec:**
|
||||
|
||||
1. `Shard` (lines 378–391) gains a constructor taking `long generation` (stored in a
|
||||
`public readonly long Generation` field) and an optional initial-capacity hint for
|
||||
`Entries` (see step 6). All four construction sites in `Set` (lines 208, 221, 232, 236)
|
||||
pass the current query generation.
|
||||
2. `TryGet` serves a hit **only when `shard.Generation == generation`**. A
|
||||
generation-mismatched shard is a miss (do not evict from the read path — `TryGet` must stay
|
||||
allocation-free and non-mutating; the winner's `ClearAll` or the next `Set` reclaims it).
|
||||
3. `Set`: after acquiring the shard, if `shard.Generation != generation`, evict-swap exactly
|
||||
like the existing per-shard bound path (`TryUpdate(sessionKey, new Shard(generation), shard)`
|
||||
+ `Interlocked.Add(ref _globalCount, -Seal(shard))`, then re-acquire) before inserting.
|
||||
`_globalCount` accounting must stay exact — the seal/booking protocol from `d8849345` is
|
||||
load-bearing; do not weaken `TryBookInsert`/`Seal`.
|
||||
4. **Deterministic pin of the window** (this is the red-first test): add an internal
|
||||
test-only seam `internal void AdvanceGenerationForTest(long generation)` that CAS-moves
|
||||
`_generation` WITHOUT calling `ClearAll` — exactly the racer's view of the window. Test:
|
||||
`Set` under gen 1, `AdvanceGenerationForTest(2)`, then `TryGet(gen 2, …)` for the same key
|
||||
**must miss**. Run it before the fix: it goes red (the stale entry is served). After the
|
||||
fix: green. Falsification after fixing: temporarily remove the `shard.Generation` check via
|
||||
Edit, confirm red, restore.
|
||||
5. Extend the existing stress test (lines 186–264) or add a sibling: interleave generation
|
||||
bumps with readers/writers and assert **no hit ever returns a value stored under an older
|
||||
generation** (encode the generation in the stored bool — e.g. store `gen % 2 == 0` — and
|
||||
assert every hit equals the current generation's encoding). Also keep asserting
|
||||
`TrackedCount`/`LiveShardCount` exactness at quiescence.
|
||||
6. **Capacity hint (second register item, same ctor):** only the eviction-swap site (line 232
|
||||
path) passes a capacity hint — the evicted shard's sealed count, clamped to a sane bound
|
||||
(e.g. `Math.Clamp(captured, 0, MaxEntriesPerShard)` or the existing per-key ceiling const) —
|
||||
because a shard evicted at the ceiling *is* the crawler signal. Plain `GetOrAdd` shards
|
||||
stay default-capacity. One test: after an eviction swap, inserts proceed correctly (a
|
||||
behaviour pin; capacity itself is not observable — say so in the test comment).
|
||||
7. Update the class-level comments where they describe the invalidation story; do not restate
|
||||
history, state the invariant ("a shard can only serve the generation it was built under").
|
||||
|
||||
**Steps:** red test (4) → fix (1–3) → green → falsification → stress (5) → capacity (6) →
|
||||
`dotnet test tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests --filter "FullyQualifiedName~SessionAclDecisionCache"`
|
||||
→ commit with pathspecs: `git commit -m "fix(acl): stamp the generation on Shard so a rollover racer cannot serve a stale-generation verdict; eviction-swap capacity hint" -- <the two files>`.
|
||||
|
||||
---
|
||||
|
||||
### Task 2: S7 read-path coverage — cancellation branches + one bad decode in a chunk
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~5 min
|
||||
**Parallelizable with:** Task 1, Task 3, Task 4, Task 5, Task 6, Task 7
|
||||
|
||||
**Files:**
|
||||
- Test (only): `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs`
|
||||
|
||||
**Context.** Three read paths carry the STAB-15a cancellation contract (`_plcDead = true` +
|
||||
rethrow): per-tag (`S7Driver.cs:649-660`, covered by
|
||||
`S7DriverReconnectTests.Cancelled_read_marks_handle_dead_and_next_read_reopens`), span block
|
||||
read (`S7Driver.cs:920-926`, **uncovered**), multi-var leftovers (`S7Driver.cs:803-809`,
|
||||
**uncovered**). The per-chunk fallback (`:810-817`) has one whole-request-fault test; the
|
||||
per-ITEM decode guard (`:819-836`, null value → `InvalidDataException` → that tag Bad) and the
|
||||
shape-mismatch arm (`:796-801`) have none. `ImageS7PlcFactory`
|
||||
(`S7SpanReadParityTests.cs:453`, `ImageS7Plc` at `:480`) exposes only `ReadFault` and
|
||||
`MultiVarFault` knobs today.
|
||||
|
||||
**Spec — all tests are pins of existing behaviour unless marked red-first:**
|
||||
|
||||
1. Extend `ImageS7PlcFactory`/`ImageS7Plc` with three knobs, mirroring the existing fault
|
||||
knobs' style:
|
||||
- `HangReadBytes` (a `TaskCompletionSource` gate): `ReadBytesAsync` awaits
|
||||
`Task.Delay(Timeout.Infinite, ct)` so cancellation is the only exit — the
|
||||
`S7DriverReconnectTests` fake at `:557-560` is the pattern.
|
||||
- `HangMultiVar`: same for `ReadMultipleVarsAsync`.
|
||||
- `NullItemIndex` (int?): `ReadMultipleVarsAsync` returns the normal answered list but with
|
||||
item N's `Value` left null — the decode guard's trigger at `:824-826`.
|
||||
Track a `CreatedPlcCount` on the factory if not already present (reopen observable).
|
||||
2. **Cancelled span block read:** plan spans on, hang `ReadBytesAsync`, cancel the read's CTS,
|
||||
assert `OperationCanceledException` propagates out of `ReadAsync`, and the NEXT read
|
||||
constructs a fresh plc (`CreatedPlcCount` increments — the `_plcDead` reopen contract),
|
||||
using the same assertion shape as the reconnect test at `S7DriverReconnectTests.cs:338-355`.
|
||||
3. **Cancelled multi-var read:** spans on with a leftover batch (reuse the existing
|
||||
multi-var-eligible tag set from `:257/:281`), hang `ReadMultipleVarsAsync`, cancel, same
|
||||
two assertions.
|
||||
4. **One bad decode inside an otherwise-good chunk:** `NullItemIndex = 1` on a 3-item
|
||||
multi-var chunk → exactly that tag lands Bad (`RecordReadFailure` status), its chunk-mates
|
||||
Good with correct values, `MultiVarCalls == 1`, and **no per-tag fallback reads occur** for
|
||||
the good items (per-item failure is not chunk failure). Assert driver health reflects the
|
||||
partial failure the same way the existing fallback test does.
|
||||
5. **Shape mismatch:** a knob (or reuse `NullItemIndex`-style) making `ReadMultipleVarsAsync`
|
||||
return `chunk.Count - 1` items → the whole chunk falls back per-tag (`:796-801` →
|
||||
`InvalidDataException` → `:810-817`), all values still served Good via fallback.
|
||||
6. Falsification for (2) and (3): via Edit, temporarily change the production OCE arm to swallow
|
||||
(mimic the pre-#527-style bug) and confirm the reopen assertion goes red; restore. Note in
|
||||
the report which line was mutated.
|
||||
|
||||
**Steps:** knobs → tests (each run individually) → falsification → full file run
|
||||
(`dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests --filter "FullyQualifiedName~S7SpanReadParityTests"`)
|
||||
→ commit `test(s7): pin the span/multi-var cancellation reopen contract, one-bad-decode-in-chunk, and multi-var shape mismatch -- tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.Tests/S7SpanReadParityTests.cs`.
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Timer/Counter live pin — TM/CT area in the s7_1500 fixture
|
||||
|
||||
**Classification:** standard (live-gated)
|
||||
**Estimated implement time:** ~5 min authoring + fixture redeploy wall-time
|
||||
**Parallelizable with:** Task 1, Task 2, Task 4, Task 5, Task 6, Task 7 (disjoint files; the
|
||||
live leg runs against the shared fixture host, not the checkout)
|
||||
|
||||
**Files:**
|
||||
- Modify: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/Docker/profiles/s7_1500.json`
|
||||
- Modify: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/S7_1500/S7_1500SpanReadTests.cs`
|
||||
- Modify (if needed for seeding TM/CT): `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/Docker/server.py`
|
||||
- Modify: `tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/Docker/README.md`
|
||||
|
||||
**Context.** The round-1 gate could not live-pin the Timer/Counter multi-var exclusion because
|
||||
the fixture profile declares only DB1 + MK (`s7_1500.json`), though `server.py:34-41`'s
|
||||
`AREA_MAP` already supports `TM`/`CT` (`snap7.type.SrvArea`). The exclusion itself:
|
||||
`S7ReadSpanPlanner.cs:94-103` (planner → leftovers) and `S7Driver.cs:1129-1132`
|
||||
(`IsBufferType` → never multi-var). Timer/Counter entries are 2-byte device-number-indexed
|
||||
cells; a "device number N" seed lands at byte offset `2*N` big-endian in the area buffer.
|
||||
|
||||
**Spec:**
|
||||
|
||||
1. Add `TM` index 0 and `CT` index 0 areas (size 64 bytes each = 32 devices) to
|
||||
`s7_1500.json` with 2–3 distinct non-zero seeds each (the #536 rule: a zero-valued source
|
||||
proves nothing) at known device numbers, `_desc`'d. Keep every existing seed byte-identical
|
||||
— the round-1 tests depend on them.
|
||||
2. New `[SkippableFact]`-style test (follow the file's existing gating convention) in
|
||||
`S7_1500SpanReadTests.cs`: author a tag set mixing multi-var-eligible scalars with one
|
||||
Timer and one Counter tag; spans on. Assert:
|
||||
- the Timer/Counter values read back as the seeded non-zero constants (correct decode on a
|
||||
real wire), and
|
||||
- via `CountingPlc`, **no multi-var request ever contains a Timer/Counter item** and the
|
||||
TM/CT reads arrive as their own block/per-tag requests — the live pin of the exclusion.
|
||||
3. **Accepted alternate outcome, stated in advance:** S7.Net's TM/CT read is a distinct wire
|
||||
shape the python-snap7 server has never served. If it genuinely cannot (live evidence:
|
||||
error/exception captured, not a guess), do NOT force it: env-gate the test with a skip
|
||||
reason naming the fixture limitation, record the evidence in the Outcomes, and the
|
||||
exclusion stays unit-pinned. A red herring here must not burn the task.
|
||||
4. Fixture redeploy recipe (round-1 ops notes, both bit last time):
|
||||
`rsync -av --rsync-path="sudo rsync" tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests/Docker/ dohertj2@10.100.0.35:/opt/otopcua-s7/`
|
||||
then `ssh dohertj2@10.100.0.35 'cd /opt/otopcua-s7 && docker compose build && docker compose up -d --force-recreate'`
|
||||
— `--build` is mandatory (profile is baked into the image) and confirm
|
||||
`seeded DB1 … seeds=<n>` (and the new TM/CT lines) in `docker logs`.
|
||||
5. Run: `dotnet test tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.S7.IntegrationTests --filter "FullyQualifiedName~S7_1500SpanReadTests"` with the fixture live. All pre-existing
|
||||
tests must stay green (the seed-compat rule in step 1).
|
||||
|
||||
**Commit:** `test(s7): live-pin the Timer/Counter multi-var exclusion against a TM/CT-seeded s7_1500 fixture` with pathspecs.
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Historian recorder tests — bounded watches instead of hope-waits
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~4 min
|
||||
**Parallelizable with:** Task 1, Task 2, Task 3, Task 5, Task 6, Task 7
|
||||
|
||||
**Files:**
|
||||
- Test (only): `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Historian/ContinuousHistorizationRecorderTests.cs`
|
||||
|
||||
**Context.** Three fixed sleeps synchronize negative assertions ("X did not happen"):
|
||||
line 602 (`FromMilliseconds(500)` before asserting no truncation after a shutdown-cancelled
|
||||
write), line 452 (300 ms before asserting an abandoned tag stopped being rewritten), line 857
|
||||
(300 ms before asserting sequential-drain concurrency). A negative can't be polled for
|
||||
directly; each needs a **positive completion observable**, then the assert. The Task-9
|
||||
treatment (`FasterLogHistorizationOutboxTests.cs:203-231`) is the pattern for the poll shape
|
||||
(Stopwatch + 25 ms + cap, return last observation). `InMemoryOutbox` has
|
||||
`AppendCallCount`/`AppendBatchCallCount`/`PeekBatchCallCount`/`DroppedCount` but **no
|
||||
`RemoveCallCount`**.
|
||||
|
||||
**Spec:**
|
||||
|
||||
1. Add `RemoveCallCount` to `InMemoryOutbox` (increment in `RemoveAsync`), same style as the
|
||||
existing counters.
|
||||
2. **Line 602 test:** replace the sleep with a bounded watch: after `writer.Cancelled`, poll
|
||||
(25 ms / 2 s cap) for the *bug's* positive signal — `RemoveCallCount > 0` or
|
||||
`CountAsync != 2` — failing fast red if either fires, green after the full window if
|
||||
neither does. Comment: "bounded negative watch — red is immediate when the fold truncates,
|
||||
green costs the window once". Keep the existing preconditions (594–595) untouched.
|
||||
3. **Line 452 test:** replace wall-clock with the drain's own clock: snapshot
|
||||
`PeekBatchCallCount`, poll until it has advanced by ≥ 3 (more passes have demonstrably
|
||||
run), then assert the `Good.Tag` write count is unchanged. Cap 5 s; on cap-out without 3
|
||||
passes, fail with a message naming the observed pass count.
|
||||
4. **Line 857 test:** same pass-count clock (≥ 2 further passes) before the concurrency
|
||||
asserts; keep the existing `AwaitAssertAsync` precondition.
|
||||
5. Do NOT touch the deliberate mechanics delays (1476, 1640, 1656, 1673 — documented
|
||||
pool-thread forcing; leave them and their comments alone).
|
||||
6. Each reworked test: run 5× locally (`--filter`) to check stability; note the timings in
|
||||
the report.
|
||||
|
||||
**Commit:** `test(historian): replace the recorder tests' fixed sleeps with bounded watches on drain-pass and truncation observables` with pathspecs.
|
||||
|
||||
---
|
||||
|
||||
### Task 5: Orphaned test-database reaper (AdminUI + Configuration fixtures)
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~4 min
|
||||
**Parallelizable with:** Task 1, Task 2, Task 3, Task 4, Task 6, Task 7
|
||||
|
||||
**Files:**
|
||||
- Modify: `tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/AdminUiSqlServerFixture.cs`
|
||||
- Modify: `tests/Core/ZB.MOM.WW.OtOpcUa.Configuration.Tests/SchemaComplianceFixture.cs`
|
||||
- Test: a new `[SqlServerFact]` (AdminUI side) proving the reaper drops an old-named DB
|
||||
|
||||
**Context.** A killed test process leaks its uniquely-named database forever:
|
||||
`OtOpcUaAdminUi_Test_<yyyyMMddHHmmss>_<GuidN>` (`AdminUiSqlServerFixture.cs:77`) and
|
||||
`OtOpcUaConfig_Test_…` (`SchemaComplianceFixture.cs:30`). The fixture comment at 127–128 even
|
||||
says the timestamp exists "so a leaked one is identifiable" — the reaper was never written.
|
||||
Natural insertion point: fixture ctor, after the probe early-return (`:75`), before the name
|
||||
assignment (`:77`). The drop SQL to reuse is `:118-121`; `TryDropDatabase` must be extracted
|
||||
to a static taking a name.
|
||||
|
||||
**Spec — safety first, this runs DROP DATABASE against a shared server:**
|
||||
|
||||
1. Extract the drop body into a static `TryDropDatabase(string databaseName)` (best-effort,
|
||||
`DbException`-swallowing, exactly the current semantics); the instance method delegates.
|
||||
2. Add a static reaper, run **once per process** behind a `Lazy` in `AdminUiSqlServer`
|
||||
(mirroring the probe): `SELECT name FROM sys.databases WHERE name LIKE
|
||||
'OtOpcUaAdminUi\_Test\_%' ESCAPE '\'`, parse the embedded `yyyyMMddHHmmss` (UTC), and drop
|
||||
ONLY rows **older than 24 hours**; a name whose timestamp does not parse is left alone.
|
||||
Never throw — a reaper failure must not fail a green run. Log nothing louder than
|
||||
`Console.WriteLine` (test infra convention in this file).
|
||||
3. Same shape in `SchemaComplianceFixture` for the `OtOpcUaConfig_Test_` prefix, one-shot per
|
||||
process. (It has no probe/skip machinery — keep the reaper `catch`-all so an unreachable
|
||||
server behaves exactly as today.)
|
||||
4. **Test (red-first is impossible for a time-gated reaper without waiting a day, so pin it
|
||||
deterministically):** a `[SqlServerFact]` that creates an empty DB named with a >24 h-old
|
||||
timestamp (e.g. `OtOpcUaAdminUi_Test_20250101000000_<guid>`), invokes the reaper's core
|
||||
(make the core internal-testable, taking the cutoff as a parameter), and asserts the DB is
|
||||
gone — and a second DB named with a **fresh** timestamp survives the same call. Clean up
|
||||
the survivor in the test.
|
||||
5. The mid-run-blip semantics (one-shot probe → mid-run `SqlException` = failures, not skips)
|
||||
stay as-is — they are already documented at `:34-38`; verify the comment survives your
|
||||
edit, extend it with one line noting the deliberate non-choice if it doesn't already say so.
|
||||
|
||||
**Commit:** `test(sql): reap >24h-old orphaned test databases at fixture startup (AdminUI + Configuration prefixes)` with pathspecs.
|
||||
|
||||
---
|
||||
|
||||
### Task 6: MTConnect `AsStream` buffer-ownership comment
|
||||
|
||||
**Classification:** trivial
|
||||
**Estimated implement time:** ~1 min
|
||||
**Parallelizable with:** all
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectStreamsParser.cs` (the
|
||||
`AsStream` doc comment at lines 241–249)
|
||||
|
||||
**Spec:** extend the existing `<summary>` with the ownership facts (verified this round):
|
||||
the array-backed branch **aliases the caller's array without copying**; the buffer is not
|
||||
pooled or rented anywhere on this path; parsing is fully synchronous, so the buffer must stay
|
||||
valid and unmutated only **for the duration of the call**; the returned
|
||||
`MTConnectStreamsResult` holds no reference into the buffer (`XmlReader` materialises its own
|
||||
strings). Comment-only — no code change, no test.
|
||||
|
||||
**Commit:** `docs(mtconnect): state AsStream's buffer aliasing and lifetime contract` with pathspec.
|
||||
|
||||
---
|
||||
|
||||
### Task 7: docker-dev readiness-gated deploy helper + Traefik access log
|
||||
|
||||
**Classification:** small
|
||||
**Estimated implement time:** ~4 min
|
||||
**Parallelizable with:** all
|
||||
|
||||
**Files:**
|
||||
- Create: `docker-dev/deploy.sh`
|
||||
- Modify: `docker-dev/docker-compose.yml` (Traefik command block, lines ~912-916)
|
||||
- Modify: `docker-dev/README.md` (the bare POST snippet at lines ~88-91)
|
||||
|
||||
**Context (root-cause facts, record them in the README):** the transient first-POST 404 can NOT
|
||||
come from the AdminUI (all routes map synchronously before `RunAsync`
|
||||
(`Program.cs:740` vs `:803`); the endpoint's own failures are 503/401/422, never 404) and can
|
||||
NOT be Traefik's backend-down answer (that is 503 `no available server`). A 404 from :9200
|
||||
means **no Traefik router matched at that instant** — mechanism unconfirmed; the access log
|
||||
below is how the next occurrence gets diagnosed.
|
||||
|
||||
**Spec:**
|
||||
|
||||
1. `deploy.sh` (bash, `set -euo pipefail`): polls `http://localhost:9200/health/ready` until
|
||||
HTTP 200 (2 s interval, 120 s cap — a 200 through Traefik proves both the router table and
|
||||
a ready backend), then POSTs the artifact exactly as the README snippet does
|
||||
(`X-Api-Key: docker-dev-deploy-key`, JSON body from `$1` or stdin), prints the response,
|
||||
and exits non-zero on any non-2xx. No retry on the POST itself — the gate is the fix, and
|
||||
a post-gate 404 should fail loudly (it would be new evidence).
|
||||
2. Add `--accesslog=true` (and `--accesslog.fields.defaultmode=keep`) to the Traefik command
|
||||
list so a recurrence shows which router (or none) matched.
|
||||
3. README: replace the bare POST snippet with `./deploy.sh`, and add a short "first-POST 404"
|
||||
note carrying the root-cause facts above.
|
||||
4. Verification is docker-dev-rig-dependent; `bash -n deploy.sh` + a `docker compose config`
|
||||
parse check are the in-checkout gate. The rig run happens in Task 8's close-out if the rig
|
||||
is brought up; do not block this task on it.
|
||||
|
||||
**Commit:** `chore(docker-dev): readiness-gated deploy.sh + Traefik access log for the first-POST 404` with pathspecs.
|
||||
|
||||
---
|
||||
|
||||
### Task 8: Close-out — sweep + Outcomes + register annotations
|
||||
|
||||
**Classification:** standard
|
||||
**Estimated implement time:** ~10 min (mostly sweep wall-time)
|
||||
**Parallelizable with:** none (blockedBy all)
|
||||
|
||||
**Files:**
|
||||
- Modify: `docs/plans/2026-08-17-perf-followups-r2.md` (append `## Outcomes`)
|
||||
- Modify: `docs/plans/2026-08-17-perf-followups.md` (append-only "closed by <sha>" annotations
|
||||
on the round-1 register items this round closes; the two live-exercise items that stay open
|
||||
stay open)
|
||||
- Modify: `docs/plans/2026-08-17-perf-followups-r2.md.tasks.json` (ledger)
|
||||
|
||||
**Steps:**
|
||||
|
||||
1. `dotnet build ZB.MOM.WW.OtOpcUa.slnx` → 0 errors (pre-existing warnings tolerated).
|
||||
2. Full per-project unit sweep of every `*.Tests` project (`--no-build` after the solution
|
||||
build), same protocol as round 1. DB-backed suites must RUN (not skip) against
|
||||
`10.100.0.35,14330`. Expected: 0 failed; new-test count above round 1's 10,358 baseline.
|
||||
3. S7 integration suite against the live fixture (Task 3's evidence re-confirmed at the final
|
||||
tree).
|
||||
4. Append `## Outcomes`: per-task table (commit, outcome, deviation), the TM/CT live evidence
|
||||
(or the documented fixture limitation), the reaper's first live reap count, any new
|
||||
follow-up register (only genuinely new items).
|
||||
5. Annotate the round-1 register (append-only, never rewrite): ACL rollover + capacity hint,
|
||||
S7 cancellation + one-bad-decode + TM/CT, historian hope-wait, AdminUI reaper (blip
|
||||
semantics = documented wontfix), MTConnect comment, docker-dev readiness gate — each with
|
||||
its closing SHA.
|
||||
6. Commit docs with pathspecs; update the ledger.
|
||||
|
||||
---
|
||||
|
||||
## Execution notes for the controller
|
||||
|
||||
- Wave 1: Tasks 1–7 dispatched concurrently (all `Files:` contracts disjoint). Task 8 after
|
||||
all verdicts land.
|
||||
- Review chains per classification: T1 high-risk (spec serial → code); T2/T3/T4/T5/T8
|
||||
standard (spec ∥ code); T7 small (code only); T6 trivial (none).
|
||||
- Implementers: Opus (user preference), including the trivial task for uniformity.
|
||||
- Reviewers judge via `git show <sha>`; file-disjoint scheduling makes on-disk fallback
|
||||
acceptable as in round 1.
|
||||
@@ -0,0 +1,14 @@
|
||||
{
|
||||
"planPath": "docs/plans/2026-08-17-perf-followups-r2.md",
|
||||
"tasks": [
|
||||
{"id": 1, "subject": "Task 1: ACL generation stamp on Shard + eviction capacity hint", "status": "pending"},
|
||||
{"id": 2, "subject": "Task 2: S7 cancellation branches + one-bad-decode-in-chunk + shape mismatch", "status": "pending"},
|
||||
{"id": 3, "subject": "Task 3: Timer/Counter live pin — TM/CT area in the s7_1500 fixture", "status": "pending"},
|
||||
{"id": 4, "subject": "Task 4: Historian recorder tests — bounded watches", "status": "pending"},
|
||||
{"id": 5, "subject": "Task 5: Orphaned test-database reaper (AdminUI + Configuration)", "status": "pending"},
|
||||
{"id": 6, "subject": "Task 6: MTConnect AsStream ownership comment", "status": "pending"},
|
||||
{"id": 7, "subject": "Task 7: docker-dev readiness-gated deploy.sh + Traefik access log", "status": "pending"},
|
||||
{"id": 8, "subject": "Task 8: Close-out — sweep + Outcomes + register annotations", "status": "pending", "blockedBy": [1, 2, 3, 4, 5, 6, 7]}
|
||||
],
|
||||
"lastUpdated": "2026-08-17T16:00:00Z"
|
||||
}
|
||||
Reference in New Issue
Block a user