From 20ff67bf9a2e98b2adb634da0affbd38ac3a742d Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:05:52 -0400 Subject: [PATCH] =?UTF-8?q?fix(r2-02):=20non-idempotent=20write=20arm=20?= =?UTF-8?q?=E2=80=94=20cache=20the=20no-retry=20snapshot=20(01/P-4)=20and?= =?UTF-8?q?=20record=20tracker=20in-flight=20accounting=20(S-8=20residual)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...lience-config-hardening-plan.md.tasks.json | 2 +- .../Resilience/CapabilityInvoker.cs | 44 ++++++++++++----- .../Resilience/CapabilityInvokerTests.cs | 47 +++++++++++++++++++ 3 files changed, 79 insertions(+), 14 deletions(-) diff --git a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json index 8341c48e..1f103093 100644 --- a/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json +++ b/archreview/plans/R2-02-resilience-config-hardening-plan.md.tasks.json @@ -9,7 +9,7 @@ { "id": 5, "subject": "S-6 AdminUI: ResilienceFormModel.Validate() range messages (via ResiliencePolicyRanges through the transitive Core.Abstractions ref) + warning block in DriverResilienceSection", "status": "completed", "blockedBy": [1] }, { "id": 6, "subject": "S-8 parser layer: force Write/AlarmAcknowledge retryCount overrides to 0 with diagnostic (spec invariant, honest dead-knob surfacing) + tests + GetTierDefaults doc", "status": "completed", "blockedBy": [2] }, { "id": 7, "subject": "S-8 dispatch layer: flip HandleWriteAsync to isIdempotent:false — RED the DriverInstanceActorResilienceWiringTests expectation first, then flip; full Runtime suite green", "status": "completed", "blockedBy": [6] }, - { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "pending", "blockedBy": [7] }, + { "id": 8, "subject": "S-8/P-4 invoker: cache the no-retry options snapshot once per invoker + add RecordCallStart/Complete tracker accounting to the non-idempotent arm + 2 CapabilityInvokerTests", "status": "completed", "blockedBy": [7] }, { "id": 9, "subject": "S-8 docs + form: fix WriteIdempotentAttribute's false 'reads via reflection' claim; note the non-idempotent production default; disable Write/Ack retry cells in the razor", "status": "pending", "blockedBy": [7, 5] }, { "id": 10, "subject": "U-6 delete (Core): remove BulkheadMaxConcurrent/MaxQueue from DriverResilienceOptions + parser shape/merge/doc-example; add BulkheadKeys_InStoredJson_AreIgnored (migration-free contract)", "status": "pending", "blockedBy": [2, 6] }, { "id": 11, "subject": "U-6 delete (AdminUI): strip bulkhead fields from ResilienceFormModel + the two razor inputs; update ResilienceFormModelTests", "status": "pending", "blockedBy": [5, 10] }, diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs index 8b4f5b66..8e77b7de 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/CapabilityInvoker.cs @@ -28,6 +28,7 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker private readonly string _driverType; private readonly Func _optionsAccessor; private readonly DriverResilienceStatusTracker? _statusTracker; + private DriverResilienceOptions? _noRetryWriteOptions; /// /// Construct an invoker for one driver instance. @@ -116,27 +117,44 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker if (!isIdempotent) { - // Snapshot the options exactly once per call — invoking _optionsAccessor twice can - // (a) observe two different snapshots if an Admin edit lands between them and - // (b) wastes an allocation on the per-write hot path (Phase 6.1 1% pipeline budget). - var snapshot = _optionsAccessor(); - var noRetryOptions = snapshot with - { - CapabilityPolicies = new Dictionary - { - [DriverCapability.Write] = snapshot.Resolve(DriverCapability.Write) with { RetryCount = 0 }, - }, - }; + // Build the no-retry options snapshot ONCE per invoker lifetime (01/P-4). The options are + // immutable for the invoker's lifetime — a ResilienceConfig change respawns the driver child, + // which constructs a fresh invoker (#13 respawn-on-change) — so lazily caching the snapshot is + // safe and removes the per-write allocation the previous once-per-call snapshot incurred. The + // null-check-assign tolerates a benign first-call race (both threads build an equivalent record). + var noRetryOptions = _noRetryWriteOptions ??= BuildNoRetryWriteOptions(); var pipeline = _builder.GetOrCreate(_driverInstanceId, $"{hostName}::non-idempotent", DriverCapability.Write, noRetryOptions); - using (LogContextEnricher.Push(_driverInstanceId, _driverType, DriverCapability.Write, LogContextEnricher.NewCorrelationId())) + // Record in-flight accounting on the CALLER's host (the "::non-idempotent" suffix is a + // pipeline-cache key, not an operator-facing host) so Admin /hosts sees write depth too. + _statusTracker?.RecordCallStart(_driverInstanceId, hostName); + try { - return await pipeline.ExecuteAsync(callSite, cancellationToken).ConfigureAwait(false); + using (LogContextEnricher.Push(_driverInstanceId, _driverType, DriverCapability.Write, LogContextEnricher.NewCorrelationId())) + { + return await pipeline.ExecuteAsync(callSite, cancellationToken).ConfigureAwait(false); + } + } + finally + { + _statusTracker?.RecordCallComplete(_driverInstanceId, hostName); } } return await ExecuteAsync(DriverCapability.Write, hostName, callSite, cancellationToken).ConfigureAwait(false); } + private DriverResilienceOptions BuildNoRetryWriteOptions() + { + var snapshot = _optionsAccessor(); + return snapshot with + { + CapabilityPolicies = new Dictionary + { + [DriverCapability.Write] = snapshot.Resolve(DriverCapability.Write) with { RetryCount = 0 }, + }, + }; + } + private ResiliencePipeline ResolvePipeline(DriverCapability capability, string hostName) => _builder.GetOrCreate(_driverInstanceId, hostName, capability, _optionsAccessor()); } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/CapabilityInvokerTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/CapabilityInvokerTests.cs index 277a2307..68a4d00f 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/CapabilityInvokerTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/CapabilityInvokerTests.cs @@ -188,6 +188,53 @@ public sealed class CapabilityInvokerTests "ExecuteWriteAsync's non-idempotent branch must capture the options snapshot exactly once per call"); } + /// + /// S-8 residual: the non-idempotent write arm must record tracker in-flight accounting (start + + /// complete) on the caller's host, exactly like the idempotent arms — the previous arm skipped it. + /// + [Fact] + public async Task NonIdempotentWrite_RecordsTrackerStartAndComplete() + { + var tracker = new DriverResilienceStatusTracker(); + var invoker = new CapabilityInvoker( + new DriverResiliencePipelineBuilder(), + "drv-test", + () => new DriverResilienceOptions { Tier = DriverTier.A }, + statusTracker: tracker); + + await invoker.ExecuteWriteAsync( + "host-1", + isIdempotent: false, + _ => ValueTask.FromResult(0), + CancellationToken.None); + + var snap = tracker.TryGet("drv-test", "host-1"); + snap.ShouldNotBeNull("the non-idempotent write arm must record tracker accounting on the caller's host"); + snap!.CurrentInFlight.ShouldBe(0, "in-flight must return to 0 after the write completes"); + } + + /// + /// 01/P-4: the non-idempotent write arm must build the no-retry options snapshot ONCE per invoker + /// lifetime (not per call). Two writes to the same host reuse the cached snapshot — the options + /// accessor is invoked at most once and only one pipeline is cached. + /// + [Fact] + public async Task NonIdempotentWrite_ReusesCachedNoRetryOptions() + { + var builder = new DriverResiliencePipelineBuilder(); + var accessorCalls = 0; + var invoker = new CapabilityInvoker( + builder, + "drv-test", + () => { Interlocked.Increment(ref accessorCalls); return new DriverResilienceOptions { Tier = DriverTier.A }; }); + + await invoker.ExecuteWriteAsync("host-1", isIdempotent: false, _ => ValueTask.FromResult(0), CancellationToken.None); + await invoker.ExecuteWriteAsync("host-1", isIdempotent: false, _ => ValueTask.FromResult(0), CancellationToken.None); + + accessorCalls.ShouldBeLessThanOrEqualTo(1, "the no-retry snapshot must be cached across calls"); + builder.CachedPipelineCount.ShouldBe(1, "two writes to the same host share one cached no-retry pipeline"); + } + /// /// Core-009 regression — companion consistency assertion: the non-idempotent branch must /// not observe two different option snapshots even if the accessor's returned value changes