From 7a9a5c6d96c425655636ae00617a05a340b0c764 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Wed, 2 Sep 2026 15:21:42 +0800 Subject: [PATCH 01/10] Implement manual retention sweep --- ...hen_triggering_a_manual_retention_sweep.cs | 160 ++++++++++++++++++ .../Contracts/RetentionSweepRequest.cs | 23 +++ .../Contracts/RetentionSweepResponse.cs | 22 +++ .../Contracts/RetentionSweepStatus.cs | 25 +++ src/ServiceControl.Api/IRetentionApi.cs | 26 +++ .../Auth/Permissions.cs | 3 + .../Auth/RolePermissions.cs | 1 + .../Abstractions/BasePersistence.cs | 7 +- .../Infrastructure/RetentionSweeper.cs | 111 ++++++++++-- .../EFCore/RetentionSweepTests.cs | 138 +++++++++++++++ .../IRetentionSweeper.cs | 63 +++++++ .../APIApprovals.HttpApiRoutes.approved.txt | 2 + .../Infrastructure/Api/RetentionApi.cs | 137 +++++++++++++++ .../Retention/Api/RetentionController.cs | 51 ++++++ .../ServiceControlApiHostBuilderExtensions.cs | 1 + 15 files changed, 756 insertions(+), 14 deletions(-) create mode 100644 src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs create mode 100644 src/ServiceControl.Api/Contracts/RetentionSweepRequest.cs create mode 100644 src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs create mode 100644 src/ServiceControl.Api/Contracts/RetentionSweepStatus.cs create mode 100644 src/ServiceControl.Api/IRetentionApi.cs create mode 100644 src/ServiceControl.Persistence/IRetentionSweeper.cs create mode 100644 src/ServiceControl/Infrastructure/Api/RetentionApi.cs create mode 100644 src/ServiceControl/Retention/Api/RetentionController.cs diff --git a/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs b/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs new file mode 100644 index 0000000000..4b88c00bf2 --- /dev/null +++ b/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs @@ -0,0 +1,160 @@ +namespace ServiceControl.AcceptanceTests.WebApi; + +using System; +using System.Net; +using System.Net.Http; +using System.Net.Http.Json; +using System.Text.Json; +using System.Threading; +using System.Threading.Tasks; +using AcceptanceTesting; +using NServiceBus.AcceptanceTesting; +using NUnit.Framework; +using ServiceControl.Api.Contracts; + +class When_triggering_a_manual_retention_sweep : AcceptanceTest +{ + [Test] + public async Task Should_be_available_on_efcore_persisters() + { + if (StorageConfiguration.PersistenceType == "RavenDB") + { + Assert.Ignore("RavenDB has no sweeper — covered by Should_return_501_on_a_ravendb_backed_instance."); + return; + } + + HttpStatusCode started = default; + HttpStatusCode invalidCutoff = default; + + await Define() + .Done(async _ => + { + // Trigger a sweep with a past UTC cutoff. The delete work runs in the background, + // so the call returns 202 Accepted immediately. + using var response = await HttpClient.PostAsJsonAsync( + "/api/retention/sweep", + new RetentionSweepRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, + SerializerOptions); + + started = response.StatusCode; + + // A future-dated cutoff is rejected with 400. + using var badRequest = await HttpClient.PostAsJsonAsync( + "/api/retention/sweep", + new RetentionSweepRequest { ErrorCutoff = DateTime.UtcNow.AddDays(1) }, + SerializerOptions); + + invalidCutoff = badRequest.StatusCode; + + return true; + }) + .Run(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(started, Is.EqualTo(HttpStatusCode.Accepted), "the sweep should start in the background"); + Assert.That(invalidCutoff, Is.EqualTo(HttpStatusCode.BadRequest), "a future cutoff must be rejected"); + } + + // The status endpoint must report the run, and the background sweep must complete. + var status = await WaitUntilSweepFinishes(); + Assert.That(status.IsRunning, Is.False, "the background sweep must complete"); + Assert.That(status.LastStartedAt, Is.Not.Null); + } + + [Test] + public async Task Should_report_background_completion_via_status() + { + if (StorageConfiguration.PersistenceType == "RavenDB") + { + Assert.Ignore("RavenDB has no sweeper — covered by Should_return_501_on_a_ravendb_backed_instance."); + return; + } + + await Define() + .Done(async _ => + { + using var response = await HttpClient.PostAsJsonAsync( + "/api/retention/sweep", + new RetentionSweepRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, + SerializerOptions); + + return response.StatusCode == HttpStatusCode.Accepted; + }) + .Run(); + + var status = await WaitUntilSweepFinishes(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(status.IsRunning, Is.False); + Assert.That(status.LastFinishedAt, Is.Not.Null, "a completed run records its finish time"); + } + } + + [Test] + public async Task Should_return_501_on_a_ravendb_backed_instance() + { + if (StorageConfiguration.PersistenceType != "RavenDB") + { + Assert.Ignore("EFCore persisters support the sweep — covered by the efcore tests."); + return; + } + + HttpStatusCode postStatus = default; + HttpStatusCode getStatus = default; + + await Define() + .Done(async _ => + { + using var response = await HttpClient.PostAsJsonAsync( + "/api/retention/sweep", + new RetentionSweepRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, + SerializerOptions); + + postStatus = response.StatusCode; + + using var status = await HttpClient.GetAsync("/api/retention/sweep/status"); + + getStatus = status.StatusCode; + + return true; + }) + .Run(); + + using (Assert.EnterMultipleScope()) + { + // RavenDB retention is the server-side @expires bundle; there is no cutoff-based sweeper + // to trigger, so the optional IRetentionSweeper resolution is absent and both verbs + // return 501 Not Implemented. + Assert.That(postStatus, Is.EqualTo(HttpStatusCode.NotImplemented), "POST must report not-supported on RavenDB"); + Assert.That(getStatus, Is.EqualTo(HttpStatusCode.NotImplemented), "GET status must report not-supported on RavenDB"); + } + } + + async Task WaitUntilSweepFinishes(TimeSpan? timeout = null) + { + var deadline = DateTime.UtcNow + (timeout ?? TimeSpan.FromSeconds(30)); + + while (DateTime.UtcNow < deadline) + { + using var response = await HttpClient.GetAsync("/api/retention/sweep/status"); + + if (response.StatusCode == HttpStatusCode.OK) + { + var status = await response.Content.ReadFromJsonAsync(SerializerOptions); + + if (status is { IsRunning: false }) + { + return status; + } + } + + await Task.Delay(TimeSpan.FromMilliseconds(200)); + } + + throw new Exception("The manual retention sweep did not finish within the timeout."); + } + + class Context : ScenarioContext; +} \ No newline at end of file diff --git a/src/ServiceControl.Api/Contracts/RetentionSweepRequest.cs b/src/ServiceControl.Api/Contracts/RetentionSweepRequest.cs new file mode 100644 index 0000000000..48a352a1bf --- /dev/null +++ b/src/ServiceControl.Api/Contracts/RetentionSweepRequest.cs @@ -0,0 +1,23 @@ +namespace ServiceControl.Api.Contracts; + +using System; + +/// +/// Request body for POST /api/retention/sweep. Both cutoffs are optional; when omitted +/// the corresponding sub-sweep derives its cutoff from the configured retention period, as the +/// scheduled hourly sweep does. A bare future-dated cutoff is rejected. +/// +public class RetentionSweepRequest +{ + /// + /// Cutoff applied to the failed-message sweep. null means + /// now - ErrorRetentionPeriod. + /// + public DateTime? ErrorCutoff { get; set; } + + /// + /// Cutoff applied to the event-log sweep. null means + /// now - EventsRetentionPeriod. + /// + public DateTime? EventsCutoff { get; set; } +} \ No newline at end of file diff --git a/src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs b/src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs new file mode 100644 index 0000000000..7c68fe0c4b --- /dev/null +++ b/src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs @@ -0,0 +1,22 @@ +namespace ServiceControl.Api.Contracts; + +using System; + +/// +/// Response body for POST /api/retention/sweep. The Status field signals the +/// outcome: started (202), already-running (409), or +/// not-supported (501). +/// +public class RetentionSweepResponse +{ + public string Status { get; set; } + + public DateTime? StartedAt { get; set; } + + public DateTime? ErrorCutoff { get; set; } + + public DateTime? EventsCutoff { get; set; } + + /// A human-readable reason included when the operation is not supported. + public string Reason { get; set; } +} \ No newline at end of file diff --git a/src/ServiceControl.Api/Contracts/RetentionSweepStatus.cs b/src/ServiceControl.Api/Contracts/RetentionSweepStatus.cs new file mode 100644 index 0000000000..6e58c8f298 --- /dev/null +++ b/src/ServiceControl.Api/Contracts/RetentionSweepStatus.cs @@ -0,0 +1,25 @@ +namespace ServiceControl.Api.Contracts; + +using System; + +/// +/// Response body for GET /api/retention/sweep/status. On a persister with no sweeper +/// (e.g. RavenDB) the endpoint returns 501 with a instead. +/// +public class RetentionSweepStatus +{ + public bool IsRunning { get; set; } + + public DateTime? LastStartedAt { get; set; } + + public DateTime? LastFinishedAt { get; set; } + + public DateTime? LastErrorCutoff { get; set; } + + public DateTime? LastEventsCutoff { get; set; } + + public string LastError { get; set; } + + /// Present only on the 501 Not Implemented response. + public string Reason { get; set; } +} \ No newline at end of file diff --git a/src/ServiceControl.Api/IRetentionApi.cs b/src/ServiceControl.Api/IRetentionApi.cs new file mode 100644 index 0000000000..d497d716a7 --- /dev/null +++ b/src/ServiceControl.Api/IRetentionApi.cs @@ -0,0 +1,26 @@ +namespace ServiceControl.Api; + +using System.Threading; +using System.Threading.Tasks; +using Contracts; + +/// +/// Manual retention-sweep API. The implementation resolves the persister's sweeper +/// optionally: when no sweeper is registered (e.g. RavenDB, which uses server-side document +/// expiration) the operations report that the feature is not supported rather than silently +/// no-op'ing. +/// +public interface IRetentionApi +{ + /// + /// Starts a manual retention sweep with caller-supplied cutoffs. The delete work runs in + /// the background on a host-lifetime token; this method returns as soon as the run is + /// accepted (or refused because one is already running / unsupported). + /// + Task SweepAsync(RetentionSweepRequest request, CancellationToken cancellationToken = default); + + /// + /// Returns a point-in-time snapshot of sweep execution state for polling. + /// + Task GetStatusAsync(CancellationToken cancellationToken = default); +} \ No newline at end of file diff --git a/src/ServiceControl.Infrastructure/Auth/Permissions.cs b/src/ServiceControl.Infrastructure/Auth/Permissions.cs index 04f6582c14..b2af5c1c62 100644 --- a/src/ServiceControl.Infrastructure/Auth/Permissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/Permissions.cs @@ -58,6 +58,9 @@ public static class Permissions /// Event log area — viewing the event log. public const string ErrorEventLogView = "error:eventlog:view"; + /// Retention area — manually triggering a data retention sweep. + public const string ErrorRetentionSweep = "error:retention:sweep"; + /// Licensing area — viewing and managing license configuration. public const string ErrorLicensingView = "error:licensing:view"; /// diff --git a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs index 1263d43f1e..ad75a279d6 100644 --- a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs @@ -63,6 +63,7 @@ public static class RolePermissions Permissions.ErrorRedirectsManage, Permissions.ErrorThroughputView, Permissions.ErrorThroughputManage, + Permissions.ErrorRetentionSweep, ]; public static readonly FrozenDictionary> Roles = diff --git a/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs b/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs index e88ea72bfc..3002987a87 100644 --- a/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs +++ b/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs @@ -40,7 +40,12 @@ protected static void RegisterDataStores(IServiceCollection services, EFPersiste if (settings.RunRetentionSweep) { services.AddSingleton(); - services.AddHostedService(); + + // Register the sweeper as a resolvable singleton (concrete type + IRetentionSweeper) AND + // as a hosted service, all backed by one instance. + services.AddSingleton(); + services.AddHostedService(sp => sp.GetRequiredService()); + services.AddSingleton(sp => sp.GetRequiredService()); } services.AddSingleton(); diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs index 5396e060da..33e33ad681 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs @@ -5,6 +5,7 @@ namespace ServiceControl.Persistence.EFCore.Infrastructure; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using ServiceControl.MessageFailures; +using ServiceControl.Persistence; using ServiceControl.Persistence.EFCore.Abstractions; using ServiceControl.Persistence.EFCore.DbContexts; using ServiceControl.Persistence.EFCore.Entities; @@ -13,19 +14,39 @@ namespace ServiceControl.Persistence.EFCore.Infrastructure; // Deletes rows once they age past their retention period. // Runs hourly, in bounded batches so it never holds a large delete, and recomputes the cutoffs on // every run so a changed retention setting takes effect without rewriting any row. +// +// A manual sweep can be triggered via the API (see IRetentionSweeper / IRetentionApi) with +// caller-supplied cutoffs. public class RetentionSweeper( ILogger logger, TimeProvider timeProvider, IServiceScopeFactory serviceScopeFactory, IBodyStoragePersistence bodyStorage, RetentionMetrics metrics, - EFPersisterSettings settings) : BackgroundService + EFPersisterSettings settings, + IHostApplicationLifetime hostApplicationLifetime) : BackgroundService, IRetentionSweeper { const int BatchSize = 1000; static readonly TimeSpan Interval = TimeSpan.FromHours(1); static readonly TimeSpan InitialDelay = TimeSpan.FromMinutes(1); static readonly TimeSpan BatchPause = TimeSpan.FromSeconds(1); + // Single-flight guard shared by the hourly timer path and the manual API path so two sweeps + // never overlap. Precedent: ExternalIntegrationRequestsDataStore.drainLock. + readonly SemaphoreSlim sweepLock = new(1, 1); + + // Status snapshot for GET /api/retention/sweep/status polling. Volatile reads/writes are + // sufficient here: the fields are written under sweepLock (or once at start) and read + // lock-free for status reporting, which only needs an eventually-consistent snapshot. + volatile bool isRunning; + DateTime? lastStartedAt; + DateTime? lastFinishedAt; + DateTime? lastErrorCutoff; + DateTime? lastEventsCutoff; + string? lastError; + + public RetentionSweepConfig Config => new(settings.ErrorRetentionPeriod, settings.EventsRetentionPeriod); + protected override async Task ExecuteAsync(CancellationToken cancellationToken = default) { logger.LogInformation("Starting retention sweep"); @@ -40,7 +61,7 @@ protected override async Task ExecuteAsync(CancellationToken cancellationToken = { try { - await Sweep(pace: true, cancellationToken); + await Sweep(errorCutoff: null, eventsCutoff: null, pace: true, cancellationToken); } #pragma warning disable PS0019 // The filter already excludes OperationCanceledException, so // cancellation propagates; PS0019 only recognises a cancellationToken guard. @@ -58,13 +79,77 @@ protected override async Task ExecuteAsync(CancellationToken cancellationToken = } // Runs a full sweep immediately, bypassing the timer and the inter-batch pause. - // Intended for tests that need the effect without waiting for the hourly loop. - public Task SweepNow(CancellationToken cancellationToken = default) => Sweep(pace: false, cancellationToken); + // Intended for tests that need the effect without waiting for the hourly loop. Uses the + // default cutoff derivation (now - retention period). + public Task SweepNow(CancellationToken cancellationToken = default) => + Sweep(errorCutoff: null, eventsCutoff: null, pace: false, cancellationToken); + + public ManualSweepAttempt TryStartManualSweep(DateTime? errorCutoff, DateTime? eventsCutoff, CancellationToken cancellationToken = default) + { + // Try to acquire the single-flight lock without waiting if a scheduled or manual sweep is + // already running (holding the lock) + if (!sweepLock.Wait(0, cancellationToken)) + { + return new ManualSweepAttempt(ManualSweepOutcome.AlreadyRunning, lastStartedAt, errorCutoff, eventsCutoff); + } + + // Lock acquired on this thread. The background task owns it from here and releases it when + // the sweep body completes (SemaphoreSlim is not thread-affine, so releasing from the + // background thread is safe). isRunning is set now so a concurrent manual call sees it. + isRunning = true; + lastStartedAt = timeProvider.GetUtcNow().UtcDateTime; + lastErrorCutoff = errorCutoff; + lastEventsCutoff = eventsCutoff; + lastError = null; + + _ = SweepWithoutAcquiringLock(); + + return new ManualSweepAttempt(ManualSweepOutcome.Started, lastStartedAt, errorCutoff, eventsCutoff); + + async Task SweepWithoutAcquiringLock() + { + try + { + // if the caller doesn't hand over a real cancellation token then use the application lifetime. + await SweepBody(errorCutoff, eventsCutoff, false, cancellationToken.CanBeCanceled ? cancellationToken : hostApplicationLifetime.ApplicationStopping); + lastFinishedAt = timeProvider.GetUtcNow().UtcDateTime; + } + finally + { + isRunning = false; + sweepLock.Release(); + } + } + } + + public RetentionSweepStatus GetStatus() => new(isRunning, lastStartedAt, lastFinishedAt, lastErrorCutoff, lastEventsCutoff, lastError); + + async Task Sweep(DateTime? errorCutoff, DateTime? eventsCutoff, bool pace, CancellationToken cancellationToken) + { + await sweepLock.WaitAsync(cancellationToken); + isRunning = true; + lastStartedAt = timeProvider.GetUtcNow().UtcDateTime; + lastErrorCutoff = errorCutoff; + lastEventsCutoff = eventsCutoff; + lastError = null; + try + { + await SweepBody(errorCutoff, eventsCutoff, pace, cancellationToken); + lastFinishedAt = timeProvider.GetUtcNow().UtcDateTime; + } + finally + { + isRunning = false; + sweepLock.Release(); + } + } - async Task Sweep(bool pace, CancellationToken cancellationToken) + // The three sub-sweeps, isolated from lock management so both the locked Sweep path and the + // manual background path (which already holds the lock) share one implementation. + async Task SweepBody(DateTime? errorCutoff, DateTime? eventsCutoff, bool pace, CancellationToken cancellationToken) { - await RunPass(RetentionEntity.FailedMessages, token => SweepFailedMessages(pace, token), cancellationToken); - await RunPass(RetentionEntity.EventLog, token => SweepEventLogItems(pace, token), cancellationToken); + await RunPass(RetentionEntity.FailedMessages, token => SweepFailedMessages(pace, errorCutoff, token), cancellationToken); + await RunPass(RetentionEntity.EventLog, token => SweepEventLogItems(pace, eventsCutoff, token), cancellationToken); await RunPass(RetentionEntity.GroupComments, SweepOrphanedGroupComments, cancellationToken); } @@ -105,10 +190,10 @@ async Task SweepOrphanedGroupComments(CancellationToken cancellationToken) } // Event log items are insert-only and carry no external bodies, so each batch is a single - // ordered DELETE. - async Task SweepEventLogItems(bool pace, CancellationToken cancellationToken) + // ordered DELETE. A caller-supplied cutoff overrides the default derivation. + async Task SweepEventLogItems(bool pace, DateTime? eventsCutoff, CancellationToken cancellationToken) { - var cutoff = timeProvider.GetUtcNow().UtcDateTime - settings.EventsRetentionPeriod; + var cutoff = eventsCutoff ?? (timeProvider.GetUtcNow().UtcDateTime - settings.EventsRetentionPeriod); while (!cancellationToken.IsCancellationRequested) { @@ -135,9 +220,9 @@ async Task SweepEventLogItems(bool pace, CancellationToken cancellationToken) } } - async Task SweepFailedMessages(bool pace, CancellationToken cancellationToken) + async Task SweepFailedMessages(bool pace, DateTime? errorCutoff, CancellationToken cancellationToken) { - var cutoff = timeProvider.GetUtcNow().UtcDateTime - settings.ErrorRetentionPeriod; + var cutoff = errorCutoff ?? (timeProvider.GetUtcNow().UtcDateTime - settings.ErrorRetentionPeriod); while (!cancellationToken.IsCancellationRequested) { @@ -195,4 +280,4 @@ async Task SweepFailedMessages(bool pace, CancellationToken cancellationToken) static System.Linq.Expressions.Expression> IsExpired(DateTime cutoff) => failedMessage => (failedMessage.Status == FailedMessageStatus.Resolved || failedMessage.Status == FailedMessageStatus.Archived) && failedMessage.StatusChangedAt < cutoff; -} +} \ No newline at end of file diff --git a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs index 761e624e12..f34e759fe9 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs @@ -400,4 +400,142 @@ async Task> GetRemainingMarkers() var items = (await EventLogDataStore.GetEventLogItems(new PagingInfo(page: 1, pageSize: 100))).Results; return [.. items.Select(i => i.Description)]; } + + IRetentionSweeper GetSweeper() => ServiceProvider.GetRequiredService(); + + async Task WaitForManualSweepToFinish() + { + var sweeper = GetSweeper(); + await WaitUntil(() => Task.FromResult(!sweeper.GetStatus().IsRunning), + "the manual sweep to finish"); + } + + [Test] + public async Task Manual_sweep_uses_the_caller_supplied_error_cutoff_to_delete_early() + { + // 20 days old is within the 30 day configured retention, so the scheduled sweep would keep it. + // A caller-supplied cutoff of 15 days ago is earlier than the message, so the manual sweep deletes it. + var message = await SeedFailedMessage(FailedMessageStatus.Resolved, Now.AddDays(-20)); + + var attempt = await GetSweeper().TryStartManualSweep(Now.AddDays(-15), null, TODO); + + await WaitForManualSweepToFinish(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(attempt.Outcome, Is.EqualTo(ManualSweepOutcome.Started)); + Assert.That(await FindFailedMessage(message), Is.Null, + "the caller-supplied cutoff overrides the configured retention derivation"); + } + } + + [Test] + public async Task Manual_sweep_uses_the_caller_supplied_events_cutoff() + { + EFSettings.EventsRetentionPeriod = TimeSpan.FromDays(14); + + // 10 days old is within the 14 day configured events retention; a caller cutoff of 5 days ago deletes it. + await Store(EventLogRow("to-delete", Now.AddDays(-10))); + await Store(EventLogRow("to-keep", Now.AddDays(-3))); + + await GetSweeper().TryStartManualSweep(null, Now.AddDays(-5), TODO); + + await WaitForManualSweepToFinish(); + + var remaining = await GetRemainingMarkers(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(remaining, Does.Not.Contain("to-delete")); + Assert.That(remaining, Does.Contain("to-keep")); + } + } + + [Test] + public async Task Manual_sweep_with_null_cutoffs_keeps_the_default_derivation() + { + // No cutoff supplied => derive from settings as the scheduled path does. 29 days old is within 30 days. + var withinRetention = await SeedFailedMessage(FailedMessageStatus.Resolved, Now.AddDays(-29)); + var pastRetention = await SeedFailedMessage(FailedMessageStatus.Resolved, Now.AddDays(-31)); + + await GetSweeper().TryStartManualSweep(null, null, TODO); + + await WaitForManualSweepToFinish(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(await FindFailedMessage(withinRetention), Is.Not.Null); + Assert.That(await FindFailedMessage(pastRetention), Is.Null); + } + } + + [Test] + public async Task Manual_sweep_runs_in_the_background_and_reports_status() + { + await SeedFailedMessage(FailedMessageStatus.Resolved, Now.AddDays(-31)); + + var sweeper = GetSweeper(); + var attempt = await sweeper.TryStartManualSweep(Now.AddDays(-30), null, TODO); + + Assert.That(attempt.Outcome, Is.EqualTo(ManualSweepOutcome.Started)); + Assert.That(attempt.StartedAt, Is.Not.Null); + + await WaitForManualSweepToFinish(); + + var status = sweeper.GetStatus(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(status.IsRunning, Is.False); + Assert.That(status.LastStartedAt, Is.Not.Null); + Assert.That(status.LastFinishedAt, Is.Not.Null); + Assert.That(status.LastErrorCutoff, Is.Not.Null); + Assert.That(status.LastError, Is.Null); + } + } + + [Test] + public async Task A_second_manual_sweep_is_refused_while_one_is_running() + { + // Seed enough rows to force multiple delete batches so the first sweep is still running when the + // second, synchronous call is made. The single-flight lock is held from the moment the first call + // returns Started until the background body completes. + var rows = new List(); + for (var i = 0; i < 1500; i++) + { + rows.Add(new FailedMessageEntity + { + UniqueMessageId = Guid.NewGuid(), + Status = FailedMessageStatus.Archived, + StatusChangedAt = Now.AddDays(-31), + LastModified = Now.AddDays(-31), + NumberOfProcessingAttempts = 1, + FirstTimeOfFailure = Now.AddDays(-31), + LastTimeOfFailure = Now.AddDays(-31), + LastAttemptedAt = Now.AddDays(-31), + IsSystemMessage = false, + HeadersJson = "{}", + BodyStoredExternally = false, + BodySize = 0, + FailingEndpointAddress = "Shipping" + }); + } + + await Store([.. rows]); + + var sweeper = GetSweeper(); + var first = await sweeper.TryStartManualSweep(Now.AddDays(-30), null, TODO); + // Immediately request a second sweep on the same thread while the first is still deleting. + var second = await sweeper.TryStartManualSweep(Now.AddDays(-30), null, TODO); + + await WaitForManualSweepToFinish(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(first.Outcome, Is.EqualTo(ManualSweepOutcome.Started), + "the first call should start the sweep"); + Assert.That(second.Outcome, Is.EqualTo(ManualSweepOutcome.AlreadyRunning), + "a second sweep must not run in parallel with the first"); + } + } } diff --git a/src/ServiceControl.Persistence/IRetentionSweeper.cs b/src/ServiceControl.Persistence/IRetentionSweeper.cs new file mode 100644 index 0000000000..3f7b3d58fe --- /dev/null +++ b/src/ServiceControl.Persistence/IRetentionSweeper.cs @@ -0,0 +1,63 @@ +namespace ServiceControl.Persistence; + +using System; +using System.Threading; +using System.Threading.Tasks; + +/// +/// A persister-agnostic retention sweep operation. Only persisters that actually scan and +/// delete aged rows register this interface (e.g. the EFCore SQL persisters). RavenDB does +/// not — its retention is the server-side @expires bundle stamped per-document at +/// write time — so the interface is resolved optionally by the API, which returns +/// 501 Not Implemented when no registration is present. +/// +public interface IRetentionSweeper +{ + /// + /// The retention periods and minimum-age rules in force for this instance. + /// + RetentionSweepConfig Config { get; } + + /// + /// Starts a full retention sweep on a background task tied to the host lifetime (not the + /// caller's request token), using the caller-supplied cutoffs. When a cutoff is + /// null the corresponding sub-sweep derives its cutoff from the configured + /// retention period as the scheduled path does. + /// + /// A snapshot describing the run that was started; never throws for "already running" + /// — that is reported in the returned status. + ManualSweepAttempt TryStartManualSweep(DateTime? errorCutoff, DateTime? eventsCutoff, CancellationToken cancellationToken = default); + + /// + /// A point-in-time snapshot of sweep execution state for status polling. + /// + RetentionSweepStatus GetStatus(); +} + +/// Configuration describing the retention rules in force. +public sealed record RetentionSweepConfig(TimeSpan ErrorRetentionPeriod, TimeSpan EventsRetentionPeriod); + +/// The outcome of a manual sweep start request. +public enum ManualSweepOutcome +{ + /// The sweep was started on a background task. + Started, + /// A sweep is already running; the caller should poll . + AlreadyRunning +} + +/// The result of a call. +public sealed record ManualSweepAttempt( + ManualSweepOutcome Outcome, + DateTime? StartedAt, + DateTime? ErrorCutoff, + DateTime? EventsCutoff); + +/// A point-in-time snapshot of sweep execution state. +public sealed record RetentionSweepStatus( + bool IsRunning, + DateTime? LastStartedAt, + DateTime? LastFinishedAt, + DateTime? LastErrorCutoff, + DateTime? LastEventsCutoff, + string? LastError); \ No newline at end of file diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt index 264be98787..475a694bae 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt @@ -74,4 +74,6 @@ GET /redirects => ServiceControl.MessageRedirects.Api.MessageRedirectsController POST /redirects => ServiceControl.MessageRedirects.Api.MessageRedirectsController:NewRedirects(MessageRedirectRequest request, CancellationToken cancellationToken) DELETE /redirects/{messageRedirectId:guid} => ServiceControl.MessageRedirects.Api.MessageRedirectsController:DeleteRedirect(Guid messageRedirectId, CancellationToken cancellationToken) PUT /redirects/{messageRedirectId:guid} => ServiceControl.MessageRedirects.Api.MessageRedirectsController:UpdateRedirect(Guid messageRedirectId, MessageRedirectRequest request, CancellationToken cancellationToken) +POST /retention/sweep => ServiceControl.Retention.Api.RetentionController:Sweep(RetentionSweepRequest request, CancellationToken cancellationToken) +GET /retention/sweep/status => ServiceControl.Retention.Api.RetentionController:Status(CancellationToken cancellationToken) GET /sagas/{id} => ServiceControl.SagaAudit.SagasController:Sagas(PagingInfo pagingInfo, Guid id, CancellationToken cancellationToken) diff --git a/src/ServiceControl/Infrastructure/Api/RetentionApi.cs b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs new file mode 100644 index 0000000000..0d3e5649d6 --- /dev/null +++ b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs @@ -0,0 +1,137 @@ +namespace ServiceControl.Infrastructure.Api; + +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using ServiceBus.Management.Infrastructure.Settings; +using ServiceControl.Api; +using ServiceControl.Api.Contracts; + +// Manual retention-sweep API. The persister's IRetentionSweeper is resolved *optionally* so the +// same controller/route is mapped on every persister: EFCore registers it and gets 202/409/200; +// RavenDB registers nothing (its retention is the server-side @expires bundle) and gets 501. +class RetentionApi(IServiceProvider serviceProvider, Settings settings) : IRetentionApi +{ + const string NotSupportedReason = "The current storage has no retention sweeper."; + + public Task SweepAsync(RetentionSweepRequest request, CancellationToken cancellationToken = default) + { + // Resolve the sweeper lazily and optionally — never required via constructor injection, or + // a RavenDB-backed instance would throw at resolve time. Absent => 501 Not Implemented. + var sweeper = serviceProvider.GetService(); + if (sweeper is null) + { + return Task.FromResult(NotSupported()); + } + + // Maintenance mode refuses mutating operations; a sweep while the DB is being maintained + // would contend with the maintenance work. + if (settings.PersisterSpecificSettings?.MaintenanceMode == true) + { + return Task.FromResult(new RetentionSweepResponse { Status = "maintenance", Reason = "The instance is in maintenance mode." }); + } + + request ??= new RetentionSweepRequest(); + + // Cutoffs must be UTC and in the past. A future cutoff would delete nothing and is almost + // certainly a caller mistake, so it is rejected rather than clamped. + if (TryValidateCutoff(request.ErrorCutoff, out var errorCutoff, out var error) is false) + { + return Task.FromResult(Invalid(error)); + } + + if (TryValidateCutoff(request.EventsCutoff, out var eventsCutoff, out error) is false) + { + return Task.FromResult(Invalid(error)); + } + + var attempt = sweeper.TryStartManualSweep(errorCutoff, eventsCutoff, cancellationToken); + + return Task.FromResult(attempt.Outcome switch + { + ServiceControl.Persistence.ManualSweepOutcome.Started => new RetentionSweepResponse + { + Status = "started", + StartedAt = attempt.StartedAt, + ErrorCutoff = attempt.ErrorCutoff, + EventsCutoff = attempt.EventsCutoff + }, + ServiceControl.Persistence.ManualSweepOutcome.AlreadyRunning => new RetentionSweepResponse + { + Status = "already-running", + StartedAt = attempt.StartedAt + }, + _ => new RetentionSweepResponse + { + Status = "already-running", + StartedAt = attempt.StartedAt + } + }); + } + + public Task GetStatusAsync(CancellationToken cancellationToken = default) + { + var sweeper = serviceProvider.GetService(); + if (sweeper is null) + { + return Task.FromResult(new RetentionSweepStatus { Reason = NotSupportedReason }); + } + + // Map the persister's status record onto the API contract DTO (the two share a name but + // live in different namespaces: ServiceControl.Persistence vs ServiceControl.Api.Contracts). + ServiceControl.Persistence.RetentionSweepStatus status = sweeper.GetStatus(); + + return Task.FromResult(new RetentionSweepStatus + { + IsRunning = status.IsRunning, + LastStartedAt = status.LastStartedAt, + LastFinishedAt = status.LastFinishedAt, + LastErrorCutoff = status.LastErrorCutoff, + LastEventsCutoff = status.LastEventsCutoff, + LastError = status.LastError + }); + } + + static bool TryValidateCutoff(DateTime? supplied, out DateTime? validated, out string error) + { + if (supplied is null) + { + validated = null; + error = null; + return true; + } + + var value = supplied.Value; + + if (value.Kind != DateTimeKind.Utc) + { + validated = null; + error = "Cutoffs must be specified as UTC DateTime values."; + return false; + } + + if (value > DateTime.UtcNow) + { + validated = null; + error = "Cutoffs must not be in the future."; + return false; + } + + validated = value; + error = null; + return true; + } + + static RetentionSweepResponse NotSupported() => new() + { + Status = "not-supported", + Reason = NotSupportedReason + }; + + static RetentionSweepResponse Invalid(string reason) => new() + { + Status = "invalid-cutoff", + Reason = reason + }; +} \ No newline at end of file diff --git a/src/ServiceControl/Retention/Api/RetentionController.cs b/src/ServiceControl/Retention/Api/RetentionController.cs new file mode 100644 index 0000000000..2f0373872e --- /dev/null +++ b/src/ServiceControl/Retention/Api/RetentionController.cs @@ -0,0 +1,51 @@ +namespace ServiceControl.Retention.Api; + +using System.Threading; +using System.Threading.Tasks; +using Infrastructure.Auth; +using Microsoft.AspNetCore.Authorization; +using Microsoft.AspNetCore.Mvc; +using ServiceControl.Api; +using ServiceControl.Api.Contracts; + +// Manual retention-sweep endpoint. Lives only on the primary error instance (the sweeper is only +// registered there). On a RavenDB-backed instance IRetentionSweeper is not registered, so the +// IRetentionApi implementation returns a "not-supported" status that this controller maps to 501. +[ApiController] +[Route("api")] +public class RetentionController(IRetentionApi retentionApi) : ControllerBase +{ + // Starts a full retention sweep with caller-supplied cutoffs. The delete work runs in the + // background on a host-lifetime token; this returns as soon as the run is accepted (202), + // already running (409), in maintenance mode (503), unsupported by the persister (501), or + // the cutoff was invalid (400). + [Authorize(Policy = Permissions.ErrorRetentionSweep)] + [Route("retention/sweep")] + [HttpPost] + public async Task Sweep([FromBody] RetentionSweepRequest request, CancellationToken cancellationToken = default) + { + var response = await retentionApi.SweepAsync(request ?? new RetentionSweepRequest(), cancellationToken); + + return response.Status switch + { + "started" => Accepted(response), + "already-running" => Conflict(response), + "maintenance" => StatusCode(503, response), + "not-supported" => StatusCode(501, response), + "invalid-cutoff" => BadRequest(response), + _ => Ok(response) + }; + } + + // Polls the execution state of the most recent sweep. + [Authorize(Policy = Permissions.ErrorRetentionSweep)] + [Route("retention/sweep/status")] + [HttpGet] + public async Task Status(CancellationToken cancellationToken = default) + { + var status = await retentionApi.GetStatusAsync(cancellationToken); + + // A reason is present only when the persister has no sweeper (e.g. RavenDB). + return status.Reason is not null ? StatusCode(501, status) : Ok(status); + } +} \ No newline at end of file diff --git a/src/ServiceControl/ServiceControlApiHostBuilderExtensions.cs b/src/ServiceControl/ServiceControlApiHostBuilderExtensions.cs index 9cb637acff..ad678bcef7 100644 --- a/src/ServiceControl/ServiceControlApiHostBuilderExtensions.cs +++ b/src/ServiceControl/ServiceControlApiHostBuilderExtensions.cs @@ -12,6 +12,7 @@ public static void AddServiceControlApis(this IHostApplicationBuilder hostBuilde hostBuilder.Services.AddSingleton(); hostBuilder.Services.AddSingleton(); hostBuilder.Services.AddSingleton(); + hostBuilder.Services.AddSingleton(); } } } \ No newline at end of file From 5852ac5dd66db9c31cffb527ade83a56c6c41982 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Wed, 2 Sep 2026 15:44:29 +0800 Subject: [PATCH 02/10] fix up awaiter on tests --- .../EFCore/RetentionSweepTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs index f34e759fe9..aa4249ead8 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs @@ -417,7 +417,7 @@ public async Task Manual_sweep_uses_the_caller_supplied_error_cutoff_to_delete_e // A caller-supplied cutoff of 15 days ago is earlier than the message, so the manual sweep deletes it. var message = await SeedFailedMessage(FailedMessageStatus.Resolved, Now.AddDays(-20)); - var attempt = await GetSweeper().TryStartManualSweep(Now.AddDays(-15), null, TODO); + var attempt = GetSweeper().TryStartManualSweep(Now.AddDays(-15), null); await WaitForManualSweepToFinish(); @@ -438,7 +438,7 @@ public async Task Manual_sweep_uses_the_caller_supplied_events_cutoff() await Store(EventLogRow("to-delete", Now.AddDays(-10))); await Store(EventLogRow("to-keep", Now.AddDays(-3))); - await GetSweeper().TryStartManualSweep(null, Now.AddDays(-5), TODO); + GetSweeper().TryStartManualSweep(null, Now.AddDays(-5)); await WaitForManualSweepToFinish(); @@ -458,7 +458,7 @@ public async Task Manual_sweep_with_null_cutoffs_keeps_the_default_derivation() var withinRetention = await SeedFailedMessage(FailedMessageStatus.Resolved, Now.AddDays(-29)); var pastRetention = await SeedFailedMessage(FailedMessageStatus.Resolved, Now.AddDays(-31)); - await GetSweeper().TryStartManualSweep(null, null, TODO); + GetSweeper().TryStartManualSweep(null, null); await WaitForManualSweepToFinish(); @@ -475,7 +475,7 @@ public async Task Manual_sweep_runs_in_the_background_and_reports_status() await SeedFailedMessage(FailedMessageStatus.Resolved, Now.AddDays(-31)); var sweeper = GetSweeper(); - var attempt = await sweeper.TryStartManualSweep(Now.AddDays(-30), null, TODO); + var attempt = sweeper.TryStartManualSweep(Now.AddDays(-30), null); Assert.That(attempt.Outcome, Is.EqualTo(ManualSweepOutcome.Started)); Assert.That(attempt.StartedAt, Is.Not.Null); @@ -524,9 +524,9 @@ public async Task A_second_manual_sweep_is_refused_while_one_is_running() await Store([.. rows]); var sweeper = GetSweeper(); - var first = await sweeper.TryStartManualSweep(Now.AddDays(-30), null, TODO); + var first = sweeper.TryStartManualSweep(Now.AddDays(-30), null); // Immediately request a second sweep on the same thread while the first is still deleting. - var second = await sweeper.TryStartManualSweep(Now.AddDays(-30), null, TODO); + var second = sweeper.TryStartManualSweep(Now.AddDays(-30), null); await WaitForManualSweepToFinish(); From a833e591c26dbbe120dc177ad178d41cd6ee4c9c Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Thu, 3 Sep 2026 09:40:36 +0800 Subject: [PATCH 03/10] do completion wait within the test run where the api client still works --- ...hen_triggering_a_manual_retention_sweep.cs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs b/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs index 4b88c00bf2..4f7cfda84d 100644 --- a/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs +++ b/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs @@ -25,6 +25,7 @@ public async Task Should_be_available_on_efcore_persisters() HttpStatusCode started = default; HttpStatusCode invalidCutoff = default; + RetentionSweepStatus completion = null; await Define() .Done(async _ => @@ -46,6 +47,9 @@ await Define() invalidCutoff = badRequest.StatusCode; + // The status endpoint must report the run, and the background sweep must complete. + completion = await WaitUntilSweepFinishes(); + return true; }) .Run(); @@ -54,12 +58,10 @@ await Define() { Assert.That(started, Is.EqualTo(HttpStatusCode.Accepted), "the sweep should start in the background"); Assert.That(invalidCutoff, Is.EqualTo(HttpStatusCode.BadRequest), "a future cutoff must be rejected"); + Assert.That(completion, Is.Not.Null, "the background sweep must complete"); + Assert.That(completion.IsRunning, Is.False, "the background sweep must complete"); + Assert.That(completion.LastStartedAt, Is.Not.Null); } - - // The status endpoint must report the run, and the background sweep must complete. - var status = await WaitUntilSweepFinishes(); - Assert.That(status.IsRunning, Is.False, "the background sweep must complete"); - Assert.That(status.LastStartedAt, Is.Not.Null); } [Test] @@ -71,6 +73,8 @@ public async Task Should_report_background_completion_via_status() return; } + RetentionSweepStatus completion = null; + await Define() .Done(async _ => { @@ -79,16 +83,17 @@ await Define() new RetentionSweepRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, SerializerOptions); + completion = await WaitUntilSweepFinishes(); return response.StatusCode == HttpStatusCode.Accepted; }) .Run(); - var status = await WaitUntilSweepFinishes(); using (Assert.EnterMultipleScope()) { - Assert.That(status.IsRunning, Is.False); - Assert.That(status.LastFinishedAt, Is.Not.Null, "a completed run records its finish time"); + Assert.That(completion, Is.Not.Null, "the background sweep must complete"); + Assert.That(completion.IsRunning, Is.False); + Assert.That(completion.LastFinishedAt, Is.Not.Null, "a completed run records its finish time"); } } From 6f547516c428b9a0703232c52645ac5874a81058 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Mon, 7 Sep 2026 10:22:16 +0800 Subject: [PATCH 04/10] Tidy up retention api --- ...hen_triggering_a_manual_retention_sweep.cs | 8 ++-- .../Contracts/RetentionSweepResponse.cs | 2 +- ...tus.cs => RetentionSweepStatusResponse.cs} | 2 +- src/ServiceControl.Api/IRetentionApi.cs | 6 +-- .../Infrastructure/RetentionSweeper.cs | 8 ++-- .../EFCore/RetentionSweepTests.cs | 8 ++-- .../IRetentionSweeper.cs | 48 ++----------------- .../ManualSweepAttempt.cs | 10 ++++ .../RetentionSweepCurrentStatus.cs | 12 +++++ .../RetentionSweepStatus.cs | 10 ++++ .../Infrastructure/Api/RetentionApi.cs | 22 +++++---- .../Retention/Api/RetentionController.cs | 4 +- 12 files changed, 67 insertions(+), 73 deletions(-) rename src/ServiceControl.Api/Contracts/{RetentionSweepStatus.cs => RetentionSweepStatusResponse.cs} (94%) create mode 100644 src/ServiceControl.Persistence/ManualSweepAttempt.cs create mode 100644 src/ServiceControl.Persistence/RetentionSweepCurrentStatus.cs create mode 100644 src/ServiceControl.Persistence/RetentionSweepStatus.cs diff --git a/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs b/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs index 4f7cfda84d..2b6e23456b 100644 --- a/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs +++ b/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs @@ -25,7 +25,7 @@ public async Task Should_be_available_on_efcore_persisters() HttpStatusCode started = default; HttpStatusCode invalidCutoff = default; - RetentionSweepStatus completion = null; + RetentionSweepStatusResponse completion = null; await Define() .Done(async _ => @@ -73,7 +73,7 @@ public async Task Should_report_background_completion_via_status() return; } - RetentionSweepStatus completion = null; + RetentionSweepStatusResponse completion = null; await Define() .Done(async _ => @@ -137,7 +137,7 @@ await Define() } } - async Task WaitUntilSweepFinishes(TimeSpan? timeout = null) + async Task WaitUntilSweepFinishes(TimeSpan? timeout = null) { var deadline = DateTime.UtcNow + (timeout ?? TimeSpan.FromSeconds(30)); @@ -147,7 +147,7 @@ async Task WaitUntilSweepFinishes(TimeSpan? timeout = null if (response.StatusCode == HttpStatusCode.OK) { - var status = await response.Content.ReadFromJsonAsync(SerializerOptions); + var status = await response.Content.ReadFromJsonAsync(SerializerOptions); if (status is { IsRunning: false }) { diff --git a/src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs b/src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs index 7c68fe0c4b..f2eddd082b 100644 --- a/src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs +++ b/src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs @@ -19,4 +19,4 @@ public class RetentionSweepResponse /// A human-readable reason included when the operation is not supported. public string Reason { get; set; } -} \ No newline at end of file +} diff --git a/src/ServiceControl.Api/Contracts/RetentionSweepStatus.cs b/src/ServiceControl.Api/Contracts/RetentionSweepStatusResponse.cs similarity index 94% rename from src/ServiceControl.Api/Contracts/RetentionSweepStatus.cs rename to src/ServiceControl.Api/Contracts/RetentionSweepStatusResponse.cs index 6e58c8f298..a671f17bc5 100644 --- a/src/ServiceControl.Api/Contracts/RetentionSweepStatus.cs +++ b/src/ServiceControl.Api/Contracts/RetentionSweepStatusResponse.cs @@ -6,7 +6,7 @@ namespace ServiceControl.Api.Contracts; /// Response body for GET /api/retention/sweep/status. On a persister with no sweeper /// (e.g. RavenDB) the endpoint returns 501 with a instead. /// -public class RetentionSweepStatus +public class RetentionSweepStatusResponse { public bool IsRunning { get; set; } diff --git a/src/ServiceControl.Api/IRetentionApi.cs b/src/ServiceControl.Api/IRetentionApi.cs index d497d716a7..1915417cee 100644 --- a/src/ServiceControl.Api/IRetentionApi.cs +++ b/src/ServiceControl.Api/IRetentionApi.cs @@ -8,7 +8,7 @@ namespace ServiceControl.Api; /// Manual retention-sweep API. The implementation resolves the persister's sweeper /// optionally: when no sweeper is registered (e.g. RavenDB, which uses server-side document /// expiration) the operations report that the feature is not supported rather than silently -/// no-op'ing. +/// failing. /// public interface IRetentionApi { @@ -17,10 +17,10 @@ public interface IRetentionApi /// the background on a host-lifetime token; this method returns as soon as the run is /// accepted (or refused because one is already running / unsupported). /// - Task SweepAsync(RetentionSweepRequest request, CancellationToken cancellationToken = default); + Task Sweep(RetentionSweepRequest request, CancellationToken cancellationToken = default); /// /// Returns a point-in-time snapshot of sweep execution state for polling. /// - Task GetStatusAsync(CancellationToken cancellationToken = default); + Task GetStatus(CancellationToken cancellationToken = default); } \ No newline at end of file diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs index 33e33ad681..f76be5b2af 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs @@ -45,8 +45,6 @@ public class RetentionSweeper( DateTime? lastEventsCutoff; string? lastError; - public RetentionSweepConfig Config => new(settings.ErrorRetentionPeriod, settings.EventsRetentionPeriod); - protected override async Task ExecuteAsync(CancellationToken cancellationToken = default) { logger.LogInformation("Starting retention sweep"); @@ -90,7 +88,7 @@ public ManualSweepAttempt TryStartManualSweep(DateTime? errorCutoff, DateTime? e // already running (holding the lock) if (!sweepLock.Wait(0, cancellationToken)) { - return new ManualSweepAttempt(ManualSweepOutcome.AlreadyRunning, lastStartedAt, errorCutoff, eventsCutoff); + return new ManualSweepAttempt(RetentionSweepStatus.AlreadyRunning, lastStartedAt, errorCutoff, eventsCutoff); } // Lock acquired on this thread. The background task owns it from here and releases it when @@ -104,7 +102,7 @@ public ManualSweepAttempt TryStartManualSweep(DateTime? errorCutoff, DateTime? e _ = SweepWithoutAcquiringLock(); - return new ManualSweepAttempt(ManualSweepOutcome.Started, lastStartedAt, errorCutoff, eventsCutoff); + return new ManualSweepAttempt(RetentionSweepStatus.Started, lastStartedAt, errorCutoff, eventsCutoff); async Task SweepWithoutAcquiringLock() { @@ -122,7 +120,7 @@ async Task SweepWithoutAcquiringLock() } } - public RetentionSweepStatus GetStatus() => new(isRunning, lastStartedAt, lastFinishedAt, lastErrorCutoff, lastEventsCutoff, lastError); + public RetentionSweepCurrentStatus GetStatus() => new(isRunning, lastStartedAt, lastFinishedAt, lastErrorCutoff, lastEventsCutoff, lastError); async Task Sweep(DateTime? errorCutoff, DateTime? eventsCutoff, bool pace, CancellationToken cancellationToken) { diff --git a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs index aa4249ead8..7b54d53926 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs @@ -423,7 +423,7 @@ public async Task Manual_sweep_uses_the_caller_supplied_error_cutoff_to_delete_e using (Assert.EnterMultipleScope()) { - Assert.That(attempt.Outcome, Is.EqualTo(ManualSweepOutcome.Started)); + Assert.That(attempt.Outcome, Is.EqualTo(RetentionSweepStatus.Started)); Assert.That(await FindFailedMessage(message), Is.Null, "the caller-supplied cutoff overrides the configured retention derivation"); } @@ -477,7 +477,7 @@ public async Task Manual_sweep_runs_in_the_background_and_reports_status() var sweeper = GetSweeper(); var attempt = sweeper.TryStartManualSweep(Now.AddDays(-30), null); - Assert.That(attempt.Outcome, Is.EqualTo(ManualSweepOutcome.Started)); + Assert.That(attempt.Outcome, Is.EqualTo(RetentionSweepStatus.Started)); Assert.That(attempt.StartedAt, Is.Not.Null); await WaitForManualSweepToFinish(); @@ -532,9 +532,9 @@ public async Task A_second_manual_sweep_is_refused_while_one_is_running() using (Assert.EnterMultipleScope()) { - Assert.That(first.Outcome, Is.EqualTo(ManualSweepOutcome.Started), + Assert.That(first.Outcome, Is.EqualTo(RetentionSweepStatus.Started), "the first call should start the sweep"); - Assert.That(second.Outcome, Is.EqualTo(ManualSweepOutcome.AlreadyRunning), + Assert.That(second.Outcome, Is.EqualTo(RetentionSweepStatus.AlreadyRunning), "a second sweep must not run in parallel with the first"); } } diff --git a/src/ServiceControl.Persistence/IRetentionSweeper.cs b/src/ServiceControl.Persistence/IRetentionSweeper.cs index 3f7b3d58fe..96b52112c9 100644 --- a/src/ServiceControl.Persistence/IRetentionSweeper.cs +++ b/src/ServiceControl.Persistence/IRetentionSweeper.cs @@ -2,7 +2,6 @@ namespace ServiceControl.Persistence; using System; using System.Threading; -using System.Threading.Tasks; /// /// A persister-agnostic retention sweep operation. Only persisters that actually scan and @@ -14,50 +13,13 @@ namespace ServiceControl.Persistence; public interface IRetentionSweeper { /// - /// The retention periods and minimum-age rules in force for this instance. + /// Starts a full retention sweep on a background task /// - RetentionSweepConfig Config { get; } - - /// - /// Starts a full retention sweep on a background task tied to the host lifetime (not the - /// caller's request token), using the caller-supplied cutoffs. When a cutoff is - /// null the corresponding sub-sweep derives its cutoff from the configured - /// retention period as the scheduled path does. - /// - /// A snapshot describing the run that was started; never throws for "already running" - /// — that is reported in the returned status. + /// A snapshot describing the run that was started; will respond with a status of AlreadyRunning if there is already a sweep running. ManualSweepAttempt TryStartManualSweep(DateTime? errorCutoff, DateTime? eventsCutoff, CancellationToken cancellationToken = default); /// - /// A point-in-time snapshot of sweep execution state for status polling. + /// A point-in-time snapshot of current sweep execution state for status polling. /// - RetentionSweepStatus GetStatus(); -} - -/// Configuration describing the retention rules in force. -public sealed record RetentionSweepConfig(TimeSpan ErrorRetentionPeriod, TimeSpan EventsRetentionPeriod); - -/// The outcome of a manual sweep start request. -public enum ManualSweepOutcome -{ - /// The sweep was started on a background task. - Started, - /// A sweep is already running; the caller should poll . - AlreadyRunning -} - -/// The result of a call. -public sealed record ManualSweepAttempt( - ManualSweepOutcome Outcome, - DateTime? StartedAt, - DateTime? ErrorCutoff, - DateTime? EventsCutoff); - -/// A point-in-time snapshot of sweep execution state. -public sealed record RetentionSweepStatus( - bool IsRunning, - DateTime? LastStartedAt, - DateTime? LastFinishedAt, - DateTime? LastErrorCutoff, - DateTime? LastEventsCutoff, - string? LastError); \ No newline at end of file + RetentionSweepCurrentStatus GetStatus(); +} \ No newline at end of file diff --git a/src/ServiceControl.Persistence/ManualSweepAttempt.cs b/src/ServiceControl.Persistence/ManualSweepAttempt.cs new file mode 100644 index 0000000000..451f59c66d --- /dev/null +++ b/src/ServiceControl.Persistence/ManualSweepAttempt.cs @@ -0,0 +1,10 @@ +namespace ServiceControl.Persistence; + +using System; + +/// The result of a call. +public sealed record ManualSweepAttempt( + RetentionSweepStatus Outcome, + DateTime? StartedAt, + DateTime? ErrorCutoff, + DateTime? EventsCutoff); \ No newline at end of file diff --git a/src/ServiceControl.Persistence/RetentionSweepCurrentStatus.cs b/src/ServiceControl.Persistence/RetentionSweepCurrentStatus.cs new file mode 100644 index 0000000000..452204b5ff --- /dev/null +++ b/src/ServiceControl.Persistence/RetentionSweepCurrentStatus.cs @@ -0,0 +1,12 @@ +namespace ServiceControl.Persistence; + +using System; + +/// A point-in-time snapshot of sweep execution state. +public sealed record RetentionSweepCurrentStatus( + bool IsRunning, + DateTime? LastStartedAt, + DateTime? LastFinishedAt, + DateTime? LastErrorCutoff, + DateTime? LastEventsCutoff, + string? LastError); \ No newline at end of file diff --git a/src/ServiceControl.Persistence/RetentionSweepStatus.cs b/src/ServiceControl.Persistence/RetentionSweepStatus.cs new file mode 100644 index 0000000000..b7a1f9ca52 --- /dev/null +++ b/src/ServiceControl.Persistence/RetentionSweepStatus.cs @@ -0,0 +1,10 @@ +namespace ServiceControl.Persistence; + +/// The outcome of a sweep start request. +public enum RetentionSweepStatus +{ + /// The sweep was started on a background task. + Started, + /// A sweep is already running. + AlreadyRunning +} \ No newline at end of file diff --git a/src/ServiceControl/Infrastructure/Api/RetentionApi.cs b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs index 0d3e5649d6..21c0596fd9 100644 --- a/src/ServiceControl/Infrastructure/Api/RetentionApi.cs +++ b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs @@ -4,22 +4,24 @@ namespace ServiceControl.Infrastructure.Api; using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; +using Persistence; using ServiceBus.Management.Infrastructure.Settings; using ServiceControl.Api; using ServiceControl.Api.Contracts; +using RetentionSweepStatus = Persistence.RetentionSweepStatus; // Manual retention-sweep API. The persister's IRetentionSweeper is resolved *optionally* so the // same controller/route is mapped on every persister: EFCore registers it and gets 202/409/200; // RavenDB registers nothing (its retention is the server-side @expires bundle) and gets 501. class RetentionApi(IServiceProvider serviceProvider, Settings settings) : IRetentionApi { - const string NotSupportedReason = "The current storage has no retention sweeper."; + const string NotSupportedReason = "The currently configured storage has no retention sweeper."; - public Task SweepAsync(RetentionSweepRequest request, CancellationToken cancellationToken = default) + public Task Sweep(RetentionSweepRequest request, CancellationToken cancellationToken = default) { // Resolve the sweeper lazily and optionally — never required via constructor injection, or // a RavenDB-backed instance would throw at resolve time. Absent => 501 Not Implemented. - var sweeper = serviceProvider.GetService(); + var sweeper = serviceProvider.GetService(); if (sweeper is null) { return Task.FromResult(NotSupported()); @@ -50,14 +52,14 @@ public Task SweepAsync(RetentionSweepRequest request, Ca return Task.FromResult(attempt.Outcome switch { - ServiceControl.Persistence.ManualSweepOutcome.Started => new RetentionSweepResponse + RetentionSweepStatus.Started => new RetentionSweepResponse { Status = "started", StartedAt = attempt.StartedAt, ErrorCutoff = attempt.ErrorCutoff, EventsCutoff = attempt.EventsCutoff }, - ServiceControl.Persistence.ManualSweepOutcome.AlreadyRunning => new RetentionSweepResponse + RetentionSweepStatus.AlreadyRunning => new RetentionSweepResponse { Status = "already-running", StartedAt = attempt.StartedAt @@ -70,19 +72,19 @@ public Task SweepAsync(RetentionSweepRequest request, Ca }); } - public Task GetStatusAsync(CancellationToken cancellationToken = default) + public Task GetStatus(CancellationToken cancellationToken = default) { - var sweeper = serviceProvider.GetService(); + var sweeper = serviceProvider.GetService(); if (sweeper is null) { - return Task.FromResult(new RetentionSweepStatus { Reason = NotSupportedReason }); + return Task.FromResult(new RetentionSweepStatusResponse { Reason = NotSupportedReason }); } // Map the persister's status record onto the API contract DTO (the two share a name but // live in different namespaces: ServiceControl.Persistence vs ServiceControl.Api.Contracts). - ServiceControl.Persistence.RetentionSweepStatus status = sweeper.GetStatus(); + var status = sweeper.GetStatus(); - return Task.FromResult(new RetentionSweepStatus + return Task.FromResult(new RetentionSweepStatusResponse { IsRunning = status.IsRunning, LastStartedAt = status.LastStartedAt, diff --git a/src/ServiceControl/Retention/Api/RetentionController.cs b/src/ServiceControl/Retention/Api/RetentionController.cs index 2f0373872e..05ed776fdf 100644 --- a/src/ServiceControl/Retention/Api/RetentionController.cs +++ b/src/ServiceControl/Retention/Api/RetentionController.cs @@ -24,7 +24,7 @@ public class RetentionController(IRetentionApi retentionApi) : ControllerBase [HttpPost] public async Task Sweep([FromBody] RetentionSweepRequest request, CancellationToken cancellationToken = default) { - var response = await retentionApi.SweepAsync(request ?? new RetentionSweepRequest(), cancellationToken); + var response = await retentionApi.Sweep(request ?? new RetentionSweepRequest(), cancellationToken); return response.Status switch { @@ -43,7 +43,7 @@ public async Task Sweep([FromBody] RetentionSweepRequest request, [HttpGet] public async Task Status(CancellationToken cancellationToken = default) { - var status = await retentionApi.GetStatusAsync(cancellationToken); + var status = await retentionApi.GetStatus(cancellationToken); // A reason is present only when the persister has no sweeper (e.g. RavenDB). return status.Reason is not null ? StatusCode(501, status) : Ok(status); From 95016443b53ebe5cd9535dbbb9397cf26a9d3296 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Mon, 7 Sep 2026 12:15:22 +0800 Subject: [PATCH 05/10] Rename retention api to be more generic --- ...en_triggering_a_manual_retention_purge.cs} | 50 ++++++++--------- ...eepRequest.cs => RetentionPurgeRequest.cs} | 10 ++-- ...pResponse.cs => RetentionPurgeResponse.cs} | 4 +- ...nse.cs => RetentionPurgeStatusResponse.cs} | 4 +- src/ServiceControl.Api/IRetentionApi.cs | 10 ++-- .../Auth/Permissions.cs | 4 +- .../Auth/RolePermissions.cs | 2 +- .../Infrastructure/RetentionSweeper.cs | 2 +- .../APIApprovals.HttpApiRoutes.approved.txt | 4 +- .../Infrastructure/Api/RetentionApi.cs | 54 +++++++------------ ...ller.cs => SystemMaintenanceController.cs} | 33 ++++++------ 11 files changed, 81 insertions(+), 96 deletions(-) rename src/ServiceControl.AcceptanceTests/WebApi/{When_triggering_a_manual_retention_sweep.cs => When_triggering_a_manual_retention_purge.cs} (78%) rename src/ServiceControl.Api/Contracts/{RetentionSweepRequest.cs => RetentionPurgeRequest.cs} (54%) rename src/ServiceControl.Api/Contracts/{RetentionSweepResponse.cs => RetentionPurgeResponse.cs} (78%) rename src/ServiceControl.Api/Contracts/{RetentionSweepStatusResponse.cs => RetentionPurgeStatusResponse.cs} (79%) rename src/ServiceControl/Retention/Api/{RetentionController.cs => SystemMaintenanceController.cs} (55%) diff --git a/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs b/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_purge.cs similarity index 78% rename from src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs rename to src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_purge.cs index 2b6e23456b..da514f32db 100644 --- a/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_sweep.cs +++ b/src/ServiceControl.AcceptanceTests/WebApi/When_triggering_a_manual_retention_purge.cs @@ -12,7 +12,7 @@ namespace ServiceControl.AcceptanceTests.WebApi; using NUnit.Framework; using ServiceControl.Api.Contracts; -class When_triggering_a_manual_retention_sweep : AcceptanceTest +class When_triggering_a_manual_retention_purge : AcceptanceTest { [Test] public async Task Should_be_available_on_efcore_persisters() @@ -25,30 +25,30 @@ public async Task Should_be_available_on_efcore_persisters() HttpStatusCode started = default; HttpStatusCode invalidCutoff = default; - RetentionSweepStatusResponse completion = null; + RetentionPurgeStatusResponse completion = null; await Define() .Done(async _ => { - // Trigger a sweep with a past UTC cutoff. The delete work runs in the background, + // Trigger a purge with a past UTC cutoff. The delete work runs in the background, // so the call returns 202 Accepted immediately. using var response = await HttpClient.PostAsJsonAsync( - "/api/retention/sweep", - new RetentionSweepRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, + "/api/maintenance/retention/purge", + new RetentionPurgeRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, SerializerOptions); started = response.StatusCode; // A future-dated cutoff is rejected with 400. using var badRequest = await HttpClient.PostAsJsonAsync( - "/api/retention/sweep", - new RetentionSweepRequest { ErrorCutoff = DateTime.UtcNow.AddDays(1) }, + "/api/maintenance/retention/purge", + new RetentionPurgeRequest { ErrorCutoff = DateTime.UtcNow.AddDays(1) }, SerializerOptions); invalidCutoff = badRequest.StatusCode; - // The status endpoint must report the run, and the background sweep must complete. - completion = await WaitUntilSweepFinishes(); + // The status endpoint must report the run, and the background purge must complete. + completion = await WaitUntilPurgeFinishes(); return true; }) @@ -56,10 +56,10 @@ await Define() using (Assert.EnterMultipleScope()) { - Assert.That(started, Is.EqualTo(HttpStatusCode.Accepted), "the sweep should start in the background"); + Assert.That(started, Is.EqualTo(HttpStatusCode.Accepted), "the purge should start in the background"); Assert.That(invalidCutoff, Is.EqualTo(HttpStatusCode.BadRequest), "a future cutoff must be rejected"); - Assert.That(completion, Is.Not.Null, "the background sweep must complete"); - Assert.That(completion.IsRunning, Is.False, "the background sweep must complete"); + Assert.That(completion, Is.Not.Null, "the background purge must complete"); + Assert.That(completion.IsRunning, Is.False, "the background purge must complete"); Assert.That(completion.LastStartedAt, Is.Not.Null); } } @@ -73,17 +73,17 @@ public async Task Should_report_background_completion_via_status() return; } - RetentionSweepStatusResponse completion = null; + RetentionPurgeStatusResponse completion = null; await Define() .Done(async _ => { using var response = await HttpClient.PostAsJsonAsync( - "/api/retention/sweep", - new RetentionSweepRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, + "/api/maintenance/retention/purge", + new RetentionPurgeRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, SerializerOptions); - completion = await WaitUntilSweepFinishes(); + completion = await WaitUntilPurgeFinishes(); return response.StatusCode == HttpStatusCode.Accepted; }) .Run(); @@ -91,7 +91,7 @@ await Define() using (Assert.EnterMultipleScope()) { - Assert.That(completion, Is.Not.Null, "the background sweep must complete"); + Assert.That(completion, Is.Not.Null, "the background purge must complete"); Assert.That(completion.IsRunning, Is.False); Assert.That(completion.LastFinishedAt, Is.Not.Null, "a completed run records its finish time"); } @@ -102,7 +102,7 @@ public async Task Should_return_501_on_a_ravendb_backed_instance() { if (StorageConfiguration.PersistenceType != "RavenDB") { - Assert.Ignore("EFCore persisters support the sweep — covered by the efcore tests."); + Assert.Ignore("EFCore persisters support the purge — covered by the efcore tests."); return; } @@ -113,13 +113,13 @@ await Define() .Done(async _ => { using var response = await HttpClient.PostAsJsonAsync( - "/api/retention/sweep", - new RetentionSweepRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, + "/api/maintenance/retention/purge", + new RetentionPurgeRequest { ErrorCutoff = DateTime.UtcNow.AddDays(-30) }, SerializerOptions); postStatus = response.StatusCode; - using var status = await HttpClient.GetAsync("/api/retention/sweep/status"); + using var status = await HttpClient.GetAsync("/api/maintenance/retention/purge/status"); getStatus = status.StatusCode; @@ -137,17 +137,17 @@ await Define() } } - async Task WaitUntilSweepFinishes(TimeSpan? timeout = null) + async Task WaitUntilPurgeFinishes(TimeSpan? timeout = null) { var deadline = DateTime.UtcNow + (timeout ?? TimeSpan.FromSeconds(30)); while (DateTime.UtcNow < deadline) { - using var response = await HttpClient.GetAsync("/api/retention/sweep/status"); + using var response = await HttpClient.GetAsync("/api/maintenance/retention/purge/status"); if (response.StatusCode == HttpStatusCode.OK) { - var status = await response.Content.ReadFromJsonAsync(SerializerOptions); + var status = await response.Content.ReadFromJsonAsync(SerializerOptions); if (status is { IsRunning: false }) { @@ -158,7 +158,7 @@ async Task WaitUntilSweepFinishes(TimeSpan? timeou await Task.Delay(TimeSpan.FromMilliseconds(200)); } - throw new Exception("The manual retention sweep did not finish within the timeout."); + throw new Exception("The manual retention purge did not finish within the timeout."); } class Context : ScenarioContext; diff --git a/src/ServiceControl.Api/Contracts/RetentionSweepRequest.cs b/src/ServiceControl.Api/Contracts/RetentionPurgeRequest.cs similarity index 54% rename from src/ServiceControl.Api/Contracts/RetentionSweepRequest.cs rename to src/ServiceControl.Api/Contracts/RetentionPurgeRequest.cs index 48a352a1bf..ec24bca4b8 100644 --- a/src/ServiceControl.Api/Contracts/RetentionSweepRequest.cs +++ b/src/ServiceControl.Api/Contracts/RetentionPurgeRequest.cs @@ -3,20 +3,20 @@ namespace ServiceControl.Api.Contracts; using System; /// -/// Request body for POST /api/retention/sweep. Both cutoffs are optional; when omitted -/// the corresponding sub-sweep derives its cutoff from the configured retention period, as the +/// Request body for POST /api/maintenance/retention/purge. Both cutoffs are optional; when omitted +/// the corresponding purge derives its cutoff from the configured retention period, as the /// scheduled hourly sweep does. A bare future-dated cutoff is rejected. /// -public class RetentionSweepRequest +public class RetentionPurgeRequest { /// - /// Cutoff applied to the failed-message sweep. null means + /// Cutoff applied to the failed-message purge. null means /// now - ErrorRetentionPeriod. /// public DateTime? ErrorCutoff { get; set; } /// - /// Cutoff applied to the event-log sweep. null means + /// Cutoff applied to the event-log purge. null means /// now - EventsRetentionPeriod. /// public DateTime? EventsCutoff { get; set; } diff --git a/src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs b/src/ServiceControl.Api/Contracts/RetentionPurgeResponse.cs similarity index 78% rename from src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs rename to src/ServiceControl.Api/Contracts/RetentionPurgeResponse.cs index f2eddd082b..f305d88deb 100644 --- a/src/ServiceControl.Api/Contracts/RetentionSweepResponse.cs +++ b/src/ServiceControl.Api/Contracts/RetentionPurgeResponse.cs @@ -3,11 +3,11 @@ namespace ServiceControl.Api.Contracts; using System; /// -/// Response body for POST /api/retention/sweep. The Status field signals the +/// Response body for POST /api/maintenance/retention/purge. The Status field signals the /// outcome: started (202), already-running (409), or /// not-supported (501). /// -public class RetentionSweepResponse +public class RetentionPurgeResponse { public string Status { get; set; } diff --git a/src/ServiceControl.Api/Contracts/RetentionSweepStatusResponse.cs b/src/ServiceControl.Api/Contracts/RetentionPurgeStatusResponse.cs similarity index 79% rename from src/ServiceControl.Api/Contracts/RetentionSweepStatusResponse.cs rename to src/ServiceControl.Api/Contracts/RetentionPurgeStatusResponse.cs index a671f17bc5..df48210c81 100644 --- a/src/ServiceControl.Api/Contracts/RetentionSweepStatusResponse.cs +++ b/src/ServiceControl.Api/Contracts/RetentionPurgeStatusResponse.cs @@ -3,10 +3,10 @@ namespace ServiceControl.Api.Contracts; using System; /// -/// Response body for GET /api/retention/sweep/status. On a persister with no sweeper +/// Response body for GET /api/maintenance/retention/purge/status. On a persister with no sweeper /// (e.g. RavenDB) the endpoint returns 501 with a instead. /// -public class RetentionSweepStatusResponse +public class RetentionPurgeStatusResponse { public bool IsRunning { get; set; } diff --git a/src/ServiceControl.Api/IRetentionApi.cs b/src/ServiceControl.Api/IRetentionApi.cs index 1915417cee..d1985c1870 100644 --- a/src/ServiceControl.Api/IRetentionApi.cs +++ b/src/ServiceControl.Api/IRetentionApi.cs @@ -5,7 +5,7 @@ namespace ServiceControl.Api; using Contracts; /// -/// Manual retention-sweep API. The implementation resolves the persister's sweeper +/// Manual retention-purge API. The implementation resolves the persister's sweeper /// optionally: when no sweeper is registered (e.g. RavenDB, which uses server-side document /// expiration) the operations report that the feature is not supported rather than silently /// failing. @@ -13,14 +13,14 @@ namespace ServiceControl.Api; public interface IRetentionApi { /// - /// Starts a manual retention sweep with caller-supplied cutoffs. The delete work runs in + /// Starts a manual retention purge with caller-supplied cutoffs. The delete work runs in /// the background on a host-lifetime token; this method returns as soon as the run is /// accepted (or refused because one is already running / unsupported). /// - Task Sweep(RetentionSweepRequest request, CancellationToken cancellationToken = default); + Task Sweep(RetentionPurgeRequest request, CancellationToken cancellationToken = default); /// - /// Returns a point-in-time snapshot of sweep execution state for polling. + /// Returns a point-in-time snapshot of purge execution state for polling. /// - Task GetStatus(CancellationToken cancellationToken = default); + Task GetStatus(CancellationToken cancellationToken = default); } \ No newline at end of file diff --git a/src/ServiceControl.Infrastructure/Auth/Permissions.cs b/src/ServiceControl.Infrastructure/Auth/Permissions.cs index b2af5c1c62..45e9cec07f 100644 --- a/src/ServiceControl.Infrastructure/Auth/Permissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/Permissions.cs @@ -58,8 +58,8 @@ public static class Permissions /// Event log area — viewing the event log. public const string ErrorEventLogView = "error:eventlog:view"; - /// Retention area — manually triggering a data retention sweep. - public const string ErrorRetentionSweep = "error:retention:sweep"; + /// Retention area — manually triggering a data retention purge. + public const string ErrorRetentionPurge = "error:retention:purge"; /// Licensing area — viewing and managing license configuration. public const string ErrorLicensingView = "error:licensing:view"; diff --git a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs index ad75a279d6..b1c273ee27 100644 --- a/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs +++ b/src/ServiceControl.Infrastructure/Auth/RolePermissions.cs @@ -63,7 +63,7 @@ public static class RolePermissions Permissions.ErrorRedirectsManage, Permissions.ErrorThroughputView, Permissions.ErrorThroughputManage, - Permissions.ErrorRetentionSweep, + Permissions.ErrorRetentionPurge, ]; public static readonly FrozenDictionary> Roles = diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs index f76be5b2af..4d9da5acc7 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs @@ -35,7 +35,7 @@ public class RetentionSweeper( // never overlap. Precedent: ExternalIntegrationRequestsDataStore.drainLock. readonly SemaphoreSlim sweepLock = new(1, 1); - // Status snapshot for GET /api/retention/sweep/status polling. Volatile reads/writes are + // Status snapshot for GET /api/maintenance/retention/purge/status polling. Volatile reads/writes are // sufficient here: the fields are written under sweepLock (or once at start) and read // lock-free for status reporting, which only needs an eventually-consistent snapshot. volatile bool isRunning; diff --git a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt index 475a694bae..69498d4d04 100644 --- a/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt +++ b/src/ServiceControl.UnitTests/ApprovalFiles/APIApprovals.HttpApiRoutes.approved.txt @@ -74,6 +74,6 @@ GET /redirects => ServiceControl.MessageRedirects.Api.MessageRedirectsController POST /redirects => ServiceControl.MessageRedirects.Api.MessageRedirectsController:NewRedirects(MessageRedirectRequest request, CancellationToken cancellationToken) DELETE /redirects/{messageRedirectId:guid} => ServiceControl.MessageRedirects.Api.MessageRedirectsController:DeleteRedirect(Guid messageRedirectId, CancellationToken cancellationToken) PUT /redirects/{messageRedirectId:guid} => ServiceControl.MessageRedirects.Api.MessageRedirectsController:UpdateRedirect(Guid messageRedirectId, MessageRedirectRequest request, CancellationToken cancellationToken) -POST /retention/sweep => ServiceControl.Retention.Api.RetentionController:Sweep(RetentionSweepRequest request, CancellationToken cancellationToken) -GET /retention/sweep/status => ServiceControl.Retention.Api.RetentionController:Status(CancellationToken cancellationToken) +POST /retention/purge => ServiceControl.Retention.Api.SystemMaintenanceController:Purge(RetentionPurgeRequest request, CancellationToken cancellationToken) +GET /retention/purge/status => ServiceControl.Retention.Api.SystemMaintenanceController:Status(CancellationToken cancellationToken) GET /sagas/{id} => ServiceControl.SagaAudit.SagasController:Sagas(PagingInfo pagingInfo, Guid id, CancellationToken cancellationToken) diff --git a/src/ServiceControl/Infrastructure/Api/RetentionApi.cs b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs index 21c0596fd9..d56615bdb0 100644 --- a/src/ServiceControl/Infrastructure/Api/RetentionApi.cs +++ b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs @@ -10,14 +10,20 @@ namespace ServiceControl.Infrastructure.Api; using ServiceControl.Api.Contracts; using RetentionSweepStatus = Persistence.RetentionSweepStatus; -// Manual retention-sweep API. The persister's IRetentionSweeper is resolved *optionally* so the +// Manual retention-purge API. The persister's IRetentionSweeper is resolved *optionally* so the // same controller/route is mapped on every persister: EFCore registers it and gets 202/409/200; // RavenDB registers nothing (its retention is the server-side @expires bundle) and gets 501. class RetentionApi(IServiceProvider serviceProvider, Settings settings) : IRetentionApi { - const string NotSupportedReason = "The currently configured storage has no retention sweeper."; + public const string NotSupportedReason = "The currently configured storage has no retention sweeper."; + public const string StatusMaintenance = "maintenance"; + public const string StatusStarted = "started"; + public const string StatusNotSupported = "not-supported"; + public const string StatusAlreadyRunning = "already-running"; + public const string StatusInvalidCutoff = "invalid-cutoff"; - public Task Sweep(RetentionSweepRequest request, CancellationToken cancellationToken = default) + + public Task Sweep(RetentionPurgeRequest request, CancellationToken cancellationToken = default) { // Resolve the sweeper lazily and optionally — never required via constructor injection, or // a RavenDB-backed instance would throw at resolve time. Absent => 501 Not Implemented. @@ -31,10 +37,10 @@ public Task Sweep(RetentionSweepRequest request, Cancell // would contend with the maintenance work. if (settings.PersisterSpecificSettings?.MaintenanceMode == true) { - return Task.FromResult(new RetentionSweepResponse { Status = "maintenance", Reason = "The instance is in maintenance mode." }); + return Task.FromResult(new RetentionPurgeResponse { Status = StatusMaintenance, Reason = "The instance is in maintenance mode." }); } - request ??= new RetentionSweepRequest(); + request ??= new RetentionPurgeRequest(); // Cutoffs must be UTC and in the past. A future cutoff would delete nothing and is almost // certainly a caller mistake, so it is rejected rather than clamped. @@ -52,39 +58,25 @@ public Task Sweep(RetentionSweepRequest request, Cancell return Task.FromResult(attempt.Outcome switch { - RetentionSweepStatus.Started => new RetentionSweepResponse - { - Status = "started", - StartedAt = attempt.StartedAt, - ErrorCutoff = attempt.ErrorCutoff, - EventsCutoff = attempt.EventsCutoff - }, - RetentionSweepStatus.AlreadyRunning => new RetentionSweepResponse - { - Status = "already-running", - StartedAt = attempt.StartedAt - }, - _ => new RetentionSweepResponse - { - Status = "already-running", - StartedAt = attempt.StartedAt - } + RetentionSweepStatus.Started => new RetentionPurgeResponse { Status = StatusStarted, StartedAt = attempt.StartedAt, ErrorCutoff = attempt.ErrorCutoff, EventsCutoff = attempt.EventsCutoff }, + RetentionSweepStatus.AlreadyRunning => new RetentionPurgeResponse { Status = StatusAlreadyRunning, StartedAt = attempt.StartedAt }, + _ => new RetentionPurgeResponse { Status = StatusAlreadyRunning, StartedAt = attempt.StartedAt } }); } - public Task GetStatus(CancellationToken cancellationToken = default) + public Task GetStatus(CancellationToken cancellationToken = default) { var sweeper = serviceProvider.GetService(); if (sweeper is null) { - return Task.FromResult(new RetentionSweepStatusResponse { Reason = NotSupportedReason }); + return Task.FromResult(new RetentionPurgeStatusResponse { Reason = NotSupportedReason }); } // Map the persister's status record onto the API contract DTO (the two share a name but // live in different namespaces: ServiceControl.Persistence vs ServiceControl.Api.Contracts). var status = sweeper.GetStatus(); - return Task.FromResult(new RetentionSweepStatusResponse + return Task.FromResult(new RetentionPurgeStatusResponse { IsRunning = status.IsRunning, LastStartedAt = status.LastStartedAt, @@ -125,15 +117,7 @@ static bool TryValidateCutoff(DateTime? supplied, out DateTime? validated, out s return true; } - static RetentionSweepResponse NotSupported() => new() - { - Status = "not-supported", - Reason = NotSupportedReason - }; + static RetentionPurgeResponse NotSupported() => new() { Status = StatusNotSupported, Reason = NotSupportedReason }; - static RetentionSweepResponse Invalid(string reason) => new() - { - Status = "invalid-cutoff", - Reason = reason - }; + static RetentionPurgeResponse Invalid(string reason) => new() { Status = StatusInvalidCutoff, Reason = reason }; } \ No newline at end of file diff --git a/src/ServiceControl/Retention/Api/RetentionController.cs b/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs similarity index 55% rename from src/ServiceControl/Retention/Api/RetentionController.cs rename to src/ServiceControl/Retention/Api/SystemMaintenanceController.cs index 05ed776fdf..5cf69900bf 100644 --- a/src/ServiceControl/Retention/Api/RetentionController.cs +++ b/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs @@ -2,44 +2,45 @@ namespace ServiceControl.Retention.Api; using System.Threading; using System.Threading.Tasks; +using Infrastructure.Api; using Infrastructure.Auth; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using ServiceControl.Api; using ServiceControl.Api.Contracts; -// Manual retention-sweep endpoint. Lives only on the primary error instance (the sweeper is only +// Manual retention-purge endpoint. Lives only on the primary error instance (the sweeper is only // registered there). On a RavenDB-backed instance IRetentionSweeper is not registered, so the // IRetentionApi implementation returns a "not-supported" status that this controller maps to 501. [ApiController] -[Route("api")] -public class RetentionController(IRetentionApi retentionApi) : ControllerBase +[Route("api/maintenance")] +public class SystemMaintenanceController(IRetentionApi retentionApi) : ControllerBase { - // Starts a full retention sweep with caller-supplied cutoffs. The delete work runs in the + // Starts a full retention purge with caller-supplied cutoffs. The delete work runs in the // background on a host-lifetime token; this returns as soon as the run is accepted (202), // already running (409), in maintenance mode (503), unsupported by the persister (501), or // the cutoff was invalid (400). - [Authorize(Policy = Permissions.ErrorRetentionSweep)] - [Route("retention/sweep")] + [Authorize(Policy = Permissions.ErrorRetentionPurge)] + [Route("retention/purge")] [HttpPost] - public async Task Sweep([FromBody] RetentionSweepRequest request, CancellationToken cancellationToken = default) + public async Task Purge([FromBody] RetentionPurgeRequest request, CancellationToken cancellationToken = default) { - var response = await retentionApi.Sweep(request ?? new RetentionSweepRequest(), cancellationToken); + var response = await retentionApi.Sweep(request ?? new RetentionPurgeRequest(), cancellationToken); return response.Status switch { - "started" => Accepted(response), - "already-running" => Conflict(response), - "maintenance" => StatusCode(503, response), - "not-supported" => StatusCode(501, response), - "invalid-cutoff" => BadRequest(response), + RetentionApi.StatusStarted => Accepted(response), + RetentionApi.StatusAlreadyRunning => Conflict(response), + RetentionApi.StatusMaintenance => StatusCode(503, response), + RetentionApi.StatusNotSupported => StatusCode(501, response), + RetentionApi.StatusInvalidCutoff => BadRequest(response), _ => Ok(response) }; } - // Polls the execution state of the most recent sweep. - [Authorize(Policy = Permissions.ErrorRetentionSweep)] - [Route("retention/sweep/status")] + // Polls the execution state of the most recent sweep/purge operation. + [Authorize(Policy = Permissions.ErrorRetentionPurge)] + [Route("retention/purge/status")] [HttpGet] public async Task Status(CancellationToken cancellationToken = default) { From 4161baf9ea9157c1fb9b72ac1515c9ad23b7008c Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Tue, 8 Sep 2026 17:06:38 +0800 Subject: [PATCH 06/10] Converted strings to enum, added audit entry --- .../Contracts/RetentionPurgeResponse.cs | 4 +- .../Contracts/RetentionPurgeStatus.cs | 9 ++++ .../Auth/MessageAction.cs | 3 +- .../Auth/MessageActionAuditLog.cs | 2 +- .../Auth/MessageActionAuditLogExtensions.cs | 33 ++++++++++++++ .../Infrastructure/Api/RetentionApi.cs | 45 +++++-------------- .../Api/SystemMaintenanceController.cs | 37 ++++++++------- 7 files changed, 81 insertions(+), 52 deletions(-) create mode 100644 src/ServiceControl.Api/Contracts/RetentionPurgeStatus.cs diff --git a/src/ServiceControl.Api/Contracts/RetentionPurgeResponse.cs b/src/ServiceControl.Api/Contracts/RetentionPurgeResponse.cs index f305d88deb..bcf9edf47a 100644 --- a/src/ServiceControl.Api/Contracts/RetentionPurgeResponse.cs +++ b/src/ServiceControl.Api/Contracts/RetentionPurgeResponse.cs @@ -9,7 +9,7 @@ namespace ServiceControl.Api.Contracts; /// public class RetentionPurgeResponse { - public string Status { get; set; } + public RetentionPurgeStatus Status { get; set; } public DateTime? StartedAt { get; set; } @@ -19,4 +19,4 @@ public class RetentionPurgeResponse /// A human-readable reason included when the operation is not supported. public string Reason { get; set; } -} +} \ No newline at end of file diff --git a/src/ServiceControl.Api/Contracts/RetentionPurgeStatus.cs b/src/ServiceControl.Api/Contracts/RetentionPurgeStatus.cs new file mode 100644 index 0000000000..c8b000d5da --- /dev/null +++ b/src/ServiceControl.Api/Contracts/RetentionPurgeStatus.cs @@ -0,0 +1,9 @@ +namespace ServiceControl.Api.Contracts; + +public enum RetentionPurgeStatus +{ + Started, + AlreadyRunning, + NotSupported, + Error +} \ No newline at end of file diff --git a/src/ServiceControl.Infrastructure/Auth/MessageAction.cs b/src/ServiceControl.Infrastructure/Auth/MessageAction.cs index 37deffca8d..f8463e5eac 100644 --- a/src/ServiceControl.Infrastructure/Auth/MessageAction.cs +++ b/src/ServiceControl.Infrastructure/Auth/MessageAction.cs @@ -7,7 +7,8 @@ public enum MessageActionKind Retry, Archive, Unarchive, - Edit + Edit, + Delete } /// How the action selected the messages it acts on. diff --git a/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLog.cs b/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLog.cs index 958724d309..48e6df9149 100644 --- a/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLog.cs +++ b/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLog.cs @@ -71,7 +71,7 @@ static string BuildEcsEvent(AuditUser user, MessageActionKind kind, string permi { kind = "event", category = new[] { "configuration" }, - type = new[] { kind == MessageActionKind.Archive ? "deletion" : "change" }, + type = new[] { kind is MessageActionKind.Archive or MessageActionKind.Delete ? "deletion" : "change" }, action = permission, outcome = success ? "success" : "failure" }, diff --git a/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLogExtensions.cs b/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLogExtensions.cs index f91aa3fb46..157c16b841 100644 --- a/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLogExtensions.cs +++ b/src/ServiceControl.Infrastructure/Auth/MessageActionAuditLogExtensions.cs @@ -39,4 +39,37 @@ public static async Task AuditedOperation(this IMessageActionAuditLog auditLog, auditLog.Operation(user, kind, permission, scope, resource, count, operationId, success); } } + + /// + /// Executes a message action and records the operation-level audit entry with the actual + /// outcome: success when the action completed, failure when it threw (the exception is + /// rethrown). Logging after the action keeps the trail truthful — an entry written before the + /// send would claim success for an operation the transport may have rejected. + /// + public static async Task AuditedOperation(this IMessageActionAuditLog auditLog, AuditUser user, + MessageActionKind kind, string permission, MessageActionScope scope, string? resource, + int? count, string operationId, Func> action, + CancellationToken cancellationToken = default) + { + var success = true; + try + { + return await action(cancellationToken).ConfigureAwait(false); + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + // A cancelled operation did not complete, so it is recorded as a failure like any other. + success = false; + throw; + } + catch + { + success = false; + throw; + } + finally + { + auditLog.Operation(user, kind, permission, scope, resource, count, operationId, success); + } + } } diff --git a/src/ServiceControl/Infrastructure/Api/RetentionApi.cs b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs index d56615bdb0..e641006dd9 100644 --- a/src/ServiceControl/Infrastructure/Api/RetentionApi.cs +++ b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs @@ -5,7 +5,6 @@ namespace ServiceControl.Infrastructure.Api; using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; using Persistence; -using ServiceBus.Management.Infrastructure.Settings; using ServiceControl.Api; using ServiceControl.Api.Contracts; using RetentionSweepStatus = Persistence.RetentionSweepStatus; @@ -13,14 +12,9 @@ namespace ServiceControl.Infrastructure.Api; // Manual retention-purge API. The persister's IRetentionSweeper is resolved *optionally* so the // same controller/route is mapped on every persister: EFCore registers it and gets 202/409/200; // RavenDB registers nothing (its retention is the server-side @expires bundle) and gets 501. -class RetentionApi(IServiceProvider serviceProvider, Settings settings) : IRetentionApi +class RetentionApi(IServiceProvider serviceProvider) : IRetentionApi { public const string NotSupportedReason = "The currently configured storage has no retention sweeper."; - public const string StatusMaintenance = "maintenance"; - public const string StatusStarted = "started"; - public const string StatusNotSupported = "not-supported"; - public const string StatusAlreadyRunning = "already-running"; - public const string StatusInvalidCutoff = "invalid-cutoff"; public Task Sweep(RetentionPurgeRequest request, CancellationToken cancellationToken = default) @@ -30,38 +24,25 @@ public Task Sweep(RetentionPurgeRequest request, Cancell var sweeper = serviceProvider.GetService(); if (sweeper is null) { - return Task.FromResult(NotSupported()); + return Task.FromResult((RetentionPurgeResponse)new() { Status = RetentionPurgeStatus.NotSupported, Reason = NotSupportedReason }); } - // Maintenance mode refuses mutating operations; a sweep while the DB is being maintained - // would contend with the maintenance work. - if (settings.PersisterSpecificSettings?.MaintenanceMode == true) - { - return Task.FromResult(new RetentionPurgeResponse { Status = StatusMaintenance, Reason = "The instance is in maintenance mode." }); - } request ??= new RetentionPurgeRequest(); - // Cutoffs must be UTC and in the past. A future cutoff would delete nothing and is almost + // Cutoffs must be UTC and in the past. A future cutoff would delete everything and is almost // certainly a caller mistake, so it is rejected rather than clamped. - if (TryValidateCutoff(request.ErrorCutoff, out var errorCutoff, out var error) is false) - { - return Task.FromResult(Invalid(error)); - } - - if (TryValidateCutoff(request.EventsCutoff, out var eventsCutoff, out error) is false) + if (!TryValidateCutoff(request.ErrorCutoff, out var errorCutoff, out var error) + || !TryValidateCutoff(request.EventsCutoff, out var eventsCutoff, out error)) { - return Task.FromResult(Invalid(error)); + return Task.FromResult((RetentionPurgeResponse)new() { Status = RetentionPurgeStatus.Error, Reason = error }); } var attempt = sweeper.TryStartManualSweep(errorCutoff, eventsCutoff, cancellationToken); - return Task.FromResult(attempt.Outcome switch - { - RetentionSweepStatus.Started => new RetentionPurgeResponse { Status = StatusStarted, StartedAt = attempt.StartedAt, ErrorCutoff = attempt.ErrorCutoff, EventsCutoff = attempt.EventsCutoff }, - RetentionSweepStatus.AlreadyRunning => new RetentionPurgeResponse { Status = StatusAlreadyRunning, StartedAt = attempt.StartedAt }, - _ => new RetentionPurgeResponse { Status = StatusAlreadyRunning, StartedAt = attempt.StartedAt } - }); + return Task.FromResult(attempt.Outcome == RetentionSweepStatus.Started + ? new RetentionPurgeResponse { Status = RetentionPurgeStatus.Started, StartedAt = attempt.StartedAt, ErrorCutoff = attempt.ErrorCutoff, EventsCutoff = attempt.EventsCutoff } + : new RetentionPurgeResponse { Status = RetentionPurgeStatus.AlreadyRunning, StartedAt = attempt.StartedAt }); } public Task GetStatus(CancellationToken cancellationToken = default) @@ -98,13 +79,15 @@ static bool TryValidateCutoff(DateTime? supplied, out DateTime? validated, out s var value = supplied.Value; - if (value.Kind != DateTimeKind.Utc) + if (value.Kind == DateTimeKind.Unspecified) { validated = null; error = "Cutoffs must be specified as UTC DateTime values."; return false; } + value = value.ToUniversalTime(); + if (value > DateTime.UtcNow) { validated = null; @@ -116,8 +99,4 @@ static bool TryValidateCutoff(DateTime? supplied, out DateTime? validated, out s error = null; return true; } - - static RetentionPurgeResponse NotSupported() => new() { Status = StatusNotSupported, Reason = NotSupportedReason }; - - static RetentionPurgeResponse Invalid(string reason) => new() { Status = StatusInvalidCutoff, Reason = reason }; } \ No newline at end of file diff --git a/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs b/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs index 5cf69900bf..f586e380d8 100644 --- a/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs +++ b/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs @@ -1,41 +1,48 @@ namespace ServiceControl.Retention.Api; +using System; using System.Threading; using System.Threading.Tasks; -using Infrastructure.Api; -using Infrastructure.Auth; +using Infrastructure.WebApi; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Mvc; using ServiceControl.Api; using ServiceControl.Api.Contracts; +using ServiceControl.Infrastructure.Auth; // Manual retention-purge endpoint. Lives only on the primary error instance (the sweeper is only // registered there). On a RavenDB-backed instance IRetentionSweeper is not registered, so the // IRetentionApi implementation returns a "not-supported" status that this controller maps to 501. [ApiController] [Route("api/maintenance")] -public class SystemMaintenanceController(IRetentionApi retentionApi) : ControllerBase +public class SystemMaintenanceController(IRetentionApi retentionApi, ICurrentUserAccessor userAccessor, IMessageActionAuditLog auditLog) : ControllerBase { // Starts a full retention purge with caller-supplied cutoffs. The delete work runs in the // background on a host-lifetime token; this returns as soon as the run is accepted (202), - // already running (409), in maintenance mode (503), unsupported by the persister (501), or - // the cutoff was invalid (400). + // already running (409), unsupported by the persister (501), or the cutoff was invalid (400). [Authorize(Policy = Permissions.ErrorRetentionPurge)] [Route("retention/purge")] [HttpPost] public async Task Purge([FromBody] RetentionPurgeRequest request, CancellationToken cancellationToken = default) { - var response = await retentionApi.Sweep(request ?? new RetentionPurgeRequest(), cancellationToken); - - return response.Status switch + var user = userAccessor.Resolve(User); + var operationId = this.AuditOperationId(); + RetentionPurgeResponse response = null; + return await auditLog.AuditedOperation(user, MessageActionKind.Delete, Permissions.ErrorRetentionPurge, MessageActionScope.Range, null, null, operationId, async ct => { - RetentionApi.StatusStarted => Accepted(response), - RetentionApi.StatusAlreadyRunning => Conflict(response), - RetentionApi.StatusMaintenance => StatusCode(503, response), - RetentionApi.StatusNotSupported => StatusCode(501, response), - RetentionApi.StatusInvalidCutoff => BadRequest(response), - _ => Ok(response) - }; + response = await retentionApi.Sweep(request ?? new RetentionPurgeRequest(), ct); + return ToActionResult(response); + }, cancellationToken); + + IActionResult ToActionResult(RetentionPurgeResponse retentionPurgeResponse) => + retentionPurgeResponse.Status switch + { + RetentionPurgeStatus.Started =>Accepted(retentionPurgeResponse), + RetentionPurgeStatus.AlreadyRunning => Conflict(retentionPurgeResponse), + RetentionPurgeStatus.NotSupported => StatusCode(501, retentionPurgeResponse), + RetentionPurgeStatus.Error =>BadRequest(retentionPurgeResponse), + _ => throw new ArgumentOutOfRangeException(nameof(retentionPurgeResponse.Status), retentionPurgeResponse.Status, "Unexpected retention purge status.") + }; } // Polls the execution state of the most recent sweep/purge operation. From aa9f5450d57347af6130043522ab0a519050df27 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Tue, 8 Sep 2026 17:08:07 +0800 Subject: [PATCH 07/10] Remove unused variable --- .../Infrastructure/RetentionSweeper.cs | 5 +---- .../RetentionSweepCurrentStatus.cs | 3 +-- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs index 4d9da5acc7..59b13b24f8 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs @@ -43,7 +43,6 @@ public class RetentionSweeper( DateTime? lastFinishedAt; DateTime? lastErrorCutoff; DateTime? lastEventsCutoff; - string? lastError; protected override async Task ExecuteAsync(CancellationToken cancellationToken = default) { @@ -98,7 +97,6 @@ public ManualSweepAttempt TryStartManualSweep(DateTime? errorCutoff, DateTime? e lastStartedAt = timeProvider.GetUtcNow().UtcDateTime; lastErrorCutoff = errorCutoff; lastEventsCutoff = eventsCutoff; - lastError = null; _ = SweepWithoutAcquiringLock(); @@ -120,7 +118,7 @@ async Task SweepWithoutAcquiringLock() } } - public RetentionSweepCurrentStatus GetStatus() => new(isRunning, lastStartedAt, lastFinishedAt, lastErrorCutoff, lastEventsCutoff, lastError); + public RetentionSweepCurrentStatus GetStatus() => new(isRunning, lastStartedAt, lastFinishedAt, lastErrorCutoff, lastEventsCutoff); async Task Sweep(DateTime? errorCutoff, DateTime? eventsCutoff, bool pace, CancellationToken cancellationToken) { @@ -129,7 +127,6 @@ async Task Sweep(DateTime? errorCutoff, DateTime? eventsCutoff, bool pace, Cance lastStartedAt = timeProvider.GetUtcNow().UtcDateTime; lastErrorCutoff = errorCutoff; lastEventsCutoff = eventsCutoff; - lastError = null; try { await SweepBody(errorCutoff, eventsCutoff, pace, cancellationToken); diff --git a/src/ServiceControl.Persistence/RetentionSweepCurrentStatus.cs b/src/ServiceControl.Persistence/RetentionSweepCurrentStatus.cs index 452204b5ff..2bc32143ea 100644 --- a/src/ServiceControl.Persistence/RetentionSweepCurrentStatus.cs +++ b/src/ServiceControl.Persistence/RetentionSweepCurrentStatus.cs @@ -8,5 +8,4 @@ public sealed record RetentionSweepCurrentStatus( DateTime? LastStartedAt, DateTime? LastFinishedAt, DateTime? LastErrorCutoff, - DateTime? LastEventsCutoff, - string? LastError); \ No newline at end of file + DateTime? LastEventsCutoff); \ No newline at end of file From 516ac6a0880d49400a98801934e4f7b8cbd46be3 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Tue, 8 Sep 2026 17:19:59 +0800 Subject: [PATCH 08/10] Remove error message from status response --- .../Contracts/RetentionPurgeStatusResponse.cs | 2 -- src/ServiceControl/Infrastructure/Api/RetentionApi.cs | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/ServiceControl.Api/Contracts/RetentionPurgeStatusResponse.cs b/src/ServiceControl.Api/Contracts/RetentionPurgeStatusResponse.cs index df48210c81..716c666fdf 100644 --- a/src/ServiceControl.Api/Contracts/RetentionPurgeStatusResponse.cs +++ b/src/ServiceControl.Api/Contracts/RetentionPurgeStatusResponse.cs @@ -18,8 +18,6 @@ public class RetentionPurgeStatusResponse public DateTime? LastEventsCutoff { get; set; } - public string LastError { get; set; } - /// Present only on the 501 Not Implemented response. public string Reason { get; set; } } \ No newline at end of file diff --git a/src/ServiceControl/Infrastructure/Api/RetentionApi.cs b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs index e641006dd9..44d3c6fa71 100644 --- a/src/ServiceControl/Infrastructure/Api/RetentionApi.cs +++ b/src/ServiceControl/Infrastructure/Api/RetentionApi.cs @@ -63,8 +63,7 @@ public Task GetStatus(CancellationToken cancellati LastStartedAt = status.LastStartedAt, LastFinishedAt = status.LastFinishedAt, LastErrorCutoff = status.LastErrorCutoff, - LastEventsCutoff = status.LastEventsCutoff, - LastError = status.LastError + LastEventsCutoff = status.LastEventsCutoff }); } From c1253efe1c4d2d645dff0007b664cf35835307c9 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Wed, 9 Sep 2026 07:53:44 +0800 Subject: [PATCH 09/10] fix formatting --- .../EFCore/RetentionSweepTests.cs | 1 - .../Retention/Api/SystemMaintenanceController.cs | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs index 7b54d53926..f20f506235 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs @@ -490,7 +490,6 @@ public async Task Manual_sweep_runs_in_the_background_and_reports_status() Assert.That(status.LastStartedAt, Is.Not.Null); Assert.That(status.LastFinishedAt, Is.Not.Null); Assert.That(status.LastErrorCutoff, Is.Not.Null); - Assert.That(status.LastError, Is.Null); } } diff --git a/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs b/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs index f586e380d8..9b2496dc30 100644 --- a/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs +++ b/src/ServiceControl/Retention/Api/SystemMaintenanceController.cs @@ -37,10 +37,10 @@ public async Task Purge([FromBody] RetentionPurgeRequest request, IActionResult ToActionResult(RetentionPurgeResponse retentionPurgeResponse) => retentionPurgeResponse.Status switch { - RetentionPurgeStatus.Started =>Accepted(retentionPurgeResponse), + RetentionPurgeStatus.Started => Accepted(retentionPurgeResponse), RetentionPurgeStatus.AlreadyRunning => Conflict(retentionPurgeResponse), RetentionPurgeStatus.NotSupported => StatusCode(501, retentionPurgeResponse), - RetentionPurgeStatus.Error =>BadRequest(retentionPurgeResponse), + RetentionPurgeStatus.Error => BadRequest(retentionPurgeResponse), _ => throw new ArgumentOutOfRangeException(nameof(retentionPurgeResponse.Status), retentionPurgeResponse.Status, "Unexpected retention purge status.") }; } From 9c92da7faab63cd63867c57db6080ba07b04f370 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Wed, 9 Sep 2026 10:28:44 +0800 Subject: [PATCH 10/10] Don't leave unobserved task exceptions, combine cancellations --- .../Infrastructure/RetentionSweeper.cs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs index 59b13b24f8..a5df9ae33b 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs @@ -104,12 +104,21 @@ public ManualSweepAttempt TryStartManualSweep(DateTime? errorCutoff, DateTime? e async Task SweepWithoutAcquiringLock() { + using var cancellation = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, hostApplicationLifetime.ApplicationStopping); try { // if the caller doesn't hand over a real cancellation token then use the application lifetime. - await SweepBody(errorCutoff, eventsCutoff, false, cancellationToken.CanBeCanceled ? cancellationToken : hostApplicationLifetime.ApplicationStopping); + await SweepBody(errorCutoff, eventsCutoff, false, cancellation.Token); lastFinishedAt = timeProvider.GetUtcNow().UtcDateTime; } + catch (OperationCanceledException) when (cancellation.Token.IsCancellationRequested) + { + //smother this exception, cancelling + } + catch (Exception e) + { + logger.LogError(e, "Error during retention sweep"); + } finally { isRunning = false;