From 0589486f4e5f1d7fb3932d2e9394ccce2c315400 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 28 Aug 2026 14:57:30 -0400 Subject: [PATCH] =?UTF-8?q?fix(site-runtime):=20ConnectionQualityChanged(G?= =?UTF-8?q?ood)=20only=20resets=20the=20tag-subscribe=20backoff=20while=20?= =?UTF-8?q?it=20is=20stretched=20=E2=80=94=20a=20flapping=20connection=20c?= =?UTF-8?q?an=20no=20longer=20defer=20the=20retry=20it=20was=20meant=20to?= =?UTF-8?q?=20hasten?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on ab08ecd3: each Good re-armed the watchdog from 'now', so Goods arriving faster than the floor starved the retry. Pinned by FlappingGood_DoesNotDeferTheRetry (12 Goods over 1.2 s; naive re-arm measured 1.43 s, guarded ~0.3 s). Claude-Session: https://claude.ai/code/session_019ZxXT5sxZs5XrtdwYzJDPV --- .../Actors/InstanceActor.cs | 6 ++- .../Actors/InstanceActorTests.cs | 37 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs index 51c6b510..65bbf00a 100644 --- a/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.SiteRuntime/Actors/InstanceActor.cs @@ -1177,9 +1177,13 @@ public class InstanceActor : ReceiveActor // TagSubscribeRetryMaxIntervalMs — recovery must be prompt. Timer != null ⇔ the // subscribe is unconfirmed (it is only nulled by a Success); if a request is in // flight and young the re-armed tick just hits the guard and re-arms at the floor. + // Only acts while the interval is actually stretched: once reset to the floor a + // further Good is a no-op, so a flapping connection cannot keep pushing the armed + // tick out and starve the retry it was meant to hasten. if (qualityChanged.Quality == QualityCode.Good && _tagSubscribeStates.TryGetValue(qualityChanged.ConnectionName, out var subState) - && subState.Timer != null) + && subState.Timer != null + && subState.Interval > TimeSpan.FromMilliseconds(_options.TagSubscribeRetryIntervalMs)) { ResetTagSubscribeBackoff(subState); ArmTagSubscribeWatchdog(qualityChanged.ConnectionName, subState); diff --git a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTests.cs index 8b98d72e..a88f70df 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.SiteRuntime.Tests/Actors/InstanceActorTests.cs @@ -1442,6 +1442,43 @@ public class InstanceActorTests : TestKit, IDisposable // --- Lever 1 (2026-08-28): Good quality resets the backoff --- + [Fact] + public void FlappingGood_DoesNotDeferTheRetry() + { + // A burst of Good events faster than the floor must not keep re-arming the watchdog + // from "now" — after the first reset the interval is at the floor and later Goods are + // no-ops, so the retry still lands within one floor interval of the first Good. + var options = new SiteRuntimeOptions + { + TagSubscribeRetryIntervalMs = 300, + TagSubscribeRetryMaxIntervalMs = 5000, + TagSubscribeInFlightCeilingMs = 60_000, + }; + var (instance, dclProbe) = CreateInstanceWithDataSourcedAttribute(options); + + var req = dclProbe.ExpectMsg(); + for (var i = 0; i < 3; i++) + { + instance.Tell(new SubscribeTagsResponse(req.CorrelationId, req.InstanceUniqueName, + false, "busy", DateTimeOffset.UtcNow), dclProbe.Ref); + req = dclProbe.ExpectMsg(TimeSpan.FromSeconds(5)); + } + instance.Tell(new SubscribeTagsResponse(req.CorrelationId, req.InstanceUniqueName, + false, "busy", DateTimeOffset.UtcNow), dclProbe.Ref); + + var sw = System.Diagnostics.Stopwatch.StartNew(); + // Twelve Goods over ~1.2 s — a naive re-arm-from-now on each would push the tick to ≥1.5 s. + for (var i = 0; i < 12; i++) + { + instance.Tell(new ConnectionQualityChanged("conn-1", QualityCode.Good, DateTimeOffset.UtcNow)); + Thread.Sleep(100); + } + + // The probe already holds the retry (it landed ~300 ms after the first Good) by the time the burst ends. + dclProbe.ExpectMsg(TimeSpan.FromSeconds(2)); + Assert.True(sw.Elapsed < TimeSpan.FromMilliseconds(1400), $"retry took {sw.Elapsed} — flapping Good deferred it"); + } + [Fact] public void ConnectionBackToGood_ResetsBackoff_AndRetriesPromptly() {