diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Backup/CriticalTableBackupTimerFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Backup/CriticalTableBackupTimerFunction.cs index 88462779..86f15908 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Backup/CriticalTableBackupTimerFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Backup/CriticalTableBackupTimerFunction.cs @@ -48,6 +48,8 @@ namespace AutopilotMonitor.Functions.Functions.Backup string? backupId = null; // ── Acquire maintenance lease ─────────────────────────────────────── + // Codex-Hotfix #1: the lease must be renewed during the 50-min per-run budget; + // otherwise the lease auto-expires after 60 s and another concurrent op could enter. await _blobStore.EnsureMaintenanceLockSentinelAsync(ct).ConfigureAwait(false); Azure.Storage.Blobs.Specialized.BlobLeaseClient? lease; @@ -63,13 +65,25 @@ namespace AutopilotMonitor.Functions.Functions.Backup return; } + using var handlerCts = CancellationTokenSource.CreateLinkedTokenSource(ct); + var leaseHolder = new MaintenanceLeaseHolder(lease, handlerCts, _logger); + try { + var hct = handlerCts.Token; backupId = _backupService.GenerateBackupId(); BackupRunResult? result; try { - result = await _backupService.RunBackupUnderLeaseAsync(backupId, triggeredBy, ct).ConfigureAwait(false); + result = await _backupService.RunBackupUnderLeaseAsync(backupId, triggeredBy, hct).ConfigureAwait(false); + } + catch (OperationCanceledException) when (handlerCts.IsCancellationRequested && leaseHolder.RenewalFailureReason is not null) + { + _logger.LogError("CriticalTableBackupTimer: maintenance lease lost mid-run (backupId={BackupId}) — {Reason}", + backupId, leaseHolder.RenewalFailureReason); + await TryRecordAsync(() => _opsEvents.RecordCriticalTableBackupFailedAsync( + backupId, leaseHolder.RenewalFailureReason!, triggeredBy)).ConfigureAwait(false); + return; } catch (Exception ex) when (ex is not OperationCanceledException) { @@ -100,8 +114,7 @@ namespace AutopilotMonitor.Functions.Functions.Backup } finally { - try { await lease.ReleaseAsync(cancellationToken: ct).ConfigureAwait(false); } - catch (Exception ex) { _logger.LogWarning(ex, "CriticalTableBackupTimer: lease release failed (auto-expires)"); } + await leaseHolder.DisposeAsync().ConfigureAwait(false); } } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Backup/BlobBackupStore.cs b/src/Backend/AutopilotMonitor.Functions/Services/Backup/BlobBackupStore.cs index 8af4bde6..2fd57dbd 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/Backup/BlobBackupStore.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/Backup/BlobBackupStore.cs @@ -174,17 +174,53 @@ namespace AutopilotMonitor.Functions.Services.Backup } /// - /// Lists all backupIds present in the container (one entry per distinct first-level - /// prefix), ignoring the _lock/ prefix. Whether the manifest is actually - /// present is up to the caller — this method returns ids of all {id}/ - /// prefixes regardless. + /// Lists every backupId whose manifest.json exists — i.e. every run that + /// reached the durability anchor (plan §3 "manifest is the backup"). Orphan + /// prefixes (NDJSON written but manifest never followed) are deliberately + /// excluded so callers cannot accidentally offer an incomplete run for restore; + /// use when an admin UI needs to show + /// incomplete runs as a distinct status (Codex-Hotfix #3). /// public virtual async IAsyncEnumerable ListBackupIdsAsync( [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken ct = default) { await EnsureContainerAsync(ct).ConfigureAwait(false); var container = GetContainer(); - // GetBlobsByHierarchyAsync with delimiter="/" returns the top-level prefixes. + await foreach (var prefix in EnumerateTopLevelPrefixesAsync(container, ct).ConfigureAwait(false)) + { + // Cheap Exists() on the manifest blob — filters out runs that crashed + // before manifest.json was committed. + var manifest = container.GetBlobClient(BuildManifestBlobName(prefix)); + if (await manifest.ExistsAsync(ct).ConfigureAwait(false)) + { + yield return prefix; + } + } + } + + /// + /// Lists every top-level prefix in the container — completed runs (manifest present) + /// AND incomplete ones (manifest missing). Returns a flag so a future admin UI can + /// surface incomplete runs as their own status. PR1 callers should prefer the + /// completed-only . + /// + public virtual async IAsyncEnumerable<(string BackupId, bool HasManifest)> ListAllBackupPrefixesAsync( + [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken ct = default) + { + await EnsureContainerAsync(ct).ConfigureAwait(false); + var container = GetContainer(); + await foreach (var prefix in EnumerateTopLevelPrefixesAsync(container, ct).ConfigureAwait(false)) + { + var manifest = container.GetBlobClient(BuildManifestBlobName(prefix)); + var exists = await manifest.ExistsAsync(ct).ConfigureAwait(false); + yield return (prefix, exists.Value); + } + } + + private static async IAsyncEnumerable EnumerateTopLevelPrefixesAsync( + BlobContainerClient container, + [System.Runtime.CompilerServices.EnumeratorCancellation] CancellationToken ct) + { var seen = new HashSet(StringComparer.Ordinal); await foreach (var item in container.GetBlobsByHierarchyAsync(traits: BlobTraits.None, states: BlobStates.None, prefix: null, delimiter: "/", cancellationToken: ct).ConfigureAwait(false)) { diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Backup/MaintenanceLeaseHolder.cs b/src/Backend/AutopilotMonitor.Functions/Services/Backup/MaintenanceLeaseHolder.cs new file mode 100644 index 00000000..e23b5383 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions/Services/Backup/MaintenanceLeaseHolder.cs @@ -0,0 +1,113 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using Azure; +using Azure.Storage.Blobs.Specialized; +using Microsoft.Extensions.Logging; + +namespace AutopilotMonitor.Functions.Services.Backup +{ + /// + /// Owns a maintenance-lease for the lifetime of a backup or restore run and + /// renews it in the background. Without the renewal loop the 60-second lease + /// duration would expire long before the per-run budget (50 minutes), letting + /// a parallel worker / timer / restore acquire the same "exclusive" lease and + /// letting the watchdog see a free lease while a job is still running + /// (Codex-Hotfix Wave1). + /// + /// Renewal cadence is fixed at (45 s, well under + /// the 60-second lease). On any renewal failure the supplied + /// is cancelled so the handler bails out + /// promptly instead of running rogue under an expired lease. + /// + /// + /// Disposal stops the renewal loop deterministically (CancellationTokenSource + /// cancel + await the loop task) BEFORE releasing the lease, so a final + /// in-flight renewal cannot race the release. + /// + /// + public sealed class MaintenanceLeaseHolder : IAsyncDisposable + { + /// Sub-60s renewal cadence — leaves headroom for transient retries. + public static readonly TimeSpan RenewalInterval = TimeSpan.FromSeconds(45); + + private readonly BlobLeaseClient _lease; + private readonly CancellationTokenSource _renewalCts; + private readonly CancellationTokenSource _handlerCts; + private readonly Task _renewalLoop; + private readonly ILogger _logger; + private int _disposed; + + public MaintenanceLeaseHolder(BlobLeaseClient lease, CancellationTokenSource handlerCts, ILogger logger) + { + _lease = lease ?? throw new ArgumentNullException(nameof(lease)); + _handlerCts = handlerCts ?? throw new ArgumentNullException(nameof(handlerCts)); + _logger = logger ?? throw new ArgumentNullException(nameof(logger)); + _renewalCts = new CancellationTokenSource(); + _renewalLoop = Task.Run(() => RunRenewalLoopAsync(_renewalCts.Token)); + } + + /// + /// Sentinel value set if the renewal loop terminated because a renewal call + /// threw — the handler should treat this as a maintenance-lease loss and + /// bail out without writing the manifest or queue-deleting. + /// + public string? RenewalFailureReason { get; private set; } + + private async Task RunRenewalLoopAsync(CancellationToken ct) + { + while (!ct.IsCancellationRequested) + { + try + { + await Task.Delay(RenewalInterval, ct).ConfigureAwait(false); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + return; // normal shutdown + } + + try + { + await _lease.RenewAsync(conditions: null, ct).ConfigureAwait(false); + } + catch (OperationCanceledException) when (ct.IsCancellationRequested) + { + return; // shutdown while renewing + } + catch (RequestFailedException ex) + { + RenewalFailureReason = $"maintenance lease renewal failed (status={ex.Status}, code={ex.ErrorCode}): {ex.Message}"; + _logger.LogError(ex, "MaintenanceLeaseHolder: lease renewal failed — cancelling handler"); + try { _handlerCts.Cancel(); } catch (ObjectDisposedException) { } + return; + } + catch (Exception ex) + { + RenewalFailureReason = $"maintenance lease renewal threw: {ex.Message}"; + _logger.LogError(ex, "MaintenanceLeaseHolder: lease renewal unexpected throw — cancelling handler"); + try { _handlerCts.Cancel(); } catch (ObjectDisposedException) { } + return; + } + } + } + + /// + /// Stops the renewal loop and releases the lease. Renewal-loop cancel + + /// await happens BEFORE the release so a final renew cannot race a release. + /// Safe to call multiple times. + /// + public async ValueTask DisposeAsync() + { + if (Interlocked.Exchange(ref _disposed, 1) != 0) return; + + try { _renewalCts.Cancel(); } catch (ObjectDisposedException) { } + try { await _renewalLoop.ConfigureAwait(false); } + catch (Exception ex) { _logger.LogWarning(ex, "MaintenanceLeaseHolder: renewal loop terminated with exception"); } + _renewalCts.Dispose(); + + try { await _lease.ReleaseAsync().ConfigureAwait(false); } + catch (Exception ex) { _logger.LogWarning(ex, "MaintenanceLeaseHolder: lease release failed (auto-expires within 60 s)"); } + } + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Services/Backup/Queue/CriticalTableBackupQueueWorker.cs b/src/Backend/AutopilotMonitor.Functions/Services/Backup/Queue/CriticalTableBackupQueueWorker.cs index 28aa7c96..bef8703b 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/Backup/Queue/CriticalTableBackupQueueWorker.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/Backup/Queue/CriticalTableBackupQueueWorker.cs @@ -216,6 +216,11 @@ namespace AutopilotMonitor.Functions.Services.Backup.Queue } // ── Acquire maintenance lease BEFORE CAS Running ──────────────────── + // Codex-Hotfix #1: the lease is 60 s and the per-run budget is 50 min — without a + // renewal loop a parallel worker / timer would walk into the same supposedly-exclusive + // section after 60 s, and the watchdog would see a free lease while we're still + // running. MaintenanceLeaseHolder spins a renewal task on a sub-60s cadence and + // cancels handlerCts on any renewal failure. BlobLeaseClient? lease = null; try { @@ -237,19 +242,30 @@ namespace AutopilotMonitor.Functions.Services.Backup.Queue return; } + // Chain a per-handler CTS off the worker's stoppingToken so the renewal loop can + // cancel the in-flight backup if the lease is lost mid-run. + using var handlerCts = CancellationTokenSource.CreateLinkedTokenSource(ct); + MaintenanceLeaseHolder leaseHolder = new MaintenanceLeaseHolder(lease, handlerCts, _logger); + + BackupRunResult? result = null; + Exception? retryableException = null; + bool leaseLost = false; + try { + var hct = handlerCts.Token; + // ── CAS Queued→Running + heartbeat (plan Wave14 #1) ───────────── var nowUtc = _clock.GetUtcNow().UtcDateTime; var runningJob = job; runningJob.State = BackupJobState.Running; runningJob.StartedAtUtc = nowUtc; runningJob.LastHeartbeatUtc = nowUtc; - var casOk = await _jobs.TryUpdateWithCasAsync(runningJob, jobETag!.Value, ct).ConfigureAwait(false); + var casOk = await _jobs.TryUpdateWithCasAsync(runningJob, jobETag!.Value, hct).ConfigureAwait(false); if (!casOk) { // Concurrent state change (watchdog, parallel worker). Re-read + dispatch. - var (fresh, _) = await _jobs.GetWithETagAsync(jobId, ct).ConfigureAwait(false); + var (fresh, _) = await _jobs.GetWithETagAsync(jobId, hct).ConfigureAwait(false); if (fresh is not null && IsTerminal(fresh.State)) { await SafeDeleteAsync(msg, ct).ConfigureAwait(false); @@ -262,27 +278,32 @@ namespace AutopilotMonitor.Functions.Services.Backup.Queue } // ── Service-call: backup itself (Wave22 #3 — service is lease-unaware) ── - BackupRunResult? result = null; - Exception? retryableException = null; try { var backupId = _backupService.GenerateBackupId(); // Stamp BackupId via CAS so UI can deep-link to the manifest the moment the worker has chosen the id. - var (currentJob, currentETag) = await _jobs.GetWithETagAsync(jobId, ct).ConfigureAwait(false); + var (currentJob, currentETag) = await _jobs.GetWithETagAsync(jobId, hct).ConfigureAwait(false); if (currentJob is not null && currentETag is not null) { currentJob.BackupId = backupId; currentJob.LastHeartbeatUtc = _clock.GetUtcNow().UtcDateTime; - await _jobs.TryUpdateWithCasAsync(currentJob, currentETag.Value, ct).ConfigureAwait(false); + await _jobs.TryUpdateWithCasAsync(currentJob, currentETag.Value, hct).ConfigureAwait(false); } - result = await _backupService.RunBackupUnderLeaseAsync(backupId, runningJob.RequestedBy, ct).ConfigureAwait(false); + result = await _backupService.RunBackupUnderLeaseAsync(backupId, runningJob.RequestedBy, hct).ConfigureAwait(false); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { throw; } + catch (OperationCanceledException) when (handlerCts.IsCancellationRequested && leaseHolder.RenewalFailureReason is not null) + { + // Lease renewal lost mid-run (Codex-Hotfix #1). Treat as a non-retryable + // "previous worker died" — JobState=Failed, no delete (let the message + // reappear so a clean run can pick it up after the lease auto-expires). + leaseLost = true; + } catch (Exception ex) { // Retryable: storage 5xx / network / generic bug. Roll JobState back to Queued @@ -292,14 +313,34 @@ namespace AutopilotMonitor.Functions.Services.Backup.Queue retryableException = ex; } + if (leaseLost) + { + var (lostJob, lostETag) = await _jobs.GetWithETagAsync(jobId, ct).ConfigureAwait(false); + if (lostJob is not null && lostETag is not null) + { + lostJob.State = BackupJobState.Failed; + lostJob.CompletedAtUtc = _clock.GetUtcNow().UtcDateTime; + lostJob.LastHeartbeatUtc = lostJob.CompletedAtUtc.Value; + lostJob.Error = leaseHolder.RenewalFailureReason ?? "maintenance lease lost mid-run"; + await _jobs.TryUpdateWithCasAsync(lostJob, lostETag.Value, ct).ConfigureAwait(false); + } + // No queue delete on lease-loss path — message stays for the next dequeue, + // which will see JobState=Failed via duplicate-detection and drop it cleanly. + return; + } + if (retryableException is not null) { var (rb, rbETag) = await _jobs.GetWithETagAsync(jobId, ct).ConfigureAwait(false); if (rb is not null && rbETag is not null) { + var retryNow = _clock.GetUtcNow().UtcDateTime; rb.State = BackupJobState.Queued; rb.BackupId = null; // next attempt gets a fresh id (orphan blobs are cleaned by lifecycle) - rb.LastHeartbeatUtc = _clock.GetUtcNow().UtcDateTime; + rb.LastHeartbeatUtc = retryNow; + rb.QueuedAtUtc = retryNow; // Codex-Hotfix #2: reset queue clock so the watchdog's + // 60 min "Queued without pickup" rule does not fire on + // a job that has just rolled back for legitimate retry. rb.Error = retryableException.Message; await _jobs.TryUpdateWithCasAsync(rb, rbETag.Value, ct).ConfigureAwait(false); } @@ -311,7 +352,7 @@ namespace AutopilotMonitor.Functions.Services.Backup.Queue // ── Terminal CAS Completed (under lease) ──────────────────────── var completedAt = _clock.GetUtcNow().UtcDateTime; - var (finalJob, finalETag) = await _jobs.GetWithETagAsync(jobId, ct).ConfigureAwait(false); + var (finalJob, finalETag) = await _jobs.GetWithETagAsync(jobId, hct).ConfigureAwait(false); if (finalJob is not null && finalETag is not null) { finalJob.State = BackupJobState.Completed; @@ -319,7 +360,7 @@ namespace AutopilotMonitor.Functions.Services.Backup.Queue finalJob.LastHeartbeatUtc = completedAt; finalJob.BackupOutcome = result!.Outcome; finalJob.Error = null; - await _jobs.TryUpdateWithCasAsync(finalJob, finalETag.Value, ct).ConfigureAwait(false); + await _jobs.TryUpdateWithCasAsync(finalJob, finalETag.Value, hct).ConfigureAwait(false); } // ── OpsEvent (outside the service try — non-throwing per Wave25 #3) ── @@ -341,11 +382,9 @@ namespace AutopilotMonitor.Functions.Services.Backup.Queue finally { // Plan Wave16 #2 / Wave17 #1: terminal status set BEFORE release; release before delete. - if (lease is not null) - { - try { await lease.ReleaseAsync(cancellationToken: ct).ConfigureAwait(false); } - catch (Exception ex) { _logger.LogWarning(ex, "CriticalTableBackupQueueWorker: lease release failed (auto-expires)"); } - } + // Codex-Hotfix #1: leaseHolder disposes the renewal loop deterministically before + // calling Release on the underlying lease. + await leaseHolder.DisposeAsync().ConfigureAwait(false); } // Successful path: delete the message.