fix(site-call-audit): same-rank freshness tiebreaker — retrying calls no longer freeze RetryCount/LastError at first write
Within an equal NON-terminal rank the newest UpdatedAtUtc wins, unfreezing the Attempted-phase RetryCount/LastError/HttpStatus. Terminal ranks (>=3) are excluded from the tiebreaker so terminal immutability is preserved (Delivered never overwrites Parked); equal stamps stay idempotent, lower rank stays a no-op.
This commit is contained in:
@@ -73,9 +73,15 @@ mirror — never queried by scripts (`Tracking.Status()` is answered site-locall
|
|||||||
## Ingest & Idempotency
|
## Ingest & Idempotency
|
||||||
|
|
||||||
Telemetry ingestion is **insert-if-not-exists** keyed on `TrackedOperationId`,
|
Telemetry ingestion is **insert-if-not-exists** keyed on `TrackedOperationId`,
|
||||||
then **upsert-on-newer-status**. The lifecycle is monotonic, so status only
|
then **upsert-on-newer-status, with a newest-`UpdatedAtUtc` tiebreaker within
|
||||||
advances and never regresses; at-least-once and out-of-order telemetry are
|
equal non-terminal rank**. The lifecycle is monotonic on status rank, so status
|
||||||
therefore harmless.
|
never regresses. Within an equal *non-terminal* rank (the `Attempted`/`Skipped`
|
||||||
|
retry phase), the packet with the newest `UpdatedAtUtc` wins — so a retrying
|
||||||
|
call's `RetryCount`/`LastError`/`HttpStatus` stay live instead of freezing at the
|
||||||
|
first `Attempted` write. Equal *terminal* rank stays immutable (a later
|
||||||
|
`Delivered` never overwrites an earlier `Parked`), equal stamps are an idempotent
|
||||||
|
no-op, and a lower rank is always a no-op — so at-least-once and out-of-order
|
||||||
|
telemetry remain harmless.
|
||||||
|
|
||||||
From v1.x onward, the `CachedCallTelemetry` message additively carries the
|
From v1.x onward, the `CachedCallTelemetry` message additively carries the
|
||||||
`AuditEvent` content alongside the existing operational fields. Central's
|
`AuditEvent` content alongside the existing operational fields. Central's
|
||||||
|
|||||||
+35
-8
@@ -24,9 +24,17 @@ public class SiteCallAuditRepository : ISiteCallAuditRepository
|
|||||||
private const int SqlErrorUniqueIndexViolation = 2601;
|
private const int SqlErrorUniqueIndexViolation = 2601;
|
||||||
private const int SqlErrorPrimaryKeyViolation = 2627;
|
private const int SqlErrorPrimaryKeyViolation = 2627;
|
||||||
|
|
||||||
// Monotonic status ordering. Lower rank wins on tie (same-rank upserts are
|
// Monotonic status ordering:
|
||||||
// no-ops, including terminal-over-terminal):
|
|
||||||
// Submitted < Forwarded < Attempted == Skipped < Delivered == Failed == Parked == Discarded.
|
// Submitted < Forwarded < Attempted == Skipped < Delivered == Failed == Parked == Discarded.
|
||||||
|
// A higher incoming rank always wins. WITHIN an equal NON-terminal rank
|
||||||
|
// (Attempted/Skipped, rank 2 — and the transient Submitted/Forwarded ranks),
|
||||||
|
// the newest UpdatedAtUtc wins so a retrying call's live RetryCount/LastError
|
||||||
|
// no longer freezes at first-write (Task 11). Equal terminal ranks (rank 3)
|
||||||
|
// stay immutable — the freshness tiebreaker is deliberately scoped to
|
||||||
|
// rank < 3, so a later terminal NEVER flips an earlier one (Delivered cannot
|
||||||
|
// overwrite Parked). Still idempotent (equal stamps are inert) and still
|
||||||
|
// regression-proof (a lower rank is always a no-op).
|
||||||
|
private const int TerminalRank = 3;
|
||||||
private static readonly Dictionary<string, int> StatusRank = new(StringComparer.Ordinal)
|
private static readonly Dictionary<string, int> StatusRank = new(StringComparer.Ordinal)
|
||||||
{
|
{
|
||||||
["Submitted"] = 0,
|
["Submitted"] = 0,
|
||||||
@@ -95,10 +103,16 @@ VALUES
|
|||||||
idText);
|
idText);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 2: monotonic update. The CASE expression maps the stored Status
|
// Step 2: monotonic update with a same-rank freshness tiebreaker. The
|
||||||
// string to the same rank table the caller uses; we only mutate if the
|
// CASE expression maps the stored Status string to the same rank table
|
||||||
// incoming rank is strictly greater. Same-rank (including
|
// the caller uses. We mutate when EITHER the incoming rank is strictly
|
||||||
// terminal-over-terminal) is a no-op — first-write-wins at each rank.
|
// greater, OR the incoming rank equals the stored rank AND that rank is
|
||||||
|
// non-terminal (< TerminalRank) AND the incoming UpdatedAtUtc is strictly
|
||||||
|
// newer than the stored one — so a retrying call's Attempted-phase
|
||||||
|
// RetryCount/LastError/HttpStatus stay live instead of freezing at the
|
||||||
|
// first Attempted packet (Task 11). Terminal ranks are excluded from the
|
||||||
|
// tiebreaker, so a later terminal NEVER overwrites an earlier one; equal
|
||||||
|
// stamps are inert (idempotent replay) and a lower rank is always a no-op.
|
||||||
//
|
//
|
||||||
// SourceNode-stamping: SourceNode is updated via
|
// SourceNode-stamping: SourceNode is updated via
|
||||||
// COALESCE(@SourceNode, SourceNode). The operator returns @SourceNode
|
// COALESCE(@SourceNode, SourceNode). The operator returns @SourceNode
|
||||||
@@ -125,7 +139,7 @@ SET Status = {siteCall.Status},
|
|||||||
IngestedAtUtc = {siteCall.IngestedAtUtc},
|
IngestedAtUtc = {siteCall.IngestedAtUtc},
|
||||||
SourceNode = COALESCE({siteCall.SourceNode}, SourceNode)
|
SourceNode = COALESCE({siteCall.SourceNode}, SourceNode)
|
||||||
WHERE TrackedOperationId = {idText}
|
WHERE TrackedOperationId = {idText}
|
||||||
AND {incomingRank} > (CASE Status
|
AND ( {incomingRank} > (CASE Status
|
||||||
WHEN 'Submitted' THEN 0
|
WHEN 'Submitted' THEN 0
|
||||||
WHEN 'Forwarded' THEN 1
|
WHEN 'Forwarded' THEN 1
|
||||||
WHEN 'Attempted' THEN 2
|
WHEN 'Attempted' THEN 2
|
||||||
@@ -135,7 +149,20 @@ WHERE TrackedOperationId = {idText}
|
|||||||
WHEN 'Parked' THEN 3
|
WHEN 'Parked' THEN 3
|
||||||
WHEN 'Discarded' THEN 3
|
WHEN 'Discarded' THEN 3
|
||||||
ELSE -1
|
ELSE -1
|
||||||
END);",
|
END)
|
||||||
|
OR ( {incomingRank} = (CASE Status
|
||||||
|
WHEN 'Submitted' THEN 0
|
||||||
|
WHEN 'Forwarded' THEN 1
|
||||||
|
WHEN 'Attempted' THEN 2
|
||||||
|
WHEN 'Skipped' THEN 2
|
||||||
|
WHEN 'Delivered' THEN 3
|
||||||
|
WHEN 'Failed' THEN 3
|
||||||
|
WHEN 'Parked' THEN 3
|
||||||
|
WHEN 'Discarded' THEN 3
|
||||||
|
ELSE -1
|
||||||
|
END)
|
||||||
|
AND {incomingRank} < {TerminalRank}
|
||||||
|
AND UpdatedAtUtc < {siteCall.UpdatedAtUtc} ) );",
|
||||||
ct);
|
ct);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+82
-7
@@ -95,19 +95,23 @@ public class SiteCallAuditRepositoryTests : IClassFixture<MsSqlMigrationFixture>
|
|||||||
}
|
}
|
||||||
|
|
||||||
[SkippableFact]
|
[SkippableFact]
|
||||||
public async Task UpsertAsync_SameStatus_IsNoOp()
|
public async Task UpsertAsync_SameStatus_EqualUpdatedAt_IsNoOp()
|
||||||
{
|
{
|
||||||
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
||||||
|
|
||||||
var id = TrackedOperationId.New();
|
var id = TrackedOperationId.New();
|
||||||
|
var stamp = new DateTime(2026, 6, 1, 9, 0, 0, DateTimeKind.Utc);
|
||||||
await using var context = CreateContext();
|
await using var context = CreateContext();
|
||||||
var repo = new SiteCallAuditRepository(context);
|
var repo = new SiteCallAuditRepository(context);
|
||||||
|
|
||||||
await repo.UpsertAsync(NewRow(id, status: "Attempted", retryCount: 1, lastError: "first"));
|
await repo.UpsertAsync(NewRow(id, status: "Attempted", retryCount: 1, lastError: "first", updatedAtUtc: stamp));
|
||||||
var snapshot = await repo.GetAsync(id);
|
var snapshot = await repo.GetAsync(id);
|
||||||
|
|
||||||
// Same rank (2) — repository must treat this as a no-op (no fields move).
|
// Same rank (2) AND an EQUAL UpdatedAtUtc — an idempotent replay of the
|
||||||
await repo.UpsertAsync(NewRow(id, status: "Attempted", retryCount: 2, lastError: "second"));
|
// same telemetry packet. The same-rank freshness tiebreaker only fires
|
||||||
|
// for a STRICTLY-newer stamp, so a true duplicate stays a no-op (no
|
||||||
|
// fields move). Task 11: newest-within-rank wins, equal stamps are inert.
|
||||||
|
await repo.UpsertAsync(NewRow(id, status: "Attempted", retryCount: 2, lastError: "second", updatedAtUtc: stamp));
|
||||||
var afterDuplicate = await repo.GetAsync(id);
|
var afterDuplicate = await repo.GetAsync(id);
|
||||||
|
|
||||||
Assert.NotNull(afterDuplicate);
|
Assert.NotNull(afterDuplicate);
|
||||||
@@ -117,21 +121,92 @@ public class SiteCallAuditRepositoryTests : IClassFixture<MsSqlMigrationFixture>
|
|||||||
Assert.Equal(snapshot!.UpdatedAtUtc, afterDuplicate.UpdatedAtUtc);
|
Assert.Equal(snapshot!.UpdatedAtUtc, afterDuplicate.UpdatedAtUtc);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[SkippableFact]
|
||||||
|
public async Task UpsertAsync_SameRank_NewerUpdatedAt_RefreshesProgressFields()
|
||||||
|
{
|
||||||
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
||||||
|
|
||||||
|
var id = TrackedOperationId.New();
|
||||||
|
var t0 = new DateTime(2026, 6, 1, 10, 0, 0, DateTimeKind.Utc);
|
||||||
|
await using var context = CreateContext();
|
||||||
|
var repo = new SiteCallAuditRepository(context);
|
||||||
|
|
||||||
|
// Two Attempted packets (rank 2) from one retrying call; the second
|
||||||
|
// carries a strictly-newer UpdatedAtUtc, so its live RetryCount/LastError
|
||||||
|
// must land — the row no longer freezes at first-Attempted (Task 11).
|
||||||
|
await repo.UpsertAsync(NewRow(id, status: "Attempted", retryCount: 1, lastError: "timeout #1", updatedAtUtc: t0));
|
||||||
|
await repo.UpsertAsync(NewRow(id, status: "Attempted", retryCount: 5, lastError: "timeout #5", updatedAtUtc: t0.AddMinutes(5)));
|
||||||
|
|
||||||
|
var row = await repo.GetAsync(id);
|
||||||
|
Assert.NotNull(row);
|
||||||
|
Assert.Equal(5, row!.RetryCount);
|
||||||
|
Assert.Equal("timeout #5", row.LastError);
|
||||||
|
Assert.Equal(t0.AddMinutes(5), row.UpdatedAtUtc);
|
||||||
|
}
|
||||||
|
|
||||||
|
[SkippableFact]
|
||||||
|
public async Task UpsertAsync_SameRank_OlderUpdatedAt_IsNoOp()
|
||||||
|
{
|
||||||
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
||||||
|
|
||||||
|
var id = TrackedOperationId.New();
|
||||||
|
var t0 = new DateTime(2026, 6, 1, 11, 0, 0, DateTimeKind.Utc);
|
||||||
|
await using var context = CreateContext();
|
||||||
|
var repo = new SiteCallAuditRepository(context);
|
||||||
|
|
||||||
|
// Land retry 5 (stamp t0+5), then an out-of-order retry-2 packet with an
|
||||||
|
// OLDER stamp (t0). Same rank, older UpdatedAtUtc → rejected; stays at 5.
|
||||||
|
await repo.UpsertAsync(NewRow(id, status: "Attempted", retryCount: 5, lastError: "timeout #5", updatedAtUtc: t0.AddMinutes(5)));
|
||||||
|
await repo.UpsertAsync(NewRow(id, status: "Attempted", retryCount: 2, lastError: "timeout #2", updatedAtUtc: t0));
|
||||||
|
|
||||||
|
var row = await repo.GetAsync(id);
|
||||||
|
Assert.NotNull(row);
|
||||||
|
Assert.Equal(5, row!.RetryCount);
|
||||||
|
Assert.Equal("timeout #5", row.LastError);
|
||||||
|
Assert.Equal(t0.AddMinutes(5), row.UpdatedAtUtc);
|
||||||
|
}
|
||||||
|
|
||||||
|
[SkippableFact]
|
||||||
|
public async Task UpsertAsync_LowerRank_NewerUpdatedAt_IsStillNoOp()
|
||||||
|
{
|
||||||
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
||||||
|
|
||||||
|
var id = TrackedOperationId.New();
|
||||||
|
var t0 = new DateTime(2026, 6, 1, 12, 0, 0, DateTimeKind.Utc);
|
||||||
|
await using var context = CreateContext();
|
||||||
|
var repo = new SiteCallAuditRepository(context);
|
||||||
|
|
||||||
|
// Terminal Delivered (rank 3), then a late lower-rank Attempted (rank 2)
|
||||||
|
// with a NEWER stamp. Rank monotonicity dominates the freshness
|
||||||
|
// tiebreaker — the row stays Delivered (regression-proof).
|
||||||
|
await repo.UpsertAsync(NewRow(id, status: "Delivered", retryCount: 0, updatedAtUtc: t0, terminal: true, terminalAtUtc: t0));
|
||||||
|
await repo.UpsertAsync(NewRow(id, status: "Attempted", retryCount: 9, lastError: "late", updatedAtUtc: t0.AddMinutes(5)));
|
||||||
|
|
||||||
|
var row = await repo.GetAsync(id);
|
||||||
|
Assert.NotNull(row);
|
||||||
|
Assert.Equal("Delivered", row!.Status);
|
||||||
|
Assert.Equal(0, row.RetryCount);
|
||||||
|
}
|
||||||
|
|
||||||
[SkippableFact]
|
[SkippableFact]
|
||||||
public async Task UpsertAsync_TerminalOverTerminal_IsNoOp()
|
public async Task UpsertAsync_TerminalOverTerminal_IsNoOp()
|
||||||
{
|
{
|
||||||
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
Skip.IfNot(_fixture.Available, _fixture.SkipReason);
|
||||||
|
|
||||||
// Bundle B3 plan: terminal statuses share rank 3 and are mutually
|
// Bundle B3 plan: terminal statuses share rank 3 and are mutually
|
||||||
// exclusive — Delivered cannot overwrite Parked.
|
// exclusive — Delivered cannot overwrite Parked. Task 11 note: the
|
||||||
|
// Delivered packet carries a STRICTLY-NEWER UpdatedAtUtc, yet the row
|
||||||
|
// stays Parked — the same-rank freshness tiebreaker is scoped to
|
||||||
|
// NON-terminal ranks (< 3), so terminal immutability is preserved.
|
||||||
var id = TrackedOperationId.New();
|
var id = TrackedOperationId.New();
|
||||||
|
var t0 = new DateTime(2026, 6, 1, 13, 0, 0, DateTimeKind.Utc);
|
||||||
await using var context = CreateContext();
|
await using var context = CreateContext();
|
||||||
var repo = new SiteCallAuditRepository(context);
|
var repo = new SiteCallAuditRepository(context);
|
||||||
|
|
||||||
await repo.UpsertAsync(NewRow(id, status: "Parked", retryCount: 3, lastError: "parked-reason", terminal: true));
|
await repo.UpsertAsync(NewRow(id, status: "Parked", retryCount: 3, lastError: "parked-reason", updatedAtUtc: t0, terminal: true, terminalAtUtc: t0));
|
||||||
var afterPark = await repo.GetAsync(id);
|
var afterPark = await repo.GetAsync(id);
|
||||||
|
|
||||||
await repo.UpsertAsync(NewRow(id, status: "Delivered", retryCount: 4, lastError: null, terminal: true));
|
await repo.UpsertAsync(NewRow(id, status: "Delivered", retryCount: 4, lastError: null, updatedAtUtc: t0.AddMinutes(5), terminal: true, terminalAtUtc: t0.AddMinutes(5)));
|
||||||
var afterDeliveredAttempt = await repo.GetAsync(id);
|
var afterDeliveredAttempt = await repo.GetAsync(id);
|
||||||
|
|
||||||
Assert.NotNull(afterDeliveredAttempt);
|
Assert.NotNull(afterDeliveredAttempt);
|
||||||
|
|||||||
Reference in New Issue
Block a user