From 86742ad8cbff439ac7b3966d463bfcd49af0b39b Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 12:20:10 -0400 Subject: [PATCH] fix(abcip): throttle evict-recreate Forward-Open attempts per device (05/STAB-8) --- ...2-09-driver-fleet-batch-plan.md.tasks.json | 2 +- .../AbCipDriver.cs | 28 +++++ .../AbCipConnectBackoffTests.cs | 109 ++++++++++++++++++ 3 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipConnectBackoffTests.cs diff --git a/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json b/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json index f04d866c..0e58bbb9 100644 --- a/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json +++ b/archreview/plans/R2-09-driver-fleet-batch-plan.md.tasks.json @@ -137,7 +137,7 @@ { "id": "B4.3", "subject": "B4: AbCip evict-recreate (Forward Open) create-path throttle, cache-hit unaffected, tests FAIL->PASS \u2014 STAB-8", - "status": "pending", + "status": "completed", "blockedBy": [ "B3.2" ] diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index f55baf29..e276bf53 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -455,6 +455,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, } await probeRuntime.ReadAsync(ct).ConfigureAwait(false); success = probeRuntime.GetStatus() == 0; + if (success) + state.Backoff.RecordSuccess(); // 05/STAB-8: device reachable — reset the data-path window } catch (OperationCanceledException) when (ct.IsCancellationRequested) { @@ -471,6 +473,7 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { probeRuntime?.Dispose(); } catch { } probeRuntime = null; state.ProbeInitialized = false; + state.Backoff.RecordFailure(DateTime.UtcNow); // 05/STAB-8: keep the data-path window open } TransitionDeviceState(state, success ? HostState.Running : HostState.Stopped); @@ -900,6 +903,13 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, return device.ParentRuntimes[parentTagName]; } + /// + /// Thrown when a per-device window is still open (05/STAB-8) + /// — a fail-fast that costs no wire traffic. Read/write map it to BadCommunicationError + /// via their generic catch, exactly as a real Forward-Open failure. + /// + private sealed class ConnectionThrottledException(string message) : Exception(message); + /// /// Idempotently materialise the runtime handle for a tag definition. First call creates /// + initialises the libplctag Tag; subsequent calls reuse the cached handle for the @@ -914,6 +924,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, ?? throw new InvalidOperationException( $"AbCip tag '{def.Name}' has malformed TagPath '{def.TagPath}'."); + // 05/STAB-8: inside an open backoff window a data-path caller fails fast — no runtime + // create, no Forward-Open. The malformed-path config error above still surfaces (a permanent + // fault must never be masked by the window); only the expensive create+init is throttled. The + // probe loop bypasses this (its own runtime) and drives the failure/success recording. + if (!device.Backoff.ShouldAttempt(DateTime.UtcNow)) + throw new ConnectionThrottledException( + $"AbCip runtime create for '{def.Name}' on {device.Options.HostAddress} throttled — backoff window open after a recent Forward-Open failure."); + // Review I-1 — an array tag (the EXPLICIT IsArray flag, incl. a 1-element array) sets // libplctag's elem_count so the read pulls every element in one CIP transaction; the read // path then boxes them into a typed CLR array. Scalar tags pass count 1 + IsArray false. @@ -927,8 +945,10 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, catch { runtime.Dispose(); + device.Backoff.RecordFailure(DateTime.UtcNow); // 05/STAB-8: open the backoff window throw; } + device.Backoff.RecordSuccess(); // 05/STAB-8: Forward-Open succeeded — reset the window // Two concurrent callers can both miss the cache + both initialize a runtime; only the // first TryAdd wins. Dispose the loser so it doesn't leak a native tag handle. if (device.Runtimes.TryAdd(def.Name, runtime)) @@ -1254,6 +1274,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// Gets the lock object used for probe synchronization. public object ProbeLock { get; } = new(); + + /// + /// Per-device connect-attempt throttle (05/STAB-8). After a failed runtime create + /// (Forward-Open), data-path callers fail fast inside the backoff window instead of + /// re-creating + re-initializing a runtime per tag per tick; the probe loop bypasses it + /// (its own runtime) and drives failure/success so a recovered device resets the window. + /// + public ConnectionBackoff Backoff { get; } = new(TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(30)); /// Gets or sets the current host state of this device. public HostState HostState { get; set; } = HostState.Unknown; /// Gets or sets the UTC timestamp when the host state was last changed. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipConnectBackoffTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipConnectBackoffTests.cs new file mode 100644 index 00000000..5c5ae0fd --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipConnectBackoffTests.cs @@ -0,0 +1,109 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// 05/STAB-8 — a dead AbCip device must not re-create + re-initialize (Forward-Open) a libplctag +/// runtime per tag per tick. The per-device gates the +/// runtime-create path; cache hits are unaffected; the probe loop (its own runtime) drives +/// failure/success so a recovered device resets the window. +/// +[Trait("Category", "Unit")] +public sealed class AbCipConnectBackoffTests +{ + private static AbCipDriverOptions Options(bool probeEnabled) => new() + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + Tags = [new AbCipTagDefinition("Speed", "ab://10.0.0.5/1,0", "Motor1.Speed", AbCipDataType.DInt)], + Probe = new AbCipProbeOptions + { + Enabled = probeEnabled, + ProbeTagPath = "ProbeTag", + Interval = TimeSpan.FromMilliseconds(50), + Timeout = TimeSpan.FromMilliseconds(250), + }, + }; + + /// Rapid reads against a dead device attempt a single Forward-Open, then fail fast inside the backoff window. + [Fact] + public async Task DeadDevice_ConnectAttemptsFollowBackoffSchedule() + { + var creates = 0; + var factory = new FakeAbCipTagFactory + { + Customise = p => + { + Interlocked.Increment(ref creates); + return new FakeAbCipTag(p) { ThrowOnInitialize = true, Exception = new InvalidOperationException("down") }; + }, + }; + var drv = new AbCipDriver(Options(probeEnabled: false), "abcip-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + for (var i = 0; i < 6; i++) + await drv.ReadAsync(["Speed"], CancellationToken.None); + + creates.ShouldBe(1); // one Forward-Open attempt; the rest fail fast inside the window + } + + /// A cached healthy runtime is read without ever consulting the backoff or re-creating. + [Fact] + public async Task CacheHit_NeverConsultsBackoff() + { + var creates = 0; + var factory = new FakeAbCipTagFactory + { + Customise = p => + { + Interlocked.Increment(ref creates); + return new FakeAbCipTag(p) { Value = 5 }; + }, + }; + var drv = new AbCipDriver(Options(probeEnabled: false), "abcip-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + for (var i = 0; i < 5; i++) + { + var r = await drv.ReadAsync(["Speed"], CancellationToken.None); + r[0].StatusCode.ShouldBe(AbCipStatusMapper.Good); + } + + creates.ShouldBe(1); // created once + cached; cache hits never re-create + } + + /// The probe bypasses the backoff window and a successful probe resets it so the next read succeeds. + [Fact] + public async Task ProbeBypassesBackoff_AndSuccessResets() + { + var fail = true; + var factory = new FakeAbCipTagFactory + { + Customise = p => new FakeAbCipTag(p) + { + ThrowOnInitialize = fail, + Exception = new InvalidOperationException("down"), + Value = 7, + }, + }; + var drv = new AbCipDriver(Options(probeEnabled: true), "abcip-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var down = await drv.ReadAsync(["Speed"], CancellationToken.None); + down[0].StatusCode.ShouldBe(AbCipStatusMapper.BadCommunicationError); + + fail = false; // device recovers; the probe bypasses the window and resets it + var deadline = DateTime.UtcNow + TimeSpan.FromSeconds(3); + DataValueSnapshot? good = null; + while (DateTime.UtcNow < deadline) + { + var r = await drv.ReadAsync(["Speed"], CancellationToken.None); + if (r[0].StatusCode == AbCipStatusMapper.Good) { good = r[0]; break; } + await Task.Delay(25, CancellationToken.None); + } + + good.ShouldNotBeNull("probe-driven recovery must reset the backoff so reads recover"); + } +}