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 4c921399..fa41c229 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 @@ -17,7 +17,7 @@ { "id": 13, "subject": "C-7 RED tests: unknown top-level key / unknown capability / unknown per-policy field survive round-trip; malformed JSON sets ParseFailed and ToJson returns the original text", "status": "completed", "blockedBy": [11] }, { "id": 14, "subject": "C-7 implement: JsonObject-bag FromJson/ToJson (TagConfigJson idiom), ParseFailed + RawStoredJson, blank->null contract preserved, parser-interop test green", "status": "completed", "blockedBy": [13] }, { "id": 15, "subject": "C-7 razor: ParseFailed warning banner + input lockout + explicit discard button; raw pane shows the stored ResilienceConfig; live-/run on docker-dev :9200 (rebuild BOTH centrals; SQL-mangle + unknown-key survival checks; also drive task 5 warnings and task 9 disabled cells)", "status": "deferred-live", "note": "docker-dev :9200 live-/run; serial live pass", "blockedBy": [14] }, - { "id": 16, "subject": "S-7 builder: add options-generation to PipelineKey + GetOrCreate(optionsGeneration=0) + thread through CapabilityInvoker; StaleGeneration_ReCache_IsNotServed_ToNewGeneration", "status": "pending", "blockedBy": [3] }, + { "id": 16, "subject": "S-7 builder: add options-generation to PipelineKey + GetOrCreate(optionsGeneration=0) + thread through CapabilityInvoker; StaleGeneration_ReCache_IsNotServed_ToNewGeneration", "status": "completed", "blockedBy": [3] }, { "id": 17, "subject": "S-7 factory: Interlocked per-Create generation stamp + Respawn_interleaved_with_old_invoker_call_still_applies_new_options wiring proof; update Invalidate comment (cleanup, not correctness)", "status": "pending", "blockedBy": [16] }, { "id": 18, "subject": "Wrap-up: whole-solution build (0 warnings, analyzer silent) + full dotnet test gate; record the pass + the two report anchor drifts in archreview/plans/STATUS.md", "status": "pending", "blockedBy": [4, 8, 9, 12, 15, 17] } ], 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 79d8db97..a985acb7 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 readonly long _optionsGeneration; private DriverResilienceOptions? _noRetryWriteOptions; /// @@ -41,12 +42,18 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker /// /// Driver type name for structured-log enrichment (e.g. "Modbus"). /// Optional resilience-status tracker. When wired, every capability call records start/complete so Admin /hosts can surface as the in-flight-call-depth proxy. + /// + /// Options epoch for this invoker (01/S-7). Threaded into every pipeline-cache lookup so a lingering + /// old child's late re-cache can never poison a newer invoker's pipeline. Defaults to 0 so existing + /// callers/tests are unaffected. + /// public CapabilityInvoker( DriverResiliencePipelineBuilder builder, string driverInstanceId, Func optionsAccessor, string driverType = "Unknown", - DriverResilienceStatusTracker? statusTracker = null) + DriverResilienceStatusTracker? statusTracker = null, + long optionsGeneration = 0) { ArgumentNullException.ThrowIfNull(builder); ArgumentNullException.ThrowIfNull(optionsAccessor); @@ -56,6 +63,7 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker _driverType = driverType; _optionsAccessor = optionsAccessor; _statusTracker = statusTracker; + _optionsGeneration = optionsGeneration; } /// @@ -123,7 +131,7 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker // 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); + var pipeline = _builder.GetOrCreate(_driverInstanceId, $"{hostName}::non-idempotent", DriverCapability.Write, noRetryOptions, _optionsGeneration); // 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); @@ -156,5 +164,5 @@ public sealed class CapabilityInvoker : IDriverCapabilityInvoker } private ResiliencePipeline ResolvePipeline(DriverCapability capability, string hostName) => - _builder.GetOrCreate(_driverInstanceId, hostName, capability, _optionsAccessor()); + _builder.GetOrCreate(_driverInstanceId, hostName, capability, _optionsAccessor(), _optionsGeneration); } diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs index 51fa558a..2a6b3952 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core/Resilience/DriverResiliencePipelineBuilder.cs @@ -69,23 +69,35 @@ public sealed class DriverResiliencePipelineBuilder /// /// Which capability surface is being called. /// Per-driver-instance options (tier + per-capability overrides). + /// + /// Monotonic options epoch stamped by per invoker + /// (01/S-7). Part of the cache key so a lingering old child's post-invalidate re-cache lands under + /// its OWN (old) generation — a key the new invoker's generation never reads. Defaults to 0 so + /// every existing caller/test compiles unchanged. + /// /// The cached or newly built resilience pipeline for the key. public ResiliencePipeline GetOrCreate( string driverInstanceId, string hostName, DriverCapability capability, - DriverResilienceOptions options) + DriverResilienceOptions options, + long optionsGeneration = 0) { ArgumentNullException.ThrowIfNull(options); ArgumentException.ThrowIfNullOrWhiteSpace(hostName); - var key = new PipelineKey(driverInstanceId, hostName, capability); + var key = new PipelineKey(driverInstanceId, hostName, capability, optionsGeneration); return _pipelines.GetOrAdd(key, static (k, state) => Build( k.DriverInstanceId, k.HostName, state.capability, state.options, state.timeProvider, state.tracker, state.logger), (capability, options, timeProvider: _timeProvider, tracker: _statusTracker, logger: _logger)); } - /// Drop cached pipelines for one driver instance (e.g. on ResilienceConfig change). Test + Admin-reload use. + /// + /// Drop cached pipelines for one driver instance (e.g. on ResilienceConfig change), across ALL + /// option generations. With generation-keyed entries (01/S-7) correctness no longer depends on this + /// sweep — a stale-epoch re-cache is already unreachable by the new generation — but it bounds the + /// cache by clearing lingering old-generation entries on the next respawn. Test + Admin-reload use. + /// /// The driver instance ID whose pipelines should be invalidated. /// The number of pipelines removed. public int Invalidate(string driverInstanceId) @@ -188,5 +200,5 @@ public sealed class DriverResiliencePipelineBuilder return builder.Build(); } - private readonly record struct PipelineKey(string DriverInstanceId, string HostName, DriverCapability Capability); + private readonly record struct PipelineKey(string DriverInstanceId, string HostName, DriverCapability Capability, long Generation); } diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs index ca0b8d14..b9bebed5 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Core.Tests/Resilience/DriverResiliencePipelineBuilderTests.cs @@ -356,6 +356,54 @@ public sealed class DriverResiliencePipelineBuilderTests } } + /// + /// 01/S-7 = 03/S13: a stale-epoch re-cache must not be served to a new generation. Deterministic + /// replay of the respawn race — the old child's late GetOrCreate re-caches under generation 1 + /// AFTER the invalidate; the new invoker (generation 2) with new options must build + serve a pipeline + /// honoring the NEW options, not the poisoned old-generation entry. Generations make the interleaving + /// expressible sequentially — no timing dependence. + /// + [Fact] + public async Task StaleGeneration_ReCache_IsNotServed_ToNewGeneration() + { + var builder = new DriverResiliencePipelineBuilder(); + var optsA = new DriverResilienceOptions + { + Tier = DriverTier.A, + CapabilityPolicies = new Dictionary + { + [DriverCapability.Read] = new(TimeoutSeconds: 5, RetryCount: 3, BreakerFailureThreshold: 0), + }, + }; + var optsB = new DriverResilienceOptions + { + Tier = DriverTier.A, + CapabilityPolicies = new Dictionary + { + [DriverCapability.Read] = new(TimeoutSeconds: 5, RetryCount: 0, BreakerFailureThreshold: 0), + }, + }; + + // gen 1 warms the cache with the retrying optsA. + builder.GetOrCreate("i", "h", DriverCapability.Read, optsA, optionsGeneration: 1); + builder.Invalidate("i"); + // The lingering old child's late call re-caches optsA under its OWN generation 1. + builder.GetOrCreate("i", "h", DriverCapability.Read, optsA, optionsGeneration: 1); + + // The new invoker (gen 2, optsB) must get a pipeline honoring optsB (no retry), not the poisoned one. + var newPipeline = builder.GetOrCreate("i", "h", DriverCapability.Read, optsB, optionsGeneration: 2); + var attempts = 0; + await Should.ThrowAsync(async () => + await newPipeline.ExecuteAsync(async _ => + { + attempts++; + await Task.Yield(); + throw new InvalidOperationException("boom"); + })); + + attempts.ShouldBe(1, "the new generation must honor optsB (retryCount 0), not the stale optsA re-cache"); + } + /// Minimal in-memory capturing (level, formatted-message) pairs so the /// resilience logging contract can be asserted without a real logging provider. private sealed class CapturingLogger : ILogger