Review remediation: Lane A — Runtime driver lifecycle (fable; review fable) #731

Open
opened 2026-10-22 06:05:17 -04:00 by dohertj2 · 0 comments
Owner

From the 2026-10-21 Fable system review. Findings R0x:ID refer to docs/reviews/2026-10-21/0x-*.md (location, evidence, failure scenario, fix recipe). Plan: docs/superpowers/plans/2026-10-22-review-remediation.md.

Lane A — Runtime driver lifecycle (fable; review fable)

Owns:

  • src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/**
  • Runtime/Health/**
  • Runtime/Telemetry/**
  • src/Core/ZB.MOM.WW.OtOpcUa.Cluster/TelemetryOptions.cs + the telemetry dialer
  • src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriver.cs
  • src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs
  • the IFleetDiagnosticsClient consumer page (one new Razor file) + AdminUI/Clients/FleetDiagnosticsClient.cs

Tests: tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests, tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests

  • A1 Dispose drivers on final stop (R02:L1, R02:P4, R06 host context, R07:L2).
    • In DriverInstanceActor.PostStop: ShutdownAsync(cts.Token).WaitAsync(5 s), with a logged timeout. Then DisposeAsync/Dispose, but only when the actor is stopping for good. Override PreRestart to skip disposal, because the Props closure reuses the driver.
    • DiscoveryDriverBrowser disposes its throwaway driver in finally.
    • Tests:
      • A fake IAsyncDisposable driver is disposed exactly once on Context.Stop.
      • It is NOT disposed on a supervision restart (positive control: it still serves after restart).
      • A hung ShutdownAsync doesn't pin a thread beyond 5 s.
  • A2 Single-flight reconnect (R02:H2). Add an _initInFlight gate cleared by the piped InitializeSucceeded/Failed, and bound the teardown. Test: with a 15 s init and a 10 s cadence, ShutdownAsync and InitializeAsync never overlap (record interleavings in the fake).
  • A3 Writes off the mailbox (R02:P2). Run WriteAttribute via PipeTo, capturing Sender, with a per-driver in-flight cap (default 4). Subscribe/unsubscribe stay ReceiveAsync. Tests:
    • Two concurrent 3 s writes both complete under the host's 8 s Ask.
    • Data changes are delivered during a pending write.
  • A4 Housekeeping:
    • _lastReprovisionUtc.Remove in StopChild (R02:L2).
    • Stale() drop-receives (R02:H5).
    • Remove the unused ILoggerFactory ctor slot (R02:U5).
    • Move the OnAttributeValuePublished seam to a test helper (R02:U4).
    • DbHealthProbeActor off-thread + 2 s command timeout (R02:P7).
    • Audit all PollGroupEngine.Unsubscribe sync callers and switch them to UnsubscribeAsync (R05:8).
    • TelemetryLocalHub._evicted timestamped sweep with TTL > max in-flight call (R02:L4).
  • A5 Wire TelemetryDial:CallTimeoutSeconds into the dialer's CallOptions deadline (R02:U2). Test: a stalled stream is cancelled at the configured deadline.
  • A6 Real GetDiagnostics (R02:U1). Populate it from child health snapshots (real id, device counts from GetHostStatuses, last-change time), and give IFleetDiagnosticsClient a consumer: a per-node diagnostics panel on /fleet. That Razor file is owned here and live-verified in Wave 4.

Lane gate: Runtime.Tests green. The docker-dev gate (Wave 4) shows a driver config change disposing the old instance: the ObjectDisposed/dispose log appears, and process thread/handle counts stay flat across 20 respawns (dotnet-counters).

From the 2026-10-21 Fable system review. Findings `R0x:ID` refer to `docs/reviews/2026-10-21/0x-*.md` (location, evidence, failure scenario, fix recipe). Plan: `docs/superpowers/plans/2026-10-22-review-remediation.md`. ### Lane A — Runtime driver lifecycle (fable; review fable) **Owns:** - `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/**` - `Runtime/Health/**` - `Runtime/Telemetry/**` - `src/Core/ZB.MOM.WW.OtOpcUa.Cluster/TelemetryOptions.cs` + the telemetry dialer - `src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriver.cs` - `src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/DiscoveryDriverBrowser.cs` - the `IFleetDiagnosticsClient` consumer page (one new Razor file) + `AdminUI/Clients/FleetDiagnosticsClient.cs` **Tests:** `tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests`, `tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests` - **A1 Dispose drivers on final stop** (`R02:L1`, `R02:P4`, `R06` host context, `R07:L2`). - In `DriverInstanceActor.PostStop`: `ShutdownAsync(cts.Token).WaitAsync(5 s)`, with a logged timeout. Then `DisposeAsync`/`Dispose`, but only when the actor is stopping for good. Override `PreRestart` to skip disposal, because the Props closure reuses the driver. - `DiscoveryDriverBrowser` disposes its throwaway driver in `finally`. - Tests: - A fake `IAsyncDisposable` driver is disposed exactly once on `Context.Stop`. - It is NOT disposed on a supervision restart (positive control: it still serves after restart). - A hung `ShutdownAsync` doesn't pin a thread beyond 5 s. - **A2 Single-flight reconnect** (`R02:H2`). Add an `_initInFlight` gate cleared by the piped `InitializeSucceeded`/`Failed`, and bound the teardown. Test: with a 15 s init and a 10 s cadence, `ShutdownAsync` and `InitializeAsync` never overlap (record interleavings in the fake). - **A3 Writes off the mailbox** (`R02:P2`). Run `WriteAttribute` via `PipeTo`, capturing `Sender`, with a per-driver in-flight cap (default 4). Subscribe/unsubscribe stay `ReceiveAsync`. Tests: - Two concurrent 3 s writes both complete under the host's 8 s Ask. - Data changes are delivered during a pending write. - **A4 Housekeeping:** - `_lastReprovisionUtc.Remove` in `StopChild` (`R02:L2`). - `Stale()` drop-receives (`R02:H5`). - Remove the unused `ILoggerFactory` ctor slot (`R02:U5`). - Move the `OnAttributeValuePublished` seam to a test helper (`R02:U4`). - `DbHealthProbeActor` off-thread + 2 s command timeout (`R02:P7`). - Audit all `PollGroupEngine.Unsubscribe` sync callers and switch them to `UnsubscribeAsync` (`R05:8`). - `TelemetryLocalHub._evicted` timestamped sweep with TTL > max in-flight call (`R02:L4`). - **A5 Wire `TelemetryDial:CallTimeoutSeconds`** into the dialer's `CallOptions` deadline (`R02:U2`). Test: a stalled stream is cancelled at the configured deadline. - **A6 Real `GetDiagnostics`** (`R02:U1`). Populate it from child health snapshots (real id, device counts from `GetHostStatuses`, last-change time), and give `IFleetDiagnosticsClient` a consumer: a per-node diagnostics panel on `/fleet`. That Razor file is owned here and live-verified in Wave 4. **Lane gate:** Runtime.Tests green. The docker-dev gate (Wave 4) shows a driver config change disposing the old instance: the `ObjectDisposed`/dispose log appears, and process thread/handle counts stay flat across 20 respawns (`dotnet-counters`).
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#731