dohertj2/lmxopcua
1
0
Fork 0
Code Issues 1 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

docs(archreview): record #13 IMPLEMENTED (ResilienceConfig read-path plumbing)

Browse Source 
- New FOLLOWUP-13: the DriverInstance.ResilienceConfig column was authored/persisted
  and already in the artifact; only the driver-node read path dropped it. Read-path
  plumbing + invalidate-on-change + respawn-on-change; deterministic verification.
- FOLLOWUP-10 sub-gap (a) 'tier-defaults only' → RESOLVED by #13 (sub-gap (b) tracker
  reader still open).
- STATUS.md: task #13 → completed; crit13 branch topology row; Completed-table row;
  findings #10 bullet updated; FOLLOWUP-13 linked.
This commit is contained in:
Joseph Doherty
2026-07-08 22:51:15 -04:00
parent e233db7456
commit 051a59845d
3 changed files with 82 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files
archreview/plans/FOLLOWUP-10-resilience-dispatch-gap.md
+8 -3
View File
@@ -29,9 +29,14 @@ the concrete invoker in would reverse that architectural decision.
1. Only **6** sites are real (all in `DriverInstanceActor`). The 7th — `GenericDriverNodeManager` — is **test-only
scaffolding** (zero production references; the Server has its own address-space path). Its call is NOT wired;
the pragma is re-annotated accordingly, not left as a "tracked gap".
2. **Tier-DEFAULT policy only.** `DriverInstanceSpec` (the deploy artifact) does not carry the per-instance
`ResilienceConfig` JSON, so overrides aren't applied yet. **Sub-gap → new follow-up:** plumb `ResilienceConfig`
through the composer/artifact so per-instance overrides reach the invoker.
2. ~~**Tier-DEFAULT policy only.** `DriverInstanceSpec` (the deploy artifact) does not carry the per-instance
`ResilienceConfig` JSON, so overrides aren't applied yet.~~ **✅ RESOLVED by task #13**
(`fix/archreview-crit13-resilience-config-artifact` `75403caa`): the artifact already emitted the column (the
composer serializes the whole `DriverInstance` entity; AdminUI already authored+persisted it) — only the read
path dropped it. `DriverInstanceSpec` now carries `ResilienceConfig`, `Create` takes + layers it (invalidating
the instance's cached pipelines so a respawn takes effect), and `DriverSpawnPlanner` respawns on a
ResilienceConfig change. Per-instance overrides now reach the invoker. See
[`FOLLOWUP-13`](FOLLOWUP-13-resilience-config-artifact.md).
**Observability finding + fix.** The resilience-status tracker has **no reader** (Admin `/hosts`, Phase 6.1 Stream
E.2/E.3, was never built) and the pipeline builder logged nothing — so the pipeline ran **invisibly**. Added
archreview/plans/FOLLOWUP-13-resilience-config-artifact.md
+63
View File
@@ -0,0 +1,63 @@
# Follow-up #13 — Plumb per-instance ResilienceConfig through the deploy artifact
> **Status:** ✅ IMPLEMENTED (`fix/archreview-crit13-resilience-config-artifact` `75403caa`, off the crit10 branch)
> · **Surfaced by:** task #10's residual sub-finding (the invoker got tier defaults only) · **Task:** TaskCreate
> #13 · **Severity:** Medium (a silently-ignored authored config — the review's "unit-green ≠ wired" class) ·
> **Effort:** M. See [`STATUS.md`](STATUS.md) + [`FOLLOWUP-10`](FOLLOWUP-10-resilience-dispatch-gap.md).
## The finding
Task #10 wired the resilience pipeline into dispatch but the invoker only ever got the driver type's **tier
defaults** — `DriverCapabilityInvokerFactory.Create` passed `resilienceConfigJson: null`. The per-instance
`DriverInstance.ResilienceConfig` JSON column was therefore **silently ignored at runtime**.
**Key discovery:** the *write* side was already complete — the AdminUI `DriverResilienceSection` authors it, every
driver page persists it to the entity (create + update), and `ConfigComposer.SnapshotAndFlattenAsync` serializes
the **whole** `DriverInstance` entity, so `ResilienceConfig` was **already in the artifact JSON**. Only the driver
node's *read* path dropped it: `DriverInstanceSpec` didn't carry the column and the factory hard-coded null. So
#13 is pure read-path plumbing — a genuine dead-config-to-live fix, not a new feature.
## What shipped
- **`DriverInstanceSpec`** gains `string? ResilienceConfig`; **`DeploymentArtifact.TryReadSpec`** reads the column
(lenient `ReadString`, like the other fields).
- **`IDriverCapabilityInvokerFactory.Create`** takes `string? resilienceConfigJson`; the concrete factory parses
it via `DriverResilienceOptionsParser.ParseOrDefaults` (layering onto the tier), logs any parse diagnostic
(malformed JSON / unknown capability / misapplied Tier-C recycle) as a warning, and **never throws** — a bad
config degrades to tier defaults. `NullDriverCapabilityInvokerFactory` ignores the arg. Host DI passes a logger.
- **`DriverHostActor.SpawnChild`** threads `spec.ResilienceConfig` into `Create`.
- **Invalidate-on-change.** The pipeline cache keys on `(instance, host, capability)` and **ignores options on a
cache hit**, so a fresh invoker with changed options would otherwise keep serving the stale pipeline. `Create`
now calls `DriverResiliencePipelineBuilder.Invalidate(driverInstanceId)` first (no-op on first spawn; drops the
stale pipelines on a respawn). Invalidation is per-instance — a sibling's warm pipeline survives.
- **`DriverSpawnPlanner`** treats a `ResilienceConfig` change (incl. `null → json`) as a **stop + respawn** (the
invoker + its resolved options are bound to the child at spawn — the only way a change takes effect is to rebuild
the child). A **pure `DriverConfig` change stays an in-place delta** (no reconnect) — the resilience pipeline is
untouched. Snapshot (`DriverChildSnapshot`) carries `ResilienceConfig` for the diff.
## Design note — respawn vs. in-place swap
A ResilienceConfig-only change **reconnects the driver** (respawn). This is the simplest correct wiring: the
invoker/options are immutable for a child's lifetime (a clean invariant) and it reuses the existing stop+spawn
machinery. The alternative — an in-place invoker swap via a new actor message + mutable field — avoids the
reconnect but is materially more code for a rare operator-tuning op. Documented as a possible future optimization;
respawn is the v1.
## Verification (deterministic)
- **Override reaches execution** (`DriverCapabilityInvokerFactoryTests`): a `Read → retryCount:0` override
suppresses the tier-A Read retry (1 attempt); a **control** with no config retries (>1) — proving the single
attempt is the override, not a fluke.
- **Invalidate-on-create**: re-creating an instance's invoker drops its cached pipelines (count → 0); a sibling
instance's pipeline survives (scoped).
- **Malformed config** logs a warning + still yields a working (tier-default) invoker.
- **Planner** (`DriverSpawnPlannerTests`): ResilienceConfig change → respawn (incl. `null → json`); pure
DriverConfig change with unchanged resilience → delta.
- **Artifact** (`DeploymentArtifactTests`): the column is carried onto the spec / omitted → null.
- Core.Tests 243 (+5), Runtime.Tests 363 (+5), Host builds clean (0 warnings — dispatch sites untouched).
## Residual
None functional. A live rig check would fold into task #12 (the #10 live gate) — deploy a driver with a non-default
ResilienceConfig and confirm the observed retry/breaker behavior matches the override — but the deterministic tests
already prove the override reaches execution.
archreview/plans/STATUS.md
+11 -7
View File
@@ -17,6 +17,7 @@ truth for remediation progress. Update it as work lands.
| `fix/archreview-c1-wire-analyzer` | `f0082af5` | Guard 07/C-1 — wire OTOPCUA0001 analyzer tree-wide + triage (surfaced the RESILIENCE-DISPATCH-GAP, task #10) |
| `fix/archreview-s1-s2-ci-coverage` | `10b89830` | Guard 07/S-1,S-2,S-4 — whole-solution CI leg + fail-on-skip gate + CLI sleep deflake |
| `fix/archreview-crit10-wire-capability-invoker` | `62556c24` | **#10 — wire CapabilityInvoker into dispatch via a Core.Abstractions seam** (off `fix/archreview-c1-wire-analyzer`, NOT master — the analyzer is the standing guard + the pragmas live there). Code-complete + unit/analyzer/context-verified; live gate pending. |
| `fix/archreview-crit13-resilience-config-artifact` | `75403caa` | **#13 — apply per-instance ResilienceConfig from the artifact** (off the crit10 branch — builds on its invoker factory). Read-path plumbing + invalidate/respawn-on-change; deterministically verified. |
**Decision (user):** one branch per Critical, each off `master`. **#9 (failover test) lives on the crit1 branch**
(not its own branch off master) because SBR only exists there — it is Critical 1's integration half. The plans are only on the docs branch,
@@ -39,6 +40,7 @@ so from a critical branch read them via `git show docs/archreview-plans-and-drif
| **#10 — wire CapabilityInvoker into dispatch (seam)** | RESILIENCE-DISPATCH-GAP (07/C-1) | `fix/archreview-crit10-wire-capability-invoker` `62556c24` | **Discovered the filed plan was infeasible** — Runtime is deliberately Polly-free (refs `Core.Abstractions`, not `Core`). Fix = a seam mirroring `IDriverFactory`: `IDriverCapabilityInvoker`(+null) + `IDriverCapabilityInvokerFactory`(+null) in Core.Abstractions; `CapabilityInvoker` implements it; `DriverCapabilityInvokerFactory` (Core) builds per-instance over the singleton builder+tracker+`GetTier`; Host `DriverFactoryBootstrap` registers all three; Runtime SCE resolves like `IDriverFactory`; `DriverInstanceActor` wraps its **6** dispatch sites; analyzer taught the interface as a wrapper home; all 6 pragmas removed. Triage corrections: the 7th site (`GenericDriverNodeManager`) is test-only scaffolding (not wired, re-annotated); **tier-defaults only** (ResilienceConfig not in the artifact → sub-gap). Added retry/breaker **logging** to the pipeline builder (the tracker has no reader — Stream E.2/E.3 never shipped — so it ran invisibly). **Verified:** negative-control (unwrap→OTOPCUA0001 build error); recording-invoker routing test; **real-`CapabilityInvoker` actor-context test** (internal `ConfigureAwait(false)` doesn't leak); analyzer + logging tests; clean solution build; Runtime.Tests 358 / Core.Tests 238 / Analyzers 32. **Live behavioral gate pending** (recipe in FOLLOWUP-10). |
| **#9 — hard-kill failover integration test** | 03/S1 | `fix/archreview-crit1-split-brain-resolver` `a25c9ed0` | Critical 1's load-bearing integration half. `TwoNodeClusterHarness.HardKillNodeAAsync` (transport-shutdown crash sim → node B sees A Unreachable, no graceful Leave) + `HardKillFailoverTests` asserting B downs the alone oldest node and takes over as `driver` role-leader. **LIVE-verified in-process** (~35s); **negative control** (force `downing-provider-class=""`) makes it correctly time out. ⚠️ The negative control also surfaced a **Crit-1 premise finding (task #11): SBR was already active on master** (Akka.Cluster.Hosting applies `SplitBrainResolverOption.Default` when the typed option is null → reads the pre-existing akka.conf keep-oldest block), so the cluster was NOT NoDowning before Crit-1 — the typed option is reinforcing, not the activator. |
| **#11 — Critical 1 premise correction** | 03/S1 (from #9) | `fix/archreview-crit1-split-brain-resolver` `eaf78aad` | Docs/comment-only fix (no code revert): corrected the "HOCON inert / cluster runs NoDowning / never fails over" framing in `BuildClusterOptions` XML comment, `akka.conf` split-brain-resolver comment, `docs/Redundancy.md`, and `SplitBrainResolverActivationTests` (summary + `ShouldNotBeNull` message + method rename). Akka.Cluster.Hosting enables an SBR provider **by default** (applies `SplitBrainResolverOption.Default` when the typed option is null → reads the pre-existing akka.conf `keep-oldest` block), so the cluster was never NoDowning and the typed `KeepOldestOption` is reinforcing/explicit-in-code, not the activator. Cluster.Tests 29/29; failover behaviour still guarded by `HardKillFailoverTests`. Architecture.md carried no such framing. |
| **#13 — apply per-instance ResilienceConfig from the artifact** | RESILIENCE-DISPATCH-GAP sub-gap (from #10) | `fix/archreview-crit13-resilience-config-artifact` `75403caa` | The `DriverInstance.ResilienceConfig` column was authored+persisted (AdminUI) and already in the artifact (the composer serializes the whole entity), but the driver-node read path dropped it → tier-defaults only. `DriverInstanceSpec` now carries it; `TryReadSpec` reads it; `Create` takes+layers it (+ logs parse diagnostics, never throws); `SpawnChild` threads it. **Invalidate-on-change:** `Create` calls `builder.Invalidate(instance)` (the cache is options-blind on a hit) so a respawn with changed options rebuilds; `DriverSpawnPlanner` respawns on a ResilienceConfig change (incl. null→json), a pure DriverConfig change stays an in-place delta. **Verified:** override reaches execution (retryCount:0 suppresses tier-A retry; a control retries), invalidate is per-instance, malformed logs+falls-back; planner+artifact tests. Core.Tests 243 (+5), Runtime.Tests 363 (+5). See [`FOLLOWUP-13`](FOLLOWUP-13-resilience-config-artifact.md). |
## Task list (TaskCreate IDs)
@@ -55,24 +57,26 @@ so from a critical branch read them via `git show docs/archreview-plans-and-drif
| 9 | Critical 1 follow-up — hard-kill failover integration test (03/S1) | ✅ completed — `a25c9ed0`; surfaced task #11 |
| 10 | **Wire CapabilityInvoker into the dispatch layer** (RESILIENCE-DISPATCH-GAP, from 07/C-1) | ✅ code-complete + unit/analyzer/context-verified — `fix/archreview-crit10-wire-capability-invoker` `62556c24` (seam, NOT direct thread — Runtime is Polly-free). **Live behavioral gate pending** (rig recipe in FOLLOWUP-10). Surfaced a sub-gap: ResilienceConfig not in the deploy artifact → tier-defaults only. |
| 11 | **Review Critical 1 premise** — SBR was already active on master, not NoDowning (from #9) | ✅ completed — `fix/archreview-crit1-split-brain-resolver` `eaf78aad`; docs/comment-only correction (no code revert), Cluster.Tests 29/29. |
| 12 | **#10 live behavioral gate** — rebuild docker-dev, force a connection-level exception on an invoker-wrapped call, observe retry/breaker logs + recovery (from #10) | ⬜ pending — rig session (recipe in FOLLOWUP-10); the residual live gate for #10. |
| 13 | **Plumb per-instance `ResilienceConfig` through the deploy artifact** (`DriverInstanceSpec` + `ConfigComposer`) so overrides reach `DriverCapabilityInvokerFactory.Create` (from #10) | ⬜ pending — currently tier-defaults only; add pipeline-invalidate on change. |
| 12 | **#10 live behavioral gate** — rebuild docker-dev, force a connection-level exception on an invoker-wrapped call, observe retry/breaker logs + recovery (from #10) | ⬜ pending — rig session (recipe in FOLLOWUP-10); the residual live gate for #10 (also covers #13's override-behavior end-to-end). |
| 13 | **Plumb per-instance `ResilienceConfig` through the deploy artifact** (from #10) | ✅ completed — `fix/archreview-crit13-resilience-config-artifact` `75403caa`; read-path plumbing (write side was already complete), invalidate-on-change + respawn-on-change; Core.Tests 243/Runtime.Tests 363. See [`FOLLOWUP-13`](FOLLOWUP-13-resilience-config-artifact.md). |
## Findings surfaced by the guards (NEW — need user decision)
> Each has a dedicated follow-up file with the full design/action:
> [`FOLLOWUP-10-resilience-dispatch-gap.md`](FOLLOWUP-10-resilience-dispatch-gap.md) ·
> [`FOLLOWUP-11-crit1-sbr-premise.md`](FOLLOWUP-11-crit1-sbr-premise.md).
> [`FOLLOWUP-11-crit1-sbr-premise.md`](FOLLOWUP-11-crit1-sbr-premise.md) ·
> [`FOLLOWUP-13-resilience-config-artifact.md`](FOLLOWUP-13-resilience-config-artifact.md).
- **#10 — RESILIENCE-DISPATCH-GAP (from wiring the analyzer, 07/C-1): ✅ IMPLEMENTED** on
`fix/archreview-crit10-wire-capability-invoker` `62556c24` — code-complete + unit/analyzer/context-verified;
**live behavioral gate pending** (recipe in [`FOLLOWUP-10`](FOLLOWUP-10-resilience-dispatch-gap.md)). The filed
"thread CapabilityInvoker into the actor" was infeasible (Runtime is deliberately Polly-free); shipped a seam
mirroring `IDriverFactory` instead. Only **6** of the 7 sites were real (the 7th, `GenericDriverNodeManager`, is
test-only scaffolding). **Two residual sub-findings:** (a) per-instance `ResilienceConfig` isn't in the deploy
artifact → **tier-defaults only** until the composer/artifact is extended; (b) the resilience-status tracker has
**no reader** (Admin `/hosts` Stream E.2/E.3 never shipped) — added retry/breaker **logging** as the interim
observability surface + the live-verify enabler.
test-only scaffolding). **Two residual sub-findings:** (a) ~~per-instance `ResilienceConfig` isn't in the deploy
artifact → tier-defaults only~~ **✅ RESOLVED by task #13** (`75403caa`) — the column was already in the artifact;
the read path now applies it (see [`FOLLOWUP-13`](FOLLOWUP-13-resilience-config-artifact.md)); (b) the
resilience-status tracker has **no reader** (Admin `/hosts` Stream E.2/E.3 never shipped) — added retry/breaker
**logging** as the interim observability surface + the live-verify enabler (still open as an Admin-UI follow-on).
- **#11 — Critical 1 premise inaccurate (from the #9 failover negative control): ✅ CORRECTED** on
`fix/archreview-crit1-split-brain-resolver` `eaf78aad`. SBR was **already active on master**
(Akka.Cluster.Hosting's `WithClustering` applies `SplitBrainResolverOption.Default` when the typed option is