mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-25 05:00:07 +00:00
fix(backend,web): PR6 codex-review follow-ups (F1 legacy pre-check, F2 mid-run kill-switch, F3 OpsEvent audit shift)
Three Round-1 review findings from PR6:
F1 (High) — Legacy retention path mutated side-tables before the
CAS-guarded DeleteSessionAsync. Plan PR6 step 7 says "legacy must skip
locked sessions", but our LegacyDeleteAsync deleted Events / RuleResults
/ AppInstallSummaries first and only discovered the V2 cascade lock at
the tombstone. Fix: pre-read the Sessions row in LegacyDeleteAsync; if
DeletionState != None, return false WITHOUT touching any side table.
The original CAS-guard stays as a backstop for the T1-T3 race after the
pre-read.
F2 (High) — Kill-switch checked once per run; a flip-ON mid-fanout kept
legacy deletes running. SessionRetentionFanoutService now takes
AdminConfigurationService, calls IsSessionDeletionKillSwitchActiveAsync
(uncached, fail-closed per PR5 F1) at both the per-tenant boundary and
inside the per-session loop. FanoutResult gains AbortedByKillSwitch so
the per-tenant audit + the global OpsEvent surface the early halt.
F3 (Medium) — Global-scope lifecycle audits used LogAuditEntryAsync with
tenantId: null!, which the table-storage helper silently dropped because
the schema requires a non-null PartitionKey. Three event types
(Completed / FanoutSkipped / Failed) moved to OpsEvents (TenantId is
nullable on OpsEventEntry). Two new helpers in OpsEventService —
RecordSessionDeletionMaintenance{Completed,FanoutSkipped}Async — also
dual-registered in the OpsAlertRulesSection.tsx catalog per memory
feedback_ops_event_types_dual_register. Per-tenant audits inside the
fanout (which already had a real tenantId) are unchanged.
Tests: 4 new xUnit cases — F1 skip-without-mutation, F1 row-not-found
skip; F2 mid-tenant abort, F2 between-tenants abort. Existing 17 PR6
tests updated to assert OpsEvents instead of audit calls for the global
lifecycle events. Total: 1504/1504 backend pass, 347/347 web pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+9
-6
@@ -50,7 +50,9 @@ public class SessionDeletionMaintenanceFunctionTests
|
||||
|
||||
harness.BlobMock.Verify(b => b.DeleteDeletionManifestPairAsync("t", "s", "m", It.IsAny<CancellationToken>()), Times.Once);
|
||||
harness.Fanout.Verify(f => f.RunAsync(It.IsAny<CancellationToken>()), Times.Once);
|
||||
Assert.Contains(harness.AuditCalls, a => a.Action == "session_deletion_maintenance_completed");
|
||||
// PR6 follow-up F3: lifecycle status now lives on OpsEvents, not AuditLogs (the latter
|
||||
// would silently fail because PartitionKey requires a non-null tenantId).
|
||||
Assert.Contains(harness.OpsEvents, e => e.EventName == "SessionDeletionMaintenanceCompleted");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -66,10 +68,9 @@ public class SessionDeletionMaintenanceFunctionTests
|
||||
harness.BlobMock.Verify(b => b.DeleteDeletionManifestPairAsync("t", "s", "m", It.IsAny<CancellationToken>()), Times.Once);
|
||||
// Fanout was skipped.
|
||||
harness.Fanout.Verify(f => f.RunAsync(It.IsAny<CancellationToken>()), Times.Never);
|
||||
// Skip audit emitted with kill-switch reason.
|
||||
Assert.Contains(harness.AuditCalls, a => a.Action == "session_deletion_maintenance_fanout_skipped_killswitch");
|
||||
// Completion audit still emitted so dashboards fold the run into the timeline.
|
||||
Assert.Contains(harness.AuditCalls, a => a.Action == "session_deletion_maintenance_completed");
|
||||
// PR6 follow-up F3: fanout-skip + completion now both live on OpsEvents.
|
||||
Assert.Contains(harness.OpsEvents, e => e.EventName == "SessionDeletionMaintenanceFanoutSkipped");
|
||||
Assert.Contains(harness.OpsEvents, e => e.EventName == "SessionDeletionMaintenanceCompleted");
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────── Watchdog OpsEvents ─
|
||||
@@ -139,8 +140,9 @@ public class SessionDeletionMaintenanceFunctionTests
|
||||
var ex = await Assert.ThrowsAsync<InvalidOperationException>(() => harness.Sut.RunCoreAsync(CancellationToken.None));
|
||||
Assert.Equal("fanout exploded", ex.Message);
|
||||
|
||||
// PR6 follow-up F3: the SessionDeletionMaintenanceFailed OpsEvent IS the audit. The prior
|
||||
// parallel LogAuditEntryAsync(null!) call was a silent no-op and has been removed.
|
||||
Assert.Contains(harness.OpsEvents, e => e.EventName == "SessionDeletionMaintenanceFailed");
|
||||
Assert.Contains(harness.AuditCalls, a => a.Action == "session_deletion_maintenance_failed");
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────── Stranded-Queued detection ─
|
||||
@@ -284,6 +286,7 @@ public class SessionDeletionMaintenanceFunctionTests
|
||||
Mock.Of<ISessionRepository>(),
|
||||
new Mock<TenantConfigurationService>(Mock.Of<IConfigRepository>(), NullLogger<TenantConfigurationService>.Instance, memCache).Object,
|
||||
Mock.Of<ISessionDeletionEnqueuer>(),
|
||||
AdminConfig.Object,
|
||||
NullLogger<SessionRetentionFanoutService>.Instance);
|
||||
Fanout.Setup(f => f.RunAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new SessionRetentionFanoutService.FanoutResult { TenantsProcessed = 0, SessionsEnqueued = 0 });
|
||||
|
||||
@@ -138,6 +138,115 @@ public class SessionRetentionFanoutServiceTests
|
||||
harness.MaintenanceRepo.Verify(m => m.GetSessionsOlderThanAsync(TenantA, It.IsAny<DateTime>()), Times.Never);
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────── PR6 follow-up F1 ─
|
||||
|
||||
[Fact]
|
||||
public async Task RunAsync_legacy_path_skips_locked_session_WITHOUT_touching_side_tables()
|
||||
{
|
||||
// PR6 follow-up F1: pre-read DeletionState before any side-table mutation. Without this
|
||||
// gate the legacy path would delete Events / RuleResults / AppSummaries and only discover
|
||||
// the V2 cascade lock at the tombstone CAS — leaving the V2 cascade's manifest claiming
|
||||
// rows that were silently removed in the meantime.
|
||||
var harness = new Harness();
|
||||
harness.WithTenant(TenantA, retentionDays: 30, v2: false, sessions: new[] { Old("s1", 60) });
|
||||
// Override the default fresh-read: this session is locked by a V2 cascade.
|
||||
harness.SessionRepo.Setup(r => r.GetSessionAsync(TenantA, "s1"))
|
||||
.ReturnsAsync(new SessionSummary
|
||||
{
|
||||
TenantId = TenantA,
|
||||
SessionId = "s1",
|
||||
DeletionState = SessionDeletionState.Queued,
|
||||
PendingDeletionManifestId = "EXISTING-MANIFEST",
|
||||
});
|
||||
|
||||
var result = await harness.Sut.RunAsync(CancellationToken.None);
|
||||
|
||||
Assert.Equal(1, result.SessionsSkipped);
|
||||
Assert.Equal(0, result.SessionsLegacyDeleted);
|
||||
// Critical assertion: NONE of the side-table delete helpers were invoked.
|
||||
harness.MaintenanceRepo.Verify(m => m.DeleteSessionEventsAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
|
||||
harness.MaintenanceRepo.Verify(m => m.DeleteSessionRuleResultsAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
|
||||
harness.MaintenanceRepo.Verify(m => m.DeleteSessionAppInstallSummariesAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
|
||||
harness.SessionRepo.Verify(r => r.DeleteSessionAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RunAsync_legacy_path_skips_when_session_already_removed_during_preread()
|
||||
{
|
||||
// Edge case: row disappeared between GetSessionsOlderThan and the pre-read. Treat as skip
|
||||
// (no side-table work needed); no error.
|
||||
var harness = new Harness();
|
||||
harness.WithTenant(TenantA, retentionDays: 30, v2: false, sessions: new[] { Old("s1", 60) });
|
||||
harness.SessionRepo.Setup(r => r.GetSessionAsync(TenantA, "s1")).ReturnsAsync((SessionSummary?)null);
|
||||
|
||||
var result = await harness.Sut.RunAsync(CancellationToken.None);
|
||||
|
||||
Assert.Equal(1, result.SessionsSkipped);
|
||||
harness.MaintenanceRepo.Verify(m => m.DeleteSessionEventsAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
|
||||
}
|
||||
|
||||
// ────────────────────────────────────────────────────────────────────────── PR6 follow-up F2 ─
|
||||
|
||||
[Fact]
|
||||
public async Task RunAsync_aborts_mid_loop_when_kill_switch_flips_on()
|
||||
{
|
||||
// PR6 follow-up F2: per-session kill-switch check halts the fanout immediately when the
|
||||
// emergency switch flips, instead of finishing the rest of this tenant's backlog.
|
||||
var harness = new Harness();
|
||||
var manySessions = new List<SessionSummary>();
|
||||
for (int i = 0; i < 5; i++) manySessions.Add(Summary(TenantA, $"s{i}"));
|
||||
harness.WithTenantOverride(TenantA, retentionDays: 30, v2: true, sessions: manySessions);
|
||||
|
||||
// First 2 calls return false (kill-switch off), then true → fanout aborts at the 3rd iteration.
|
||||
var calls = 0;
|
||||
harness.AdminConfig.Setup(a => a.IsSessionDeletionKillSwitchActiveAsync())
|
||||
.Returns(() =>
|
||||
{
|
||||
calls++;
|
||||
// Sequence: tenant-entry probe (call 1, false), then per-session probes:
|
||||
// session 0 (call 2, false), session 1 (call 3, false), session 2 (call 4, TRUE — abort).
|
||||
return Task.FromResult(calls >= 4);
|
||||
});
|
||||
|
||||
var result = await harness.Sut.RunAsync(CancellationToken.None);
|
||||
|
||||
Assert.True(result.AbortedByKillSwitch);
|
||||
Assert.Equal(2, result.SessionsEnqueued); // only sessions 0 and 1 made it through
|
||||
// Confirm only the first two enqueues fired.
|
||||
harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantA, "s0", It.IsAny<string>(), It.IsAny<DeletionActor>(), It.IsAny<DeletionRetentionContext?>(), It.IsAny<CancellationToken>()), Times.Once);
|
||||
harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantA, "s1", It.IsAny<string>(), It.IsAny<DeletionActor>(), It.IsAny<DeletionRetentionContext?>(), It.IsAny<CancellationToken>()), Times.Once);
|
||||
harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantA, "s2", It.IsAny<string>(), It.IsAny<DeletionActor>(), It.IsAny<DeletionRetentionContext?>(), It.IsAny<CancellationToken>()), Times.Never);
|
||||
harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantA, "s4", It.IsAny<string>(), It.IsAny<DeletionActor>(), It.IsAny<DeletionRetentionContext?>(), It.IsAny<CancellationToken>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task RunAsync_aborts_at_tenant_boundary_when_kill_switch_flips_between_tenants()
|
||||
{
|
||||
// Kill-switch flips on between tenant A and tenant B → tenant B is never started.
|
||||
var harness = new Harness();
|
||||
harness.WithTenant(TenantA, retentionDays: 30, v2: true, sessions: new[] { Old("a1", 60) });
|
||||
harness.WithTenant(TenantB, retentionDays: 30, v2: true, sessions: new[] { Old("b1", 60) });
|
||||
|
||||
var calls = 0;
|
||||
harness.AdminConfig.Setup(a => a.IsSessionDeletionKillSwitchActiveAsync())
|
||||
.Returns(() =>
|
||||
{
|
||||
calls++;
|
||||
// Tenant A: entry probe (call 1, false), session probe (call 2, false), enqueue runs.
|
||||
// Tenant B: entry probe (call 3, TRUE — abort).
|
||||
return Task.FromResult(calls >= 3);
|
||||
});
|
||||
|
||||
var result = await harness.Sut.RunAsync(CancellationToken.None);
|
||||
|
||||
Assert.True(result.AbortedByKillSwitch);
|
||||
Assert.Equal(1, result.TenantsProcessed);
|
||||
harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantA, "a1", It.IsAny<string>(), It.IsAny<DeletionActor>(), It.IsAny<DeletionRetentionContext?>(), It.IsAny<CancellationToken>()), Times.Once);
|
||||
harness.Enqueuer.Verify(e => e.EnqueueAsync(TenantB, It.IsAny<string>(), It.IsAny<string>(), It.IsAny<DeletionActor>(), It.IsAny<DeletionRetentionContext?>(), It.IsAny<CancellationToken>()), Times.Never);
|
||||
}
|
||||
|
||||
// ───────────────────────────────────────────────────────────────────────────────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task RunAsync_counts_AlreadyInFlight_outcome_as_skip()
|
||||
{
|
||||
@@ -180,6 +289,7 @@ public class SessionRetentionFanoutServiceTests
|
||||
public Mock<ISessionRepository> SessionRepo { get; }
|
||||
public Mock<TenantConfigurationService> TenantConfig { get; }
|
||||
public Mock<ISessionDeletionEnqueuer> Enqueuer { get; }
|
||||
public Mock<AdminConfigurationService> AdminConfig { get; }
|
||||
public SessionRetentionFanoutService Sut { get; }
|
||||
|
||||
private readonly List<string> _tenantIds = new();
|
||||
@@ -193,6 +303,17 @@ public class SessionRetentionFanoutServiceTests
|
||||
Mock.Of<IConfigRepository>(), NullLogger<TenantConfigurationService>.Instance, memCache);
|
||||
Enqueuer = new Mock<ISessionDeletionEnqueuer>();
|
||||
|
||||
// PR6 follow-up F1: legacy path now pre-reads the Sessions row via ISessionRepository
|
||||
// to check DeletionState. Default: row exists with DeletionState=None so legacy proceeds.
|
||||
SessionRepo.Setup(r => r.GetSessionAsync(It.IsAny<string>(), It.IsAny<string>()))
|
||||
.Returns<string, string>((tenant, sid) => Task.FromResult<SessionSummary?>(
|
||||
new SessionSummary { TenantId = tenant, SessionId = sid, DeletionState = SessionDeletionState.None }));
|
||||
|
||||
// PR6 follow-up F2: per-session kill-switch check inside the fanout loop.
|
||||
AdminConfig = new Mock<AdminConfigurationService>(
|
||||
Mock.Of<IConfigRepository>(), NullLogger<AdminConfigurationService>.Instance, memCache);
|
||||
AdminConfig.Setup(a => a.IsSessionDeletionKillSwitchActiveAsync()).ReturnsAsync(false);
|
||||
|
||||
// Default: every enqueue succeeds.
|
||||
Enqueuer.Setup(e => e.EnqueueAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(),
|
||||
It.IsAny<DeletionActor>(), It.IsAny<DeletionRetentionContext?>(), It.IsAny<CancellationToken>()))
|
||||
@@ -213,6 +334,7 @@ public class SessionRetentionFanoutServiceTests
|
||||
// (50ms × 100 = 5s otherwise).
|
||||
Sut = new SessionRetentionFanoutService(
|
||||
MaintenanceRepo.Object, SessionRepo.Object, TenantConfig.Object, Enqueuer.Object,
|
||||
AdminConfig.Object,
|
||||
NullLogger<SessionRetentionFanoutService>.Instance,
|
||||
throttle: (_, _) => Task.CompletedTask);
|
||||
}
|
||||
|
||||
+26
-48
@@ -146,24 +146,16 @@ namespace AutopilotMonitor.Functions.Functions.Maintenance
|
||||
// (3) Stranded-Queued detection (alert-only) — runs even when kill-switch=true.
|
||||
var strandedQueuedDetected = await DetectStrandedQueuedAsync(cancellationToken);
|
||||
|
||||
// (4) Retention fanout — skipped when kill-switch=true. Audit the skip so the
|
||||
// operator sees in MCP/admin logs that the fanout was deliberately bypassed.
|
||||
// (4) Retention fanout — skipped when kill-switch=true. Emit a Fanout-Skipped
|
||||
// OpsEvent so the operator can fold the skip into the OpsEvents dashboard alongside
|
||||
// the 30/60min watchdogs. PR6 follow-up F3: previously a LogAuditEntryAsync(null!)
|
||||
// call that silently failed because the AuditLogs schema requires a non-null
|
||||
// PartitionKey — OpsEvents tolerates null TenantId, so global-scope events live there.
|
||||
SessionRetentionFanoutService.FanoutResult? fanoutResult = null;
|
||||
if (killSwitchActive)
|
||||
{
|
||||
await _maintenanceRepo.LogAuditEntryAsync(
|
||||
tenantId: null!,
|
||||
action: "session_deletion_maintenance_fanout_skipped_killswitch",
|
||||
entityType: "Maintenance",
|
||||
entityId: "fanout",
|
||||
performedBy: "System.Maintenance",
|
||||
details: new Dictionary<string, string>
|
||||
{
|
||||
{ "Reason", "SessionDeletionKillSwitch is active" },
|
||||
{ "BlobsTtlGced", blobsTtlGced.ToString() },
|
||||
{ "PreparingRowsCleared", preparingRowsCleared.ToString() },
|
||||
{ "StrandedQueuedDetected", strandedQueuedDetected.ToString() },
|
||||
});
|
||||
await _opsEvents.RecordSessionDeletionMaintenanceFanoutSkippedAsync(
|
||||
blobsTtlGced, preparingRowsCleared, strandedQueuedDetected);
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -172,26 +164,20 @@ namespace AutopilotMonitor.Functions.Functions.Maintenance
|
||||
sessionsEnqueuedSoFar = fanoutResult.SessionsEnqueued + fanoutResult.SessionsLegacyDeleted;
|
||||
}
|
||||
|
||||
// (5) Completion audit — same shape regardless of kill-switch state, so dashboards can fold both paths together.
|
||||
await _maintenanceRepo.LogAuditEntryAsync(
|
||||
tenantId: null!,
|
||||
action: "session_deletion_maintenance_completed",
|
||||
entityType: "Maintenance",
|
||||
entityId: $"run-{DateTime.UtcNow:yyyyMMddHHmmss}",
|
||||
performedBy: "System.Maintenance",
|
||||
details: new Dictionary<string, string>
|
||||
{
|
||||
{ "KillSwitchActive", killSwitchActive.ToString() },
|
||||
{ "TenantsProcessed", (fanoutResult?.TenantsProcessed ?? 0).ToString() },
|
||||
{ "SessionsEnqueued", (fanoutResult?.SessionsEnqueued ?? 0).ToString() },
|
||||
{ "SessionsLegacyDeleted", (fanoutResult?.SessionsLegacyDeleted ?? 0).ToString() },
|
||||
{ "SessionsSkipped", (fanoutResult?.SessionsSkipped ?? 0).ToString() },
|
||||
{ "RateLimitedTenants", (fanoutResult?.RateLimitedTenants ?? 0).ToString() },
|
||||
{ "BlobsTtlGced", blobsTtlGced.ToString() },
|
||||
{ "PreparingRowsCleared", preparingRowsCleared.ToString() },
|
||||
{ "StrandedQueuedDetected", strandedQueuedDetected.ToString() },
|
||||
{ "DurationMs", sw.ElapsedMilliseconds.ToString() },
|
||||
});
|
||||
// (5) Completion OpsEvent — same shape regardless of kill-switch state, so dashboards
|
||||
// can fold both paths together. PR6 follow-up F3.
|
||||
await _opsEvents.RecordSessionDeletionMaintenanceCompletedAsync(
|
||||
killSwitchActive: killSwitchActive,
|
||||
tenantsProcessed: fanoutResult?.TenantsProcessed ?? 0,
|
||||
sessionsEnqueued: fanoutResult?.SessionsEnqueued ?? 0,
|
||||
sessionsLegacyDeleted: fanoutResult?.SessionsLegacyDeleted ?? 0,
|
||||
sessionsSkipped: fanoutResult?.SessionsSkipped ?? 0,
|
||||
rateLimitedTenants: fanoutResult?.RateLimitedTenants ?? 0,
|
||||
blobsTtlGced: blobsTtlGced,
|
||||
preparingRowsCleared: preparingRowsCleared,
|
||||
strandedQueuedDetected: strandedQueuedDetected,
|
||||
durationMs: (int)sw.ElapsedMilliseconds,
|
||||
abortedByKillSwitch: fanoutResult?.AbortedByKillSwitch ?? false);
|
||||
|
||||
_logger.LogInformation(
|
||||
"SessionDeletionMaintenance: completed in {Ms}ms — killSwitch={KillSwitch} tenants={Tenants} enqueued={Enqueued} legacy={Legacy} skipped={Skipped} blobs={Blobs} preparing={Preparing} stranded={Stranded}",
|
||||
@@ -204,21 +190,13 @@ namespace AutopilotMonitor.Functions.Functions.Maintenance
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
// Failure path (Plan §5 PR6): OpsEvent → audit → re-throw so the runtime records it.
|
||||
// Failure path (Plan §5 PR6) — emit the SessionDeletionMaintenanceFailed OpsEvent
|
||||
// (captures exceptionType + message + stack preview as event details) and re-throw
|
||||
// so the Azure Functions runtime records the failure. PR6 follow-up F3: the prior
|
||||
// parallel LogAuditEntryAsync(null!) call was a silent no-op because AuditLogs
|
||||
// requires a non-null PartitionKey; the OpsEvent IS the authoritative audit record.
|
||||
var stackPreview = ex.StackTrace is { Length: > 2048 } s ? s.Substring(0, 2048) : ex.StackTrace ?? string.Empty;
|
||||
await _opsEvents.RecordSessionDeletionMaintenanceFailedAsync(ex.GetType().FullName ?? ex.GetType().Name, ex.Message, stackPreview);
|
||||
await _maintenanceRepo.LogAuditEntryAsync(
|
||||
tenantId: null!,
|
||||
action: "session_deletion_maintenance_failed",
|
||||
entityType: "Maintenance",
|
||||
entityId: $"run-{DateTime.UtcNow:yyyyMMddHHmmss}",
|
||||
performedBy: "System.Maintenance",
|
||||
details: new Dictionary<string, string>
|
||||
{
|
||||
{ "ExceptionType", ex.GetType().FullName ?? ex.GetType().Name },
|
||||
{ "Message", ex.Message },
|
||||
{ "DurationMs", sw.ElapsedMilliseconds.ToString() },
|
||||
});
|
||||
throw;
|
||||
}
|
||||
finally
|
||||
|
||||
+68
-10
@@ -40,6 +40,7 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
private readonly ISessionRepository _sessionRepo;
|
||||
private readonly TenantConfigurationService _tenantConfig;
|
||||
private readonly ISessionDeletionEnqueuer _enqueuer;
|
||||
private readonly AdminConfigurationService _adminConfig;
|
||||
private readonly ILogger<SessionRetentionFanoutService> _logger;
|
||||
private readonly Func<TimeSpan, CancellationToken, Task> _throttle;
|
||||
|
||||
@@ -48,8 +49,9 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
ISessionRepository sessionRepo,
|
||||
TenantConfigurationService tenantConfig,
|
||||
ISessionDeletionEnqueuer enqueuer,
|
||||
AdminConfigurationService adminConfig,
|
||||
ILogger<SessionRetentionFanoutService> logger)
|
||||
: this(maintenanceRepo, sessionRepo, tenantConfig, enqueuer, logger, throttle: Task.Delay)
|
||||
: this(maintenanceRepo, sessionRepo, tenantConfig, enqueuer, adminConfig, logger, throttle: Task.Delay)
|
||||
{
|
||||
}
|
||||
|
||||
@@ -62,6 +64,7 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
ISessionRepository sessionRepo,
|
||||
TenantConfigurationService tenantConfig,
|
||||
ISessionDeletionEnqueuer enqueuer,
|
||||
AdminConfigurationService adminConfig,
|
||||
ILogger<SessionRetentionFanoutService> logger,
|
||||
Func<TimeSpan, CancellationToken, Task> throttle)
|
||||
{
|
||||
@@ -69,6 +72,7 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
_sessionRepo = sessionRepo;
|
||||
_tenantConfig = tenantConfig;
|
||||
_enqueuer = enqueuer;
|
||||
_adminConfig = adminConfig;
|
||||
_logger = logger;
|
||||
_throttle = throttle;
|
||||
}
|
||||
@@ -84,6 +88,7 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
public int SessionsLegacyDeleted { get; set; } // flag-OFF path: direct-delete succeeded
|
||||
public int SessionsSkipped { get; set; } // already locked / poisoned / kill-switch / etc.
|
||||
public int RateLimitedTenants { get; set; } // tenants that hit MaxEnqueuesPerTenantPerRun
|
||||
public bool AbortedByKillSwitch { get; set; } // PR6 follow-up F2: kill-switch flipped mid-run
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -100,6 +105,18 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
foreach (var tenantId in tenantIds)
|
||||
{
|
||||
cancellationToken.ThrowIfCancellationRequested();
|
||||
|
||||
// PR6 follow-up F2: per-tenant kill-switch check so a flip-ON mid-run halts the
|
||||
// remaining tenants. The maintenance function gates entry; this gates iteration.
|
||||
if (await _adminConfig.IsSessionDeletionKillSwitchActiveAsync().ConfigureAwait(false))
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"SessionRetentionFanout: kill-switch flipped on mid-run — halting before tenant {TenantId}",
|
||||
tenantId);
|
||||
result.AbortedByKillSwitch = true;
|
||||
break;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
await RunForTenantAsync(tenantId, result, cancellationToken).ConfigureAwait(false);
|
||||
@@ -158,6 +175,18 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
break;
|
||||
}
|
||||
|
||||
// PR6 follow-up F2: per-session kill-switch check so an emergency flip-ON halts
|
||||
// immediately instead of after the rest of this tenant's backlog. Uncached read
|
||||
// (PR5 F1) — uniform behaviour across scaled-out instances within seconds.
|
||||
if (await _adminConfig.IsSessionDeletionKillSwitchActiveAsync().ConfigureAwait(false))
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"SessionRetentionFanout: kill-switch flipped on mid-tenant — halting at session {SessionId} of {TenantId}",
|
||||
session.SessionId, tenantId);
|
||||
result.AbortedByKillSwitch = true;
|
||||
break;
|
||||
}
|
||||
|
||||
if (useV2Cascade)
|
||||
{
|
||||
var outcome = await EnqueueV2Async(session, cancellationToken).ConfigureAwait(false);
|
||||
@@ -198,6 +227,7 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
{ "Enqueued", enqueued.ToString() },
|
||||
{ "LegacyDeleted", legacyDeleted.ToString() },
|
||||
{ "Skipped", skipped.ToString() },
|
||||
{ "AbortedByKillSwitch", result.AbortedByKillSwitch.ToString() },
|
||||
}).ConfigureAwait(false);
|
||||
|
||||
_logger.LogInformation(
|
||||
@@ -257,11 +287,19 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
|
||||
/// <summary>
|
||||
/// Legacy direct-delete path — invoked when the tenant has not yet been migrated to the V2
|
||||
/// cascade (per-tenant flag OFF). Reuses the exact sequence
|
||||
/// <c>MaintenanceService.CleanupOldDataAsync</c> ran before PR6 extracted it: delete
|
||||
/// events / rule-results / app-summaries, then delete the Sessions row itself. PR5
|
||||
/// finding 2 added a CAS guard into <c>DeleteSessionAsync</c>, so a V2 cascade locked by
|
||||
/// a parallel admin click during this run will refuse to be tombstoned here.
|
||||
/// cascade (per-tenant flag OFF). Sequence:
|
||||
/// <list type="number">
|
||||
/// <item>Pre-read the Sessions row (PR6 follow-up F1). If <c>DeletionState != None</c>
|
||||
/// the V2 cascade owns the session — return false without touching ANY side table.
|
||||
/// Without this gate, we would delete Events / RuleResults / AppInstallSummaries
|
||||
/// and only discover the lock at the tombstone CAS, leaving the V2 cascade with a
|
||||
/// half-eaten session.</item>
|
||||
/// <item>Delete events / rule-results / app-summaries.</item>
|
||||
/// <item>Delete the Sessions row via the CAS-guarded
|
||||
/// <see cref="ISessionRepository.DeleteSessionAsync"/> (PR5 finding 2) — second-line
|
||||
/// protection against the T1-T3 race where another instance CAS-claimed the row
|
||||
/// after our pre-read but before our side-table mutations.</item>
|
||||
/// </list>
|
||||
/// </summary>
|
||||
private async Task<bool> LegacyDeleteAsync(SessionSummary session, CancellationToken cancellationToken)
|
||||
{
|
||||
@@ -269,18 +307,38 @@ namespace AutopilotMonitor.Functions.Services.Deletion
|
||||
var tenantId = session.TenantId;
|
||||
var sessionId = session.SessionId;
|
||||
|
||||
// F1: gate side-table mutations on DeletionState. The CAS guard inside DeleteSessionAsync
|
||||
// is the storage-atomic backstop (PR5 finding 2), but it fires AFTER the side-table
|
||||
// deletes. By the time it returns false we've already orphaned events from the V2
|
||||
// cascade's planned manifest. Reading the Sessions row up-front is cheap (one Get) and
|
||||
// skips ~3 side-table deletes when the lock is held.
|
||||
var refreshed = await _sessionRepo.GetSessionAsync(tenantId, sessionId).ConfigureAwait(false);
|
||||
if (refreshed == null)
|
||||
{
|
||||
// Row already gone — nothing to clean up. Treat as skip (no events were ours to delete either).
|
||||
return false;
|
||||
}
|
||||
if (SessionDeletionState.IsLocked(refreshed.DeletionState))
|
||||
{
|
||||
_logger.LogInformation(
|
||||
"Retention fanout (legacy): session {SessionId} is locked by a V2 cascade (state={State}, manifestId={ManifestId}) — skipping ALL deletes, V2 worker owns the cascade",
|
||||
sessionId, refreshed.DeletionState, refreshed.PendingDeletionManifestId);
|
||||
return false;
|
||||
}
|
||||
|
||||
_ = await _maintenanceRepo.DeleteSessionEventsAsync(tenantId, sessionId).ConfigureAwait(false);
|
||||
_ = await _maintenanceRepo.DeleteSessionRuleResultsAsync(tenantId, sessionId).ConfigureAwait(false);
|
||||
_ = await _maintenanceRepo.DeleteSessionAppInstallSummariesAsync(tenantId, sessionId).ConfigureAwait(false);
|
||||
|
||||
// CAS guard inside DeleteSessionAsync (PR5 finding 2) refuses if DeletionState is locked.
|
||||
// A `false` here means either 404 (already gone) is treated as success internally, OR
|
||||
// a concurrent V2 cascade claimed the row. The latter is a skip, not a failure.
|
||||
// CAS guard inside DeleteSessionAsync (PR5 finding 2) refuses if DeletionState is locked
|
||||
// — backstop for the T1-T3 race after our pre-read above. A `false` here means either
|
||||
// 404 (already gone) is treated as success internally, OR a concurrent V2 cascade
|
||||
// claimed the row in the brief window between our pre-read and the tombstone.
|
||||
var deleted = await _sessionRepo.DeleteSessionAsync(tenantId, sessionId).ConfigureAwait(false);
|
||||
if (!deleted)
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"Retention fanout (legacy): session {SessionId} could not be tombstoned — likely claimed by a concurrent V2 cascade. Will retry next run",
|
||||
"Retention fanout (legacy): session {SessionId} could not be tombstoned — likely claimed by a concurrent V2 cascade. Side tables already deleted; cascade is idempotent so V2 will absorb the difference. Will retry next run",
|
||||
sessionId);
|
||||
}
|
||||
return deleted;
|
||||
|
||||
@@ -98,6 +98,36 @@ namespace AutopilotMonitor.Functions.Services
|
||||
$"Session {sessionId} stuck in DeletionState=Queued since {queuedSince:o} (manifestId={manifestId})",
|
||||
tenantId, "System.Maintenance", new { tenantId, sessionId, queuedSince = queuedSince.ToString("o"), manifestId });
|
||||
|
||||
/// <summary>
|
||||
/// Successful end of a <see cref="Functions.Maintenance.SessionDeletionMaintenanceFunction"/>
|
||||
/// run — records the per-block totals so dashboards can fold the cadence into the timeline.
|
||||
/// PR6 follow-up F3: replaces the prior <c>LogAuditEntryAsync(null!, ...)</c> call, which
|
||||
/// silently failed because the AuditLogs schema requires a non-null PartitionKey (tenantId).
|
||||
/// </summary>
|
||||
public Task RecordSessionDeletionMaintenanceCompletedAsync(
|
||||
bool killSwitchActive, int tenantsProcessed, int sessionsEnqueued, int sessionsLegacyDeleted,
|
||||
int sessionsSkipped, int rateLimitedTenants, int blobsTtlGced, int preparingRowsCleared,
|
||||
int strandedQueuedDetected, int durationMs, bool abortedByKillSwitch)
|
||||
=> WriteAsync(OpsEventCategory.Maintenance, "SessionDeletionMaintenanceCompleted", OpsEventSeverity.Info,
|
||||
$"SessionDeletionMaintenance completed in {durationMs}ms — tenants={tenantsProcessed} enqueued={sessionsEnqueued} legacy={sessionsLegacyDeleted} skipped={sessionsSkipped} blobsTtlGced={blobsTtlGced} preparingCleared={preparingRowsCleared} stranded={strandedQueuedDetected} killSwitch={killSwitchActive}",
|
||||
null, "System.Maintenance", new {
|
||||
killSwitchActive, tenantsProcessed, sessionsEnqueued, sessionsLegacyDeleted,
|
||||
sessionsSkipped, rateLimitedTenants, blobsTtlGced, preparingRowsCleared,
|
||||
strandedQueuedDetected, durationMs, abortedByKillSwitch,
|
||||
});
|
||||
|
||||
/// <summary>
|
||||
/// Records that the retention fanout half of a <see cref="Functions.Maintenance.SessionDeletionMaintenanceFunction"/>
|
||||
/// run was skipped because the global kill-switch was active at entry. The three GCs
|
||||
/// (manifest TTL sweep, stale-Preparing, stranded-Queued) still ran — see the totals on
|
||||
/// the paired <see cref="RecordSessionDeletionMaintenanceCompletedAsync"/> event.
|
||||
/// </summary>
|
||||
public Task RecordSessionDeletionMaintenanceFanoutSkippedAsync(
|
||||
int blobsTtlGced, int preparingRowsCleared, int strandedQueuedDetected)
|
||||
=> WriteAsync(OpsEventCategory.Maintenance, "SessionDeletionMaintenanceFanoutSkipped", OpsEventSeverity.Info,
|
||||
$"SessionDeletionMaintenance fanout skipped (kill-switch active) — GCs ran: blobsTtlGced={blobsTtlGced} preparingCleared={preparingRowsCleared} stranded={strandedQueuedDetected}",
|
||||
null, "System.Maintenance", new { reason = "SessionDeletionKillSwitch", blobsTtlGced, preparingRowsCleared, strandedQueuedDetected });
|
||||
|
||||
// ── Security ───────────────────────────────────────────────────────────
|
||||
|
||||
public Task RecordDeviceBlockedAsync(string tenantId, string serialNumber, string reason, string blockedBy)
|
||||
|
||||
@@ -13,10 +13,16 @@ const OPS_EVENT_TYPES: Record<string, string[]> = {
|
||||
"SessionTimeouts",
|
||||
// Cascade-delete maintenance (PR6) — dual-register per memory feedback_ops_event_types_dual_register.
|
||||
// Dispatched by SessionDeletionMaintenanceFunction; backend helpers in OpsEventService.cs are
|
||||
// RecordSessionDeletionMaintenance{LongRunning,LongRunningSevere,Failed}Async + RecordSessionDeletionStrandedQueuedAsync.
|
||||
// RecordSessionDeletionMaintenance{LongRunning,LongRunningSevere,Failed,Completed,FanoutSkipped}Async +
|
||||
// RecordSessionDeletionStrandedQueuedAsync. The Completed + FanoutSkipped events landed with the
|
||||
// PR6 follow-up (F3) — they replaced the AuditLogs-based lifecycle audits that silently failed
|
||||
// because the schema requires a non-null PartitionKey (tenantId), but the maintenance lifecycle
|
||||
// events are global-scope.
|
||||
"SessionDeletionMaintenanceCompleted",
|
||||
"SessionDeletionMaintenanceLongRunning",
|
||||
"SessionDeletionMaintenanceLongRunningSevere",
|
||||
"SessionDeletionMaintenanceFailed",
|
||||
"SessionDeletionMaintenanceFanoutSkipped",
|
||||
"SessionDeletionStrandedQueued",
|
||||
],
|
||||
Security: [
|
||||
|
||||
Reference in New Issue
Block a user