fix(kpi): classify the failover fold-race pass loss as a self-healing Information event (plan R2-04 T9)

This commit is contained in:
Joseph Doherty
2026-07-13 10:21:02 -04:00
parent cd93ad3976
commit 439dee52d3
3 changed files with 128 additions and 3 deletions
+1 -1
View File
@@ -88,7 +88,7 @@ A timer fires every `SampleInterval` (default 60s; an immediate first tick prime
**Best-effort, per-source isolation.** Each source call and the write are individually guarded. A throwing source is logged and its samples skipped for that tick; it never aborts the tick, the other sources, or the source component itself. This is the same `IEnumerable<>`-of-adapters decoupling pattern used by `INotificationDeliveryAdapter`. **Best-effort, per-source isolation.** Each source call and the write are individually guarded. A throwing source is logged and its samples skipped for that tick; it never aborts the tick, the other sources, or the source component itself. This is the same `IEnumerable<>`-of-adapters decoupling pattern used by `INotificationDeliveryAdapter`.
**Hourly rollup tick.** A third timer (`kpi-rollup`) fires every `RollupInterval` (default 1h). On each tick the recorder opens a per-tick DI scope and calls `IKpiHistoryRepository.FoldHourlyRollupsAsync` over the window `[TruncateToHour(now) RollupLookbackHours, TruncateToHour(now))` — an **exclusive** upper bound at the current hour start, so the in-progress hour is never folded. The fold groups raw `KpiSample` rows by `(series, hour)`, applies the per-metric aggregation intent (sum for rate metrics, last-value for gauges — see Query), and **idempotently upserts** each `(Source, Metric, Scope, ScopeKey, HourStartUtc)` row. Re-folding the trailing lookback window (default 3h) rather than only the last hour is the self-heal for a **singleton-failover-missed tick**: because the upsert overwrites the aggregate in place, re-running the same window produces identical values (no double-count), so any complete hour a missed tick skipped is refilled on the next fold. A `_rollupInFlight` guard (mirroring `_sampleInFlight`) skips a tick while a prior fold is still running so two idempotent upserts never race on overlapping rows. **Hourly rollup tick.** A third timer (`kpi-rollup`) fires every `RollupInterval` (default 1h). On each tick the recorder opens a per-tick DI scope and calls `IKpiHistoryRepository.FoldHourlyRollupsAsync` over the window `[TruncateToHour(now) RollupLookbackHours, TruncateToHour(now))` — an **exclusive** upper bound at the current hour start, so the in-progress hour is never folded. The fold groups raw `KpiSample` rows by `(series, hour)`, applies the per-metric aggregation intent (sum for rate metrics, last-value for gauges — see Query), and **idempotently upserts** each `(Source, Metric, Scope, ScopeKey, HourStartUtc)` row. Re-folding the trailing lookback window (default 3h) rather than only the last hour is the self-heal for a **singleton-failover-missed tick**: because the upsert overwrites the aggregate in place, re-running the same window produces identical values (no double-count), so any complete hour a missed tick skipped is refilled on the next fold. A `_rollupInFlight` guard (mirroring `_sampleInFlight`) skips a tick while a prior fold is still running so two idempotent upserts never race on overlapping rows. During a **failover overlap** — the old singleton incarnation's in-flight fold and the new node's first fold both `Add`-ing the same `(series, hour)` row — the loser of the unique `IX_KpiRollupHourly_Series` upsert race discards its **whole fold pass** (a single `SaveChanges`), logged at Information and repaired by the next idempotent fold; when reading fold logs after a failover, the failure grain is the **pass, not the row** (arch-review 04 round 2, R5).
**One-shot backfill.** So the 30 d / 90 d charts are not blank until enough wall-clock passes, the recorder runs a backfill on start (`kpi-rollup-backfill` timer) that folds the retention window, reusing the same `FoldHourlyRollupsAsync` path. The `FoldHourlyRollupsAsync` repository call materializes its whole window in memory, so the backfill folds the window in **≤24 h slices, oldest-first** — one bounded slice per fold — rather than handing the fold the full 90-day window at once (which at fleet volume is tens of millions of rows in one tracked pass). Between slices it lowers its in-flight guard so periodic folds **interleave** instead of stalling behind the historical pass; a periodic fold and a backfill slice strictly alternate through the single `_rollupInFlight`/`_backfillInFlight` gate pair, so the two idempotent upserts never race on overlapping rows. On a **failover restart** the plan first consults the `IKpiHistoryRepository.GetLatestRollupHourAsync` watermark: if the newest folded hour is already within `RollupLookbackHours` of now (the common case) the whole historical pass is **skipped** — the periodic fold's lookback covers the tail — otherwise the window is **shrunk to the un-rolled tail** `[watermark, TruncateToHour(now))` (re-folding the watermark hour itself as an idempotent safety margin) instead of the full `RetentionDays` floor. A `_backfillDone` latch makes the whole pass at-most-once per actor lifetime, and it defers (re-arms) while a periodic fold is in flight. Being an idempotent upsert, it is safe (idempotent) to re-run on a later failover restart; the watermark fast-path makes the failover re-run cheap as well. **One-shot backfill.** So the 30 d / 90 d charts are not blank until enough wall-clock passes, the recorder runs a backfill on start (`kpi-rollup-backfill` timer) that folds the retention window, reusing the same `FoldHourlyRollupsAsync` path. The `FoldHourlyRollupsAsync` repository call materializes its whole window in memory, so the backfill folds the window in **≤24 h slices, oldest-first** — one bounded slice per fold — rather than handing the fold the full 90-day window at once (which at fleet volume is tens of millions of rows in one tracked pass). Between slices it lowers its in-flight guard so periodic folds **interleave** instead of stalling behind the historical pass; a periodic fold and a backfill slice strictly alternate through the single `_rollupInFlight`/`_backfillInFlight` gate pair, so the two idempotent upserts never race on overlapping rows. On a **failover restart** the plan first consults the `IKpiHistoryRepository.GetLatestRollupHourAsync` watermark: if the newest folded hour is already within `RollupLookbackHours` of now (the common case) the whole historical pass is **skipped** — the periodic fold's lookback covers the tail — otherwise the window is **shrunk to the un-rolled tail** `[watermark, TruncateToHour(now))` (re-folding the watermark hour itself as an idempotent safety margin) instead of the full `RetentionDays` floor. A `_backfillDone` latch makes the whole pass at-most-once per actor lifetime, and it defers (re-arms) while a periodic fold is in flight. Being an idempotent upsert, it is safe (idempotent) to re-run on a later failover restart; the watermark fast-path makes the failover re-run cheap as well.
@@ -1,4 +1,6 @@
using Microsoft.EntityFrameworkCore; using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Abstractions;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Kpi; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Kpi;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Kpi; using ZB.MOM.WW.ScadaBridge.Commons.Types.Kpi;
@@ -13,14 +15,22 @@ namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories;
public sealed class KpiHistoryRepository : IKpiHistoryRepository public sealed class KpiHistoryRepository : IKpiHistoryRepository
{ {
private readonly ScadaBridgeDbContext _context; private readonly ScadaBridgeDbContext _context;
private readonly ILogger<KpiHistoryRepository> _logger;
/// <summary> /// <summary>
/// Initializes a new instance of the <see cref="KpiHistoryRepository"/> class. /// Initializes a new instance of the <see cref="KpiHistoryRepository"/> class.
/// </summary> /// </summary>
/// <param name="context">The EF Core database context.</param> /// <param name="context">The EF Core database context.</param>
public KpiHistoryRepository(ScadaBridgeDbContext context) /// <param name="logger">
/// Optional logger; defaults to <see cref="NullLogger{T}"/> (mirrors
/// <c>SiteCallAuditRepository</c> — MS.DI resolves <see cref="ILogger{T}"/> automatically,
/// so no registration churn). Used to classify the self-healing failover fold-race.
/// </param>
public KpiHistoryRepository(
ScadaBridgeDbContext context, ILogger<KpiHistoryRepository>? logger = null)
{ {
_context = context ?? throw new ArgumentNullException(nameof(context)); _context = context ?? throw new ArgumentNullException(nameof(context));
_logger = logger ?? NullLogger<KpiHistoryRepository>.Instance;
} }
/// <inheritdoc /> /// <inheritdoc />
@@ -170,7 +180,22 @@ public sealed class KpiHistoryRepository : IKpiHistoryRepository
} }
} }
await _context.SaveChangesAsync(cancellationToken); try
{
await _context.SaveChangesAsync(cancellationToken);
}
catch (DbUpdateException ex)
{
// Failover-overlap race: the old singleton incarnation's in-flight fold and the
// new node's first fold can both Add the same (series, hour) row; the unique
// IX_KpiRollupHourly_Series then faults the loser's ENTIRE SaveChanges — the
// failure grain is the PASS, not the row (arch-review 04 round 2, R5). The fold
// only ever writes KpiRollupHourly rows, so any save fault here is a lost upsert
// race or a transient the next idempotent re-fold repairs identically; classify
// at Information instead of surfacing a scary error for a self-healing no-op.
_logger.LogInformation(ex,
"KPI rollup fold lost a failover-overlap upsert race; pass discarded, next fold self-heals.");
}
} }
/// <inheritdoc /> /// <inheritdoc />
@@ -429,10 +429,110 @@ public class KpiHistoryRepositoryTests
Assert.Equal(Base.AddHours(3), await repo.GetLatestRollupHourAsync()); Assert.Equal(Base.AddHours(3), await repo.GetLatestRollupHourAsync());
} }
// ---- Task 9: failover fold-race failure grain is the pass, not the row (R5) ----
[Fact]
public async Task Fold_LosingFailoverUpsertRace_DoesNotThrow_AndNextFoldSelfHeals()
{
// A single shared in-memory SQLite connection so a second context (the interceptor's
// side-write, and the "next tick" fold) sees the same database.
await using var connection = new Microsoft.Data.Sqlite.SqliteConnection("DataSource=:memory:");
await connection.OpenAsync();
await using (var schema = ContextOn(connection))
{
await schema.Database.EnsureCreatedAsync();
}
// The old singleton incarnation's in-flight fold wins the (series, hour) upsert race:
// the interceptor inserts the conflicting rollup row on the same connection just before
// THIS fold's SaveChanges, so the unique IX_KpiRollupHourly_Series faults the loser's save.
// A non-null ScopeKey (site-scoped series) is used deliberately: SQLite treats NULLs as
// distinct in a UNIQUE index, so only a non-null key deterministically forces the conflict.
var interceptor = new InsertConflictingRollupOnFirstSave(connection, () => new KpiRollupHourly
{
Source = "NotificationOutbox", Metric = "queueDepth", Scope = "Site", ScopeKey = "plant-a",
HourStartUtc = Base, Value = -1, MinValue = -1, MaxValue = -1, SampleCount = 1,
});
await using var ctx = ContextOn(connection, interceptor);
var repo = new KpiHistoryRepository(ctx);
await repo.RecordSamplesAsync(new[]
{
Sample("NotificationOutbox", "queueDepth", "Site", "plant-a", 5, Base.AddMinutes(10)),
});
// Losing the race must NOT propagate DbUpdateException — the failure grain is the pass.
var ex = await Record.ExceptionAsync(() => repo.FoldHourlyRollupsAsync(Base, Base.AddHours(1)));
Assert.Null(ex);
// Next tick's fold (a fresh scope/context over the same DB) self-heals: it finds the
// conflicting row and idempotently upserts it in place to the correct folded value.
await using var ctx2 = ContextOn(connection);
var repo2 = new KpiHistoryRepository(ctx2);
await repo2.FoldHourlyRollupsAsync(Base, Base.AddHours(1));
var series = await repo2.GetHourlySeriesAsync(
"NotificationOutbox", "queueDepth", "Site", "plant-a", Base, Base.AddHours(1));
Assert.Single(series);
Assert.Equal(5, series[0].Value); // the fold's own value overwrote the side-inserted -1
}
// Local mirror of the repo's private hour-truncation, for cutoff assertions. // Local mirror of the repo's private hour-truncation, for cutoff assertions.
private static DateTime TruncateHour(DateTime utc) => private static DateTime TruncateHour(DateTime utc) =>
new(utc.Year, utc.Month, utc.Day, utc.Hour, 0, 0, DateTimeKind.Utc); new(utc.Year, utc.Month, utc.Day, utc.Hour, 0, 0, DateTimeKind.Utc);
/// <summary>Builds a SQLite test context bound to an externally-owned open connection
/// (so multiple contexts share one in-memory database), with optional interceptors.</summary>
private static ScadaBridgeDbContext ContextOn(DbConnection connection, params IInterceptor[] interceptors)
{
var options = new DbContextOptionsBuilder<ScadaBridgeDbContext>()
.UseSqlite(connection)
.ConfigureWarnings(w => w.Ignore(RelationalEventId.PendingModelChangesWarning))
.AddInterceptors(interceptors)
.Options;
return new SqliteTestDbContext(options);
}
/// <summary>
/// SaveChanges interceptor that, exactly once and only when a fold is about to insert a
/// <see cref="KpiRollupHourly"/>, inserts a conflicting (series, hour) rollup row through a
/// second context on the same connection — deterministically reproducing the failover-overlap
/// upsert race the losing fold must survive (arch-review 04 round 2, R5).
/// </summary>
private sealed class InsertConflictingRollupOnFirstSave : Microsoft.EntityFrameworkCore.Diagnostics.SaveChangesInterceptor
{
private readonly DbConnection _connection;
private readonly Func<KpiRollupHourly> _rowFactory;
private bool _fired;
public InsertConflictingRollupOnFirstSave(DbConnection connection, Func<KpiRollupHourly> rowFactory)
{
_connection = connection;
_rowFactory = rowFactory;
}
public override async ValueTask<InterceptionResult<int>> SavingChangesAsync(
DbContextEventData eventData, InterceptionResult<int> result,
CancellationToken cancellationToken = default)
{
if (!_fired && eventData.Context is not null &&
eventData.Context.ChangeTracker.Entries<KpiRollupHourly>()
.Any(e => e.State == EntityState.Added))
{
_fired = true;
var options = new DbContextOptionsBuilder<ScadaBridgeDbContext>()
.UseSqlite(_connection)
.ConfigureWarnings(w => w.Ignore(RelationalEventId.PendingModelChangesWarning))
.Options;
await using var side = new SqliteTestDbContext(options);
side.KpiRollupHourly.Add(_rowFactory());
await side.SaveChangesAsync(cancellationToken);
}
return await base.SavingChangesAsync(eventData, result, cancellationToken);
}
}
/// <summary> /// <summary>
/// EF command interceptor that counts the DELETE statements actually issued /// EF command interceptor that counts the DELETE statements actually issued
/// to the store, so a test can prove the purge is sliced into multiple /// to the store, so a test can prove the purge is sliced into multiple