From 427979d84c742735707767484f5c8c9fe3ff3833 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Thu, 17 Sep 2026 14:08:14 +0800 Subject: [PATCH 1/4] Add custom check for failing retention --- .../Abstractions/BasePersistence.cs | 2 + .../RetentionSweepCustomCheck.cs | 26 ++++ .../Infrastructure/RetentionSweeper.cs | 38 +++++- ...CheckTests.VerifyCustomChecks.approved.txt | 1 + ...CheckTests.VerifyCustomChecks.approved.txt | 1 + .../EFCore/RetentionSweepCustomCheckTests.cs | 113 ++++++++++++++++++ .../EFCore/RetentionSweepTests.cs | 19 +++ .../InternalCustomCheckClassification.cs | 1 + .../InternalCustomCheckClassificationTests.cs | 1 + src/ServiceControl/App.config | 6 +- 10 files changed, 202 insertions(+), 6 deletions(-) create mode 100644 src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs create mode 100644 src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepCustomCheckTests.cs diff --git a/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs b/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs index 249ac13aed..407081f8ea 100644 --- a/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs +++ b/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs @@ -47,6 +47,8 @@ protected static void RegisterDataStores(IServiceCollection services, EFPersiste services.AddSingleton(); services.AddHostedService(sp => sp.GetRequiredService()); services.AddSingleton(sp => sp.GetRequiredService()); + + services.AddCustomCheck(); } services.AddSingleton(); diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs new file mode 100644 index 0000000000..77b9c81915 --- /dev/null +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs @@ -0,0 +1,26 @@ +namespace ServiceControl.Persistence.EFCore.Infrastructure; + +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using NServiceBus.CustomChecks; + +// Fails after any retention pass fails and recovers after the next fully successful sweep. +// The standard custom-check state-transition pipeline deduplicates repeated results. +class RetentionSweepCustomCheck(RetentionSweeper sweeper) + : CustomCheck("Retention sweep", "ServiceControl Health", TimeSpan.FromMinutes(1)) +{ + public override Task PerformCheck(CancellationToken cancellationToken = default) + { + var activeFailures = sweeper.GetActiveFailures(); + + if (activeFailures.Length == 0) + { + return Task.FromResult(CheckResult.Pass); + } + + var summary = string.Join("; ", activeFailures.Select(f => $"{f.Entity}: {f.Reason}")); + return Task.FromResult(CheckResult.Failed( + $"Retention sweep has recent failures. Last failure per entity: {summary}")); + } +} \ 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 a5df9ae33b..5888f0a81f 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs @@ -31,6 +31,8 @@ public class RetentionSweeper( static readonly TimeSpan InitialDelay = TimeSpan.FromMinutes(1); static readonly TimeSpan BatchPause = TimeSpan.FromSeconds(1); + readonly Dictionary failures = []; + // 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); @@ -129,6 +131,22 @@ async Task SweepWithoutAcquiringLock() public RetentionSweepCurrentStatus GetStatus() => new(isRunning, lastStartedAt, lastFinishedAt, lastErrorCutoff, lastEventsCutoff); + internal void RecordFailure(RetentionEntity entity, string reason) + { + lock (failures) + { + failures[entity] = reason; + } + } + + internal (RetentionEntity Entity, string Reason)[] GetActiveFailures() + { + lock (failures) + { + return failures.Select(failure => (failure.Key, failure.Value)).ToArray(); + } + } + async Task Sweep(DateTime? errorCutoff, DateTime? eventsCutoff, bool pace, CancellationToken cancellationToken) { await sweepLock.WaitAsync(cancellationToken); @@ -152,28 +170,38 @@ async Task Sweep(DateTime? errorCutoff, DateTime? eventsCutoff, bool pace, Cance // 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, errorCutoff, token), cancellationToken); - await RunPass(RetentionEntity.EventLog, token => SweepEventLogItems(pace, eventsCutoff, token), cancellationToken); - await RunPass(RetentionEntity.GroupComments, SweepOrphanedGroupComments, cancellationToken); + var failedMessagesSucceeded = await RunPass(RetentionEntity.FailedMessages, token => SweepFailedMessages(pace, errorCutoff, token), cancellationToken); + var eventLogSucceeded = await RunPass(RetentionEntity.EventLog, token => SweepEventLogItems(pace, eventsCutoff, token), cancellationToken); + var groupCommentsSucceeded = await RunPass(RetentionEntity.GroupComments, SweepOrphanedGroupComments, cancellationToken); + + if (failedMessagesSucceeded && eventLogSucceeded && groupCommentsSucceeded) + { + lock (failures) + { + failures.Clear(); + } + } } // Each pass is isolated so one failing kind of row does not stop the others from being // reclaimed, and so the metrics report an outcome for every pass on every run. - async Task RunPass(RetentionEntity entity, Func pass, CancellationToken cancellationToken) + async Task RunPass(RetentionEntity entity, Func pass, CancellationToken cancellationToken) { using var cycle = metrics.BeginCycle(entity, cancellationToken); try { await pass(cancellationToken); - cycle.Complete(); + return true; } #pragma warning disable PS0019 // The filter already excludes OperationCanceledException, so // cancellation propagates; PS0019 only recognises a cancellationToken guard. catch (Exception ex) when (ex is not OperationCanceledException) { logger.LogError(ex, "Error during the {RetentionEntity} retention pass", entity); + RecordFailure(entity, ex.Message); + return false; } #pragma warning restore PS0019 } diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt b/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt index f3b0cc7462..9e307e121b 100644 --- a/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt @@ -1 +1,2 @@ +ServiceControl Health: Retention sweep Storage space: ServiceControl body storage \ No newline at end of file diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt b/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt index f3b0cc7462..9e307e121b 100644 --- a/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt +++ b/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt @@ -1 +1,2 @@ +ServiceControl Health: Retention sweep Storage space: ServiceControl body storage \ No newline at end of file diff --git a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepCustomCheckTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepCustomCheckTests.cs new file mode 100644 index 0000000000..9a7ca2beba --- /dev/null +++ b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepCustomCheckTests.cs @@ -0,0 +1,113 @@ +namespace ServiceControl.Persistence.Tests; + +using System; +using System.Linq; +using System.Threading.Tasks; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NServiceBus.CustomChecks; +using NUnit.Framework; +using ServiceControl.Contracts.CustomChecks; +using ServiceControl.CustomChecks; +using ServiceControl.Infrastructure.DomainEvents; +using ServiceControl.Operations; +using ServiceControl.Persistence.EFCore.Infrastructure; +using ServiceControl.Persistence.EFCore.Infrastructure.Metrics; + +class RetentionSweepCustomCheckTests : PersistenceTestBase +{ + RetentionSweeper Sweeper => ServiceProvider.GetRequiredService(); + + RetentionSweepCustomCheck Check => + ServiceProvider.GetServices().OfType().Single(); + + [Test] + public async Task Check_initially_passes() + { + var result = await Check.PerformCheck(); + Assert.That(result, Is.EqualTo(CheckResult.Pass)); + } + + [Test] + public async Task Check_fails_after_a_retention_failure() + { + Sweeper.RecordFailure(RetentionEntity.FailedMessages, "db timeout"); + + var result = await Check.PerformCheck(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(result.HasFailed, Is.True); + Assert.That(result.FailureReason, Does.Contain("FailedMessages")); + Assert.That(result.FailureReason, Does.Contain("db timeout")); + } + } + + [Test] + public async Task Check_passes_after_a_fully_successful_sweep() + { + Sweeper.RecordFailure(RetentionEntity.FailedMessages, "db timeout"); + + await Sweeper.SweepNow(); + + var result = await Check.PerformCheck(); + Assert.That(result, Is.EqualTo(CheckResult.Pass)); + } + + [Test] + public async Task Check_does_not_expire_with_time() + { + Sweeper.RecordFailure(RetentionEntity.FailedMessages, "db timeout"); + AdvanceClock(TimeSpan.FromHours(2)); + + var result = await Check.PerformCheck(); + Assert.That(result.HasFailed, Is.True); + } + + [Test] + public async Task Multiple_failing_entities_are_represented() + { + Sweeper.RecordFailure(RetentionEntity.FailedMessages, "body delete failed"); + Sweeper.RecordFailure(RetentionEntity.EventLog, "batch delete timeout"); + + var result = await Check.PerformCheck(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(result.HasFailed, Is.True); + Assert.That(result.FailureReason, Does.Contain("FailedMessages")); + Assert.That(result.FailureReason, Does.Contain("EventLog")); + Assert.That(result.FailureReason, Does.Contain("body delete failed")); + Assert.That(result.FailureReason, Does.Contain("batch delete timeout")); + } + } + + [Test] + public async Task Repeated_failed_results_raise_one_state_change_event() + { + Sweeper.RecordFailure(RetentionEntity.FailedMessages, "db timeout"); + + var result = await Check.PerformCheck(); + var domainEvents = (FakeDomainEvents)ServiceProvider.GetRequiredService(); + var processor = new CustomCheckResultProcessor(domainEvents, CustomChecks, NullLogger.Instance); + var detail = new CustomCheckDetail + { + Category = "ServiceControl Health", + CustomCheckId = "Retention sweep", + HasFailed = result.HasFailed, + FailureReason = result.FailureReason, + ReportedAt = Now, + OriginatingEndpoint = new EndpointDetails + { + Host = "localhost", + HostId = Guid.NewGuid(), + Name = "ServiceControl" + } + }; + + await processor.ProcessResult(detail); + await processor.ProcessResult(detail); + + Assert.That(domainEvents.RaisedEvents.OfType().Count(), Is.EqualTo(1)); + } +} \ 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 3db635707e..57b3a7bd84 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepTests.cs @@ -10,6 +10,7 @@ namespace ServiceControl.Persistence.Tests; using ServiceControl.EventLog; using ServiceControl.MessageFailures; using ServiceControl.Persistence.EFCore.Entities; +using ServiceControl.Persistence.EFCore.Infrastructure; using ServiceControl.Persistence.EFCore.Infrastructure.Metrics; using ServiceControl.Persistence.Infrastructure; @@ -308,6 +309,24 @@ public async Task A_failing_pass_does_not_stop_the_others() } } + [Test] + public async Task A_failing_pass_is_tracked() + { + // Subtracting this from the clock cannot be represented, so the failed messages pass throws + // before it reaches the database. + EFSettings.ErrorRetentionPeriod = TimeSpan.FromDays(1_000_000); + + await RunRetentionSweep(); + + var failure = ServiceProvider.GetRequiredService().GetActiveFailures().Single(); + + using (Assert.EnterMultipleScope()) + { + Assert.That(failure.Entity, Is.EqualTo(RetentionEntity.FailedMessages)); + Assert.That(failure.Reason, Is.Not.Empty); + } + } + RecordedRetentionMetrics ListenToRetentionMetrics() => new(ServiceProvider.GetRequiredService()); async Task SeedGroup(Guid uniqueMessageId) diff --git a/src/ServiceControl.Persistence/InternalCustomCheckClassification.cs b/src/ServiceControl.Persistence/InternalCustomCheckClassification.cs index 10a328db78..3f41efc7ed 100644 --- a/src/ServiceControl.Persistence/InternalCustomCheckClassification.cs +++ b/src/ServiceControl.Persistence/InternalCustomCheckClassification.cs @@ -43,6 +43,7 @@ public static class InternalCustomCheckClassification "Error Database Search Engine", // RavenDB persister "ServiceControl body storage", // EF Core persisters "Dead Letter Queue", // ASBS / IBMMQ / MSMQ + "Retention sweep", // EF Core persisters // ----- Audit instance (forwarded to the primary via ReportCustomCheckResult) ----- "Audit Message Ingestion", diff --git a/src/ServiceControl.UnitTests/CustomChecks/InternalCustomCheckClassificationTests.cs b/src/ServiceControl.UnitTests/CustomChecks/InternalCustomCheckClassificationTests.cs index c5536ad574..47d6db52b3 100644 --- a/src/ServiceControl.UnitTests/CustomChecks/InternalCustomCheckClassificationTests.cs +++ b/src/ServiceControl.UnitTests/CustomChecks/InternalCustomCheckClassificationTests.cs @@ -33,6 +33,7 @@ public void Internal_checks_are_flagged_internal() [TestCase("Error Message Ingestion")] [TestCase("Dead Letter Queue")] [TestCase("ServiceControl body storage")] + [TestCase("Retention sweep")] [TestCase("Audit Message Ingestion Process")] public void Every_shipped_check_is_internal(string id) { diff --git a/src/ServiceControl/App.config b/src/ServiceControl/App.config index 98d8fc8433..8afc7351d0 100644 --- a/src/ServiceControl/App.config +++ b/src/ServiceControl/App.config @@ -23,7 +23,9 @@ These settings are only here so that we can debug ServiceControl while developin - + + + @@ -84,6 +86,8 @@ These settings are only here so that we can debug ServiceControl while developin + + From b8bd8e89a10c8903bea78999999a606683a47f03 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Thu, 17 Sep 2026 14:24:06 +0800 Subject: [PATCH 2/4] Better naming for custom check --- .../Infrastructure/RetentionSweepCustomCheck.cs | 4 ++-- .../CustomCheckTests.VerifyCustomChecks.approved.txt | 2 +- .../CustomCheckTests.VerifyCustomChecks.approved.txt | 2 +- .../EFCore/RetentionSweepCustomCheckTests.cs | 2 +- .../InternalCustomCheckClassification.cs | 2 +- .../InternalCustomCheckClassificationTests.cs | 2 +- src/ServiceControl/App.config | 9 +++++++-- 7 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs index 77b9c81915..4af1a8d10a 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs @@ -8,7 +8,7 @@ namespace ServiceControl.Persistence.EFCore.Infrastructure; // Fails after any retention pass fails and recovers after the next fully successful sweep. // The standard custom-check state-transition pipeline deduplicates repeated results. class RetentionSweepCustomCheck(RetentionSweeper sweeper) - : CustomCheck("Retention sweep", "ServiceControl Health", TimeSpan.FromMinutes(1)) + : CustomCheck("ServiceControl Retention", "ServiceControl Health", TimeSpan.FromMinutes(1)) { public override Task PerformCheck(CancellationToken cancellationToken = default) { @@ -21,6 +21,6 @@ public override Task PerformCheck(CancellationToken cancellationTok var summary = string.Join("; ", activeFailures.Select(f => $"{f.Entity}: {f.Reason}")); return Task.FromResult(CheckResult.Failed( - $"Retention sweep has recent failures. Last failure per entity: {summary}")); + $"Retention processing has failures. Last failure per entity: {summary}")); } } \ No newline at end of file diff --git a/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt b/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt index 9e307e121b..88004c0e54 100644 --- a/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt +++ b/src/ServiceControl.Persistence.Tests.PostgreSql/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt @@ -1,2 +1,2 @@ -ServiceControl Health: Retention sweep +ServiceControl Health: ServiceControl Retention Storage space: ServiceControl body storage \ No newline at end of file diff --git a/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt b/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt index 9e307e121b..88004c0e54 100644 --- a/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt +++ b/src/ServiceControl.Persistence.Tests.SqlServer/ApprovalFiles/CustomCheckTests.VerifyCustomChecks.approved.txt @@ -1,2 +1,2 @@ -ServiceControl Health: Retention sweep +ServiceControl Health: ServiceControl Retention Storage space: ServiceControl body storage \ No newline at end of file diff --git a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepCustomCheckTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepCustomCheckTests.cs index 9a7ca2beba..a5a2261d9f 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepCustomCheckTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/RetentionSweepCustomCheckTests.cs @@ -93,7 +93,7 @@ public async Task Repeated_failed_results_raise_one_state_change_event() var detail = new CustomCheckDetail { Category = "ServiceControl Health", - CustomCheckId = "Retention sweep", + CustomCheckId = "ServiceControl Retention", HasFailed = result.HasFailed, FailureReason = result.FailureReason, ReportedAt = Now, diff --git a/src/ServiceControl.Persistence/InternalCustomCheckClassification.cs b/src/ServiceControl.Persistence/InternalCustomCheckClassification.cs index 3f41efc7ed..3c314eaed9 100644 --- a/src/ServiceControl.Persistence/InternalCustomCheckClassification.cs +++ b/src/ServiceControl.Persistence/InternalCustomCheckClassification.cs @@ -43,7 +43,7 @@ public static class InternalCustomCheckClassification "Error Database Search Engine", // RavenDB persister "ServiceControl body storage", // EF Core persisters "Dead Letter Queue", // ASBS / IBMMQ / MSMQ - "Retention sweep", // EF Core persisters + "ServiceControl Retention", // EF Core persisters // ----- Audit instance (forwarded to the primary via ReportCustomCheckResult) ----- "Audit Message Ingestion", diff --git a/src/ServiceControl.UnitTests/CustomChecks/InternalCustomCheckClassificationTests.cs b/src/ServiceControl.UnitTests/CustomChecks/InternalCustomCheckClassificationTests.cs index 47d6db52b3..6f83521d4b 100644 --- a/src/ServiceControl.UnitTests/CustomChecks/InternalCustomCheckClassificationTests.cs +++ b/src/ServiceControl.UnitTests/CustomChecks/InternalCustomCheckClassificationTests.cs @@ -33,7 +33,7 @@ public void Internal_checks_are_flagged_internal() [TestCase("Error Message Ingestion")] [TestCase("Dead Letter Queue")] [TestCase("ServiceControl body storage")] - [TestCase("Retention sweep")] + [TestCase("ServiceControl Retention")] [TestCase("Audit Message Ingestion Process")] public void Every_shipped_check_is_internal(string id) { diff --git a/src/ServiceControl/App.config b/src/ServiceControl/App.config index 8afc7351d0..bc62213e84 100644 --- a/src/ServiceControl/App.config +++ b/src/ServiceControl/App.config @@ -23,10 +23,15 @@ These settings are only here so that we can debug ServiceControl while developin - - + + + + + + + From f8041bccf2abb340718dbdddd35a1e7c9a70a36d Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Thu, 17 Sep 2026 14:38:09 +0800 Subject: [PATCH 3/4] Reset dev config --- src/ServiceControl/App.config | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/ServiceControl/App.config b/src/ServiceControl/App.config index bc62213e84..8afc7351d0 100644 --- a/src/ServiceControl/App.config +++ b/src/ServiceControl/App.config @@ -23,15 +23,10 @@ These settings are only here so that we can debug ServiceControl while developin - - + + - - - - - From c41e48bd3455b7b0977ded1d3702a12f689639f4 Mon Sep 17 00:00:00 2001 From: Rhys Bevilaqua Date: Thu, 17 Sep 2026 15:26:57 +0800 Subject: [PATCH 4/4] Add targeted interface for faking so custom check doesn't need a real retention sweeper in tests --- .../Abstractions/BasePersistence.cs | 1 + .../RetentionSweepCustomCheck.cs | 21 +++++++++---------- .../Infrastructure/RetentionSweeper.cs | 12 ++++++++++- .../FileSystemBodyStorageCustomCheckTests.cs | 6 ++++++ .../PersistenceTestBase.cs | 1 + 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs b/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs index 407081f8ea..18480bd8bc 100644 --- a/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs +++ b/src/ServiceControl.Persistence.EFCore/Abstractions/BasePersistence.cs @@ -47,6 +47,7 @@ protected static void RegisterDataStores(IServiceCollection services, EFPersiste services.AddSingleton(); services.AddHostedService(sp => sp.GetRequiredService()); services.AddSingleton(sp => sp.GetRequiredService()); + services.AddSingleton(sp => sp.GetRequiredService()); services.AddCustomCheck(); } diff --git a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs index 4af1a8d10a..7b266bfd7e 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweepCustomCheck.cs @@ -1,26 +1,25 @@ namespace ServiceControl.Persistence.EFCore.Infrastructure; -using System.Linq; using System.Threading; using System.Threading.Tasks; using NServiceBus.CustomChecks; +interface IRetentionSweepHealth +{ + string? GetFailureSummary(); +} + // Fails after any retention pass fails and recovers after the next fully successful sweep. // The standard custom-check state-transition pipeline deduplicates repeated results. -class RetentionSweepCustomCheck(RetentionSweeper sweeper) +class RetentionSweepCustomCheck(IRetentionSweepHealth retentionSweepHealth) : CustomCheck("ServiceControl Retention", "ServiceControl Health", TimeSpan.FromMinutes(1)) { public override Task PerformCheck(CancellationToken cancellationToken = default) { - var activeFailures = sweeper.GetActiveFailures(); - - if (activeFailures.Length == 0) - { - return Task.FromResult(CheckResult.Pass); - } + var failureSummary = retentionSweepHealth.GetFailureSummary(); - var summary = string.Join("; ", activeFailures.Select(f => $"{f.Entity}: {f.Reason}")); - return Task.FromResult(CheckResult.Failed( - $"Retention processing has failures. Last failure per entity: {summary}")); + return Task.FromResult(failureSummary is null + ? CheckResult.Pass + : CheckResult.Failed($"Retention processing has failures. Last failure per entity: {failureSummary}")); } } \ 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 5888f0a81f..ef2de77428 100644 --- a/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs +++ b/src/ServiceControl.Persistence.EFCore/Infrastructure/RetentionSweeper.cs @@ -24,7 +24,7 @@ public class RetentionSweeper( IBodyStoragePersistence bodyStorage, RetentionMetrics metrics, EFPersisterSettings settings, - IHostApplicationLifetime hostApplicationLifetime) : BackgroundService, IRetentionSweeper + IHostApplicationLifetime hostApplicationLifetime) : BackgroundService, IRetentionSweeper, IRetentionSweepHealth { const int BatchSize = 1000; static readonly TimeSpan Interval = TimeSpan.FromHours(1); @@ -147,6 +147,16 @@ internal void RecordFailure(RetentionEntity entity, string reason) } } + string? IRetentionSweepHealth.GetFailureSummary() + { + lock (failures) + { + return failures.Count == 0 + ? null + : string.Join("; ", failures.Select(failure => $"{failure.Key}: {failure.Value}")); + } + } + async Task Sweep(DateTime? errorCutoff, DateTime? eventsCutoff, bool pace, CancellationToken cancellationToken) { await sweepLock.WaitAsync(cancellationToken); diff --git a/src/ServiceControl.Persistence.Tests/EFCore/FileSystemBodyStorageCustomCheckTests.cs b/src/ServiceControl.Persistence.Tests/EFCore/FileSystemBodyStorageCustomCheckTests.cs index 490fb87625..8b421dd5d9 100644 --- a/src/ServiceControl.Persistence.Tests/EFCore/FileSystemBodyStorageCustomCheckTests.cs +++ b/src/ServiceControl.Persistence.Tests/EFCore/FileSystemBodyStorageCustomCheckTests.cs @@ -4,6 +4,7 @@ namespace ServiceControl.Persistence.Tests; using System.IO; using System.Linq; using System.Threading.Tasks; +using EFCore.Infrastructure; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using NServiceBus.CustomChecks; @@ -136,6 +137,7 @@ public void Is_registered_and_resolvable_for_file_system_body_storage() ConnectionString = "Server=nowhere", BodyStorage = CreateSettings(StoragePath, 15) }); + services.AddSingleton(); var check = services.BuildServiceProvider().GetServices().OfType().SingleOrDefault(); @@ -196,4 +198,8 @@ class ThrowingDriveSpaceProvider(Exception exception) : IDriveSpaceProvider } sealed class TestPersisterSettings : EFPersisterSettings; + sealed class StubRetentionHealth : IRetentionSweepHealth + { + public string GetFailureSummary() => ""; + } } diff --git a/src/ServiceControl.Persistence.Tests/PersistenceTestBase.cs b/src/ServiceControl.Persistence.Tests/PersistenceTestBase.cs index 3db037dc57..f065c190f6 100644 --- a/src/ServiceControl.Persistence.Tests/PersistenceTestBase.cs +++ b/src/ServiceControl.Persistence.Tests/PersistenceTestBase.cs @@ -34,6 +34,7 @@ public async Task SetUp() } var hostBuilder = Host.CreateApplicationBuilder(); + hostBuilder.Services.AddMetrics(); LoggerUtil.ActiveLoggers = Loggers.Test; hostBuilder.Logging.ConfigureLogging(LogLevel.Information);