fix(centralui): short negative TTL after a faulted pending-updates flight (review 2026-10-21 04/C4)

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