chore(GWC-31): relabel perf-pass GWC-3x comments to PERF-nn; tracker housekeeping
Sweep code/doc comments citing unregistered GWC-30/GWC-31 (2026-08-15 perf
pass) to PERF-02/PERF-07/PERF-22, keyed to task numbers in
docs/plans/2026-08-15-perf-review-remediation.md; add a PERF-ID register
table to that plan. Genuine cycle-2 GWC-30 citations (gateway-side frame-
reader 4-byte prefix fix in Server/Workers/WorkerFrameReader.cs and its
test) are left untouched.
Add a line to REVIEW-PROCESS.md reserving the GWC/WRK/IPC/SEC/CLI/TST
prefixes for the archreview domain registers.
Tracker housekeeping per archreview/2026-08-19/00-overall.md:
- GWC-16 -> Won't fix (rationale: PERF(followup) comment documents the
double session lookup as measured-negligible, deliberately not collapsed)
- IPC-08 -> Done (closed by 7c1ea12/a1a38b5; residual tracked as IPC-34)
- IPC-31 -> Done (superseded by GWC-28's write-time sequence stamping)
- TST-06, TST-17 rows refreshed per cycle-3 60-testing-docs-gaps.md
- WRK-31 (and defensively WRK-29/30) registered as reserved/do-not-reuse
interim IDs burned by e913dab, in both the cycle-2 tracker and the
cycle-2 worker remediation doc
Flip Batch C to Done in archreview/2026-08-19/remediation/00-tracking.md.
Worker-side files (Worker/Ipc/WorkerFrameReader.cs, WorkerFrameWriter.cs)
are comment-only edits, unverified here (net48 x86, windev-only build).
This commit is contained in:
Joseph Doherty
+2
-2
@@ -72,7 +72,7 @@ Observable gauges are pull-based; the `Meter` invokes the supplied callback when
|
||||
|------------|--------------|-------------|
|
||||
| `mxgateway.sessions.open` | `_openSessions` | Currently open sessions tracked by `SessionManager`. |
|
||||
| `mxgateway.workers.running` | `_workersRunning` | Worker clients in a running state. |
|
||||
| `mxgateway.events.worker_queue.depth` | `_workerEventQueueDepthSources` (summed on demand) | Undelivered worker events held by `WorkerClient` — staged *and* queued (GWC-24) — summed across every live client at collection time (GWC-30). Each client owns an interlocked counter incremented when the read loop stages an event and decremented when the consumer reads it, and registers it as a gauge source for its lifetime, so a backlog stuck in a staging channel is visible and concurrent sessions add up instead of overwriting one another. |
|
||||
| `mxgateway.events.worker_queue.depth` | `_workerEventQueueDepthSources` (summed on demand) | Undelivered worker events held by `WorkerClient` — staged *and* queued (GWC-24) — summed across every live client at collection time (PERF-02). Each client owns an interlocked counter incremented when the read loop stages an event and decremented when the consumer reads it, and registers it as a gauge source for its lifetime, so a backlog stuck in a staging channel is visible and concurrent sessions add up instead of overwriting one another. |
|
||||
| `mxgateway.events.grpc_stream_queue.depth` | `_eventStreamBacklogSources` (summed on demand) | Live backlog buffered across every active `EventStreamService` subscriber, summed from the subscribers' channel `Count` at collection time. |
|
||||
|
||||
## Snapshot Shape
|
||||
@@ -111,7 +111,7 @@ The scalar fields mirror the counters and gauges. The four dictionaries provide
|
||||
- `EventsBySession` keys by `sessionId`; entries are removed via `RemoveSessionEvents` when a session closes so the map does not grow without bound.
|
||||
- `RetryAttemptsByArea` keys by the resilience `area` tag, e.g. `worker_startup`.
|
||||
|
||||
`EventsReceived` is read with `Interlocked.Read(ref _eventsReceived)` because `EventReceived` increments it via `Interlocked.Increment` outside the lock to keep the event-ingestion path non-blocking. `CommandsStarted`, `CommandsSucceeded`, `CommandsFailed`, and `CommandFailuresByMethod` are read the same way: the command counters run two-to-three times per gRPC call, so they are recorded with `Interlocked` and a `ConcurrentDictionary` rather than under `_syncRoot` (GWC-30). The two queue depths are pulled from their registered sources before the lock is taken, since those delegates reach into subscriber channels and worker clients.
|
||||
`EventsReceived` is read with `Interlocked.Read(ref _eventsReceived)` because `EventReceived` increments it via `Interlocked.Increment` outside the lock to keep the event-ingestion path non-blocking. `CommandsStarted`, `CommandsSucceeded`, `CommandsFailed`, and `CommandFailuresByMethod` are read the same way: the command counters run two-to-three times per gRPC call, so they are recorded with `Interlocked` and a `ConcurrentDictionary` rather than under `_syncRoot` (PERF-02). The two queue depths are pulled from their registered sources before the lock is taken, since those delegates reach into subscriber channels and worker clients.
|
||||
|
||||
## Recording Sites
|
||||
|
||||
|
||||
@@ -241,7 +241,7 @@ Run `dotnet build src/ZB.MOM.WW.MxGateway.NonWindows.slnx` (expect 0 warnings) a
|
||||
- Modify: `src/ZB.MOM.WW.MxGateway.Worker.Tests/Ipc/WorkerPipeSessionTests.cs`
|
||||
- Modify: `src/ZB.MOM.WW.MxGateway.Worker/Ipc/WorkerFrameReader.cs` (comment only)
|
||||
|
||||
**Why:** On net48, `NamedPipeClientStream.ReadAsync` ignores its `CancellationToken` (`WorkerFrameReader.cs:109-111`). Fault-path exits (event-drain fault, oversized event, watchdog, heartbeat write failure) leave `readTask` pending; it is unblocked only when `WorkerPipeClient`'s `using` disposes the pipe, at which point it faults with `ObjectDisposedException`/`IOException` on a Task nobody observes (the finally at `:303-309` awaits only heartbeat and drain). The frame-pooling change (GWC-30) makes this sharper: the abandoned read owns the per-instance `_lengthPrefix` buffer and possibly a rented ArrayPool payload — the reader's single-consumer invariant holds today only because nothing ever reads again after abandonment.
|
||||
**Why:** On net48, `NamedPipeClientStream.ReadAsync` ignores its `CancellationToken` (`WorkerFrameReader.cs:109-111`). Fault-path exits (event-drain fault, oversized event, watchdog, heartbeat write failure) leave `readTask` pending; it is unblocked only when `WorkerPipeClient`'s `using` disposes the pipe, at which point it faults with `ObjectDisposedException`/`IOException` on a Task nobody observes (the finally at `:303-309` awaits only heartbeat and drain). The frame-pooling change (PERF-22) makes this sharper: the abandoned read owns the per-instance `_lengthPrefix` buffer and possibly a rented ArrayPool payload — the reader's single-consumer invariant holds today only because nothing ever reads again after abandonment.
|
||||
|
||||
**Spec — constraints, implementer designs within them:**
|
||||
1. **No unobserved faulted Task.** After the stream is disposed, `readTask`'s fault must be awaited/observed (reuse `ObserveBackgroundTaskStopAsync`'s timeout-and-log shape, `:312-348`) before `WorkerPipeClient.RunAsync` returns.
|
||||
|
||||
@@ -608,6 +608,24 @@ Commit anything found: `docs: remediation plan doc sweep`
|
||||
|
||||
---
|
||||
|
||||
## PERF-ID register
|
||||
|
||||
This plan's tasks were cited in code comments and test names as `GWC-30`/`GWC-31` before
|
||||
those labels were recognized as colliding with the `archreview` finding register — see
|
||||
cycle-3 finding `GWC-31` (`archreview/2026-08-19/10-gateway-core.md`). The comments were
|
||||
swept to the `PERF-nn` prefix below (`nn` = this doc's task number); `GWC-30`/`GWC-31` no
|
||||
longer appear in perf-pass comments. Citations of the *genuine* archreview `GWC-30`
|
||||
("Frame reader still allocates a fresh 4-byte prefix array per frame",
|
||||
`archreview/2026-07-12/10-gateway-core.md`) — the gateway-side
|
||||
`Server/Workers/WorkerFrameReader.cs` fix and its test — were left untouched; they refer
|
||||
to a real register entry, not this plan.
|
||||
|
||||
| PERF-ID | Task | Files |
|
||||
|---|---|---|
|
||||
| PERF-02 | Task 2 — Metrics: pull-gauge for worker queue depth, lock-free command counters | `Server/Workers/WorkerClient.cs` (`:44`, `:355`), `Server/Metrics/GatewayMetrics.cs` (`:46`, `:213`, `:284`, `:463`, `:524`), `Tests/Metrics/GatewayMetricsTests.cs` (`:22`, `:61`), `docs/Metrics.md` |
|
||||
| PERF-07 | Task 7 — WorkerClient: pooled-timer timeout, single sizing pass, `WorkerCancel` on timeout | `Server/Workers/WorkerClient.cs` (`:236`, `:253`), `Tests/Gateway/Workers/WorkerClientTests.cs` (`:163`, `:179`) |
|
||||
| PERF-22 | Task 22 — Worker frame writer/reader: pooled buffers | `Worker/Ipc/WorkerFrameReader.cs` (`:23`), `Worker/Ipc/WorkerFrameWriter.cs` (`:671`), `docs/plans/2026-08-15-deferred-remediation.md` (Task cross-reference) |
|
||||
|
||||
## Explicitly deferred (decided, not forgotten)
|
||||
|
||||
| Finding | Why deferred |
|
||||
|
||||
Reference in New Issue
Block a user