From 0bd856552c34604b5431be75a02d1a49389f6d19 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 08:35:10 -0400 Subject: [PATCH] fix(centralui): short negative TTL after a faulted pending-updates flight (review 2026-10-21 04/C4) --- .../Services/PendingUpdatesCache.cs | 6 +++- .../Services/SingleFlightMemo.cs | 31 +++++++++++++++---- .../Services/PendingUpdatesCacheTests.cs | 12 ++++--- .../Services/SingleFlightMemoTests.cs | 20 ++++++++++++ 4 files changed, 57 insertions(+), 12 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/PendingUpdatesCache.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/PendingUpdatesCache.cs index a7d3a13d..11bb3c2c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/PendingUpdatesCache.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/PendingUpdatesCache.cs @@ -20,6 +20,9 @@ public sealed class PendingUpdatesCache : IPendingUpdatesCache private readonly IServiceScopeFactory _scopeFactory; private readonly SingleFlightMemo _memo; + /// Negative TTL after a faulted fleet evaluation, so polling does not hammer a sick DB. + internal static readonly TimeSpan FaultBackoff = TimeSpan.FromSeconds(10); + /// /// Initializes the shared pending-updates cache. /// @@ -30,7 +33,8 @@ public sealed class PendingUpdatesCache : IPendingUpdatesCache _scopeFactory = scopeFactory ?? throw new ArgumentNullException(nameof(scopeFactory)); ArgumentNullException.ThrowIfNull(options); - _memo = new SingleFlightMemo(options.Value.PendingUpdatesCacheTtl); + _memo = new SingleFlightMemo( + options.Value.PendingUpdatesCacheTtl, faultBackoff: FaultBackoff); } /// diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/SingleFlightMemo.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/SingleFlightMemo.cs index a95731e8..bd5c778b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/SingleFlightMemo.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/SingleFlightMemo.cs @@ -11,9 +11,10 @@ namespace ZB.MOM.WW.ScadaBridge.CentralUI.Services; /// memoized value until the TTL expires. /// /// -/// Failures are never cached — a faulted flight is discarded so the next caller retries. -/// The caller's own exception handling is therefore unchanged: it still sees the fault -/// (all callers of that one flight see it), and the next window starts clean. +/// By default a faulted flight is discarded so the next caller retries. A slot built with a +/// faultBackoff instead remembers the fault for that short negative TTL so a sick +/// dependency is not hammered by every poll; a forced refresh bypasses the back-off. +/// The caller's own exception handling is unchanged: it still sees the fault. /// /// /// The memoized value type. @@ -29,14 +30,25 @@ internal sealed class SingleFlightMemo /// Wall-clock instant after which is stale. Only meaningful when it completed successfully. private DateTimeOffset _freshUntil; + /// + /// How long a faulted flight is remembered (zero = never). While inside the window, callers + /// are served the same fault instead of starting another flight against a sick dependency. + /// + private readonly TimeSpan _faultBackoff; + + /// Wall-clock instant until which a faulted is re-served. + private DateTimeOffset _faultedUntil; + /// /// Initializes a memo slot. /// /// How long a successfully produced value stays fresh. /// Clock used for freshness, injectable so tests need no real delay. - public SingleFlightMemo(TimeSpan ttl, Func? clock = null) + /// Negative TTL after a faulted flight; null/zero disables it. + public SingleFlightMemo(TimeSpan ttl, Func? clock = null, TimeSpan? faultBackoff = null) { _ttl = ttl; + _faultBackoff = faultBackoff ?? TimeSpan.Zero; _clock = clock ?? (() => DateTimeOffset.UtcNow); } @@ -83,6 +95,12 @@ internal sealed class SingleFlightMemo // A round is already in progress — join it, force or not. result = current; } + else if (!forceRefresh + && current is { IsFaulted: true } + && _clock() < _faultedUntil) + { + result = current; + } else if (!forceRefresh && current is { IsCompletedSuccessfully: true } && _clock() < _freshUntil) @@ -124,11 +142,12 @@ internal sealed class SingleFlightMemo } catch (Exception ex) { - // A failure is not memoized: leave _freshUntil in the past so the next caller - // starts a new flight instead of re-serving the fault for the whole TTL. + // A failure is memoized only for the optional short fault back-off so a sick dependency + // is not retried by every poll; _freshUntil stays in the past. lock (_lock) { _freshUntil = default; + _faultedUntil = _faultBackoff > TimeSpan.Zero ? _clock() + _faultBackoff : default; } completion.SetException(ex); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/PendingUpdatesCacheTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/PendingUpdatesCacheTests.cs index c38ff5f0..133f1291 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/PendingUpdatesCacheTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/PendingUpdatesCacheTests.cs @@ -115,10 +115,10 @@ public class PendingUpdatesCacheTests : IDisposable await _evaluator.Received(1).EvaluateAsync(Arg.Any()); } - /// Verifies that a failed evaluation surfaces to the caller and is not memoized. + /// Verifies that a failed evaluation surfaces to the caller and is remembered only for the short back-off (review 2026-10-21 04/C4). /// A task that represents the asynchronous operation. [Fact] - public async Task FailedEvaluation_SurfacesToTheCaller_AndIsNotMemoized() + public async Task FailedEvaluation_SurfacesToTheCaller_AndIsBackedOffNotRetriedPerPoll() { _evaluator.EvaluateAsync(Arg.Any()) .Returns( @@ -128,10 +128,12 @@ public class PendingUpdatesCacheTests : IDisposable await Assert.ThrowsAsync(() => sut.GetAsync()); - // Same instant, well inside the TTL — a cached fault would starve every dashboard - // for the rest of the window, so the next caller must start a new evaluation. - var recovered = await sut.GetAsync(); + // Inside the negative TTL a poll is served the same fault with no new flatten. + await Assert.ThrowsAsync(() => sut.GetAsync()); + await _evaluator.Received(1).EvaluateAsync(Arg.Any()); + // An operator Re-check bypasses the back-off. + var recovered = await sut.GetAsync(forceRefresh: true); Assert.NotNull(recovered); await _evaluator.Received(2).EvaluateAsync(Arg.Any()); } diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/SingleFlightMemoTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/SingleFlightMemoTests.cs index ce7105fe..08f84078 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/SingleFlightMemoTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/SingleFlightMemoTests.cs @@ -141,6 +141,26 @@ public class SingleFlightMemoTests Assert.Equal(2, memo.FlightCount); } + /// Verifies the opt-in fault back-off memoizes a fault briefly, then retries. + /// A task that represents the asynchronous operation. + [Fact] + public async Task FaultBackoff_ReservesTheFault_ThenRetriesAfterWindow() + { + var now = T0; + var invocations = 0; + var memo = new SingleFlightMemo(TimeSpan.FromSeconds(60), () => now, TimeSpan.FromSeconds(10)); + + await Assert.ThrowsAsync(() => + memo.GetAsync(() => { invocations++; return Task.FromException(new InvalidOperationException("x")); })); + now = T0.AddSeconds(5); + await Assert.ThrowsAsync(() => memo.GetAsync(() => Task.FromResult(++invocations))); + Assert.Equal(1, memo.FlightCount); + + now = T0.AddSeconds(11); + Assert.Equal(2, await memo.GetAsync(() => Task.FromResult(++invocations))); + Assert.Equal(2, memo.FlightCount); + } + /// Verifies that a TTL override applies to the flight it was supplied for. /// A task that represents the asynchronous operation. [Fact]