Move cache instructions pruning to background job (#19598)

lauraneto · web-flow · commit dcd8b4252270 · 2025-07-01T11:17:59.000+02:00
* Remove pruning logic from `CacheInstructionService.ProcessInstructions()`

* Add and register `CacheInstructionsPruningJob` background job

* Add unit tests

* Remove breaking change in ICacheInstructionService

* Adjust some obsoletion messages to mention v17

* Added missing scope

* Update tests

* Fix obsoletion messages version

* Update ProcessInstructions methods summary
diff --git a/src/Umbraco.Core/Services/ICacheInstructionService.cs b/src/Umbraco.Core/Services/ICacheInstructionService.cs
@@ -34,14 +34,37 @@ public interface ICacheInstructionService
     void DeliverInstructionsInBatches(IEnumerable<RefreshInstruction> instructions, string localIdentity);
 
     /// <summary>
-    ///     Processes and then prunes pending database cache instructions.
+    ///     Processes pending database cache instructions.
+    /// </summary>
+    /// <param name="cacheRefreshers">Cache refreshers.</param>
+    /// <param name="cancellationToken">Cancellation token.</param>
+    /// <param name="localIdentity">Local identity of the executing AppDomain.</param>
+    /// <param name="lastId">Id of the latest processed instruction.</param>
+    /// <returns>The processing result.</returns>
+    ProcessInstructionsResult ProcessInstructions(
+        CacheRefresherCollection cacheRefreshers,
+        CancellationToken cancellationToken,
+        string localIdentity,
+        int lastId) =>
+        ProcessInstructions(
+            cacheRefreshers,
+            ServerRole.Unknown,
+            cancellationToken,
+            localIdentity,
+            lastPruned: DateTime.UtcNow,
+            lastId);
+
+    /// <summary>
+    ///     Processes pending database cache instructions.
     /// </summary>
     /// <param name="cacheRefreshers">Cache refreshers.</param>
     /// <param name="serverRole">Server role.</param>
     /// <param name="cancellationToken">Cancellation token.</param>
     /// <param name="localIdentity">Local identity of the executing AppDomain.</param>
     /// <param name="lastPruned">Date of last prune operation.</param>
-    /// <param name="lastId">Id of the latest processed instruction</param>
+    /// <param name="lastId">Id of the latest processed instruction.</param>
+    /// <returns>The processing result.</returns>
+    [Obsolete("Use the non-obsolete overload. Scheduled for removal in V17.")]
     ProcessInstructionsResult ProcessInstructions(
         CacheRefresherCollection cacheRefreshers,
         ServerRole serverRole,
diff --git a/src/Umbraco.Core/Services/ProcessInstructionsResult.cs b/src/Umbraco.Core/Services/ProcessInstructionsResult.cs
@@ -14,11 +14,13 @@ private ProcessInstructionsResult()
 
     public int LastId { get; private set; }
 
+    [Obsolete("Instruction pruning has been moved to a separate background job. Scheduled for removal in V18.")]
     public bool InstructionsWerePruned { get; private set; }
 
     public static ProcessInstructionsResult AsCompleted(int numberOfInstructionsProcessed, int lastId) =>
         new() { NumberOfInstructionsProcessed = numberOfInstructionsProcessed, LastId = lastId };
 
+    [Obsolete("Instruction pruning has been moved to a separate background job. Scheduled for removal in V18.")]
     public static ProcessInstructionsResult AsCompletedAndPruned(int numberOfInstructionsProcessed, int lastId) =>
         new()
         {
diff --git a/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/CacheInstructionsPruningJob.cs b/src/Umbraco.Infrastructure/BackgroundJobs/Jobs/CacheInstructionsPruningJob.cs
@@ -0,0 +1,61 @@
+using Microsoft.Extensions.Options;
+using Umbraco.Cms.Core.Configuration.Models;
+using Umbraco.Cms.Core.Persistence.Repositories;
+using Umbraco.Cms.Core.Scoping;
+using Umbraco.Cms.Infrastructure.Scoping;
+
+namespace Umbraco.Cms.Infrastructure.BackgroundJobs.Jobs;
+
+/// <summary>
+/// A background job that prunes cache instructions from the database.
+/// </summary>
+public class CacheInstructionsPruningJob : IRecurringBackgroundJob
+{
+    private readonly IOptions<GlobalSettings> _globalSettings;
+    private readonly ICacheInstructionRepository _cacheInstructionRepository;
+    private readonly ICoreScopeProvider _scopeProvider;
+    private readonly TimeProvider _timeProvider;
+
+    /// <summary>
+    /// Initializes a new instance of the <see cref="CacheInstructionsPruningJob"/> class.
+    /// </summary>
+    /// <param name="scopeProvider">Provides scopes for database operations.</param>
+    /// <param name="globalSettings">The global settings configuration.</param>
+    /// <param name="cacheInstructionRepository">The repository for cache instructions.</param>
+    /// <param name="timeProvider">The time provider.</param>
+    public CacheInstructionsPruningJob(
+        IOptions<GlobalSettings> globalSettings,
+        ICacheInstructionRepository cacheInstructionRepository,
+        ICoreScopeProvider scopeProvider,
+        TimeProvider timeProvider)
+    {
+        _globalSettings = globalSettings;
+        _cacheInstructionRepository = cacheInstructionRepository;
+        _scopeProvider = scopeProvider;
+        _timeProvider = timeProvider;
+        Period = globalSettings.Value.DatabaseServerMessenger.TimeBetweenPruneOperations;
+    }
+
+    /// <inheritdoc />
+    public event EventHandler PeriodChanged
+    {
+        add { }
+        remove { }
+    }
+
+    /// <inheritdoc />
+    public TimeSpan Period { get; }
+
+    /// <inheritdoc />
+    public Task RunJobAsync()
+    {
+        DateTimeOffset pruneDate = _timeProvider.GetUtcNow() - _globalSettings.Value.DatabaseServerMessenger.TimeToRetainInstructions;
+        using (ICoreScope scope = _scopeProvider.CreateCoreScope())
+        {
+            _cacheInstructionRepository.DeleteInstructionsOlderThan(pruneDate.DateTime);
+            scope.Complete();
+        }
+
+        return Task.CompletedTask;
+    }
+}
diff --git a/src/Umbraco.Infrastructure/Services/CacheInstructionService.cs b/src/Umbraco.Infrastructure/Services/CacheInstructionService.cs
@@ -122,43 +122,30 @@ public void DeliverInstructionsInBatches(IEnumerable<RefreshInstruction> instruc
             /// <inheritdoc />
             public ProcessInstructionsResult ProcessInstructions(
                 CacheRefresherCollection cacheRefreshers,
-                ServerRole serverRole,
                 CancellationToken cancellationToken,
                 string localIdentity,
-                DateTime lastPruned,
                 int lastId)
             {
                 using (!_profilingLogger.IsEnabled(Core.Logging.LogLevel.Debug) ? null : _profilingLogger.DebugDuration<CacheInstructionService>("Syncing from database..."))
                 using (ICoreScope scope = ScopeProvider.CreateCoreScope())
                 {
                     var numberOfInstructionsProcessed = ProcessDatabaseInstructions(cacheRefreshers, cancellationToken, localIdentity, ref lastId);
-
-                    // Check for pruning throttling.
-                    if (cancellationToken.IsCancellationRequested || DateTime.UtcNow - lastPruned <=
-                        _globalSettings.DatabaseServerMessenger.TimeBetweenPruneOperations)
-                    {
-                        scope.Complete();
-                        return ProcessInstructionsResult.AsCompleted(numberOfInstructionsProcessed, lastId);
-                    }
-
-                    var instructionsWerePruned = false;
-                    switch (serverRole)
-                    {
-                        case ServerRole.Single:
-                        case ServerRole.SchedulingPublisher:
-                            PruneOldInstructions();
-                            instructionsWerePruned = true;
-                            break;
-                    }
-
                     scope.Complete();
-
-                    return instructionsWerePruned
-                        ? ProcessInstructionsResult.AsCompletedAndPruned(numberOfInstructionsProcessed, lastId)
-                        : ProcessInstructionsResult.AsCompleted(numberOfInstructionsProcessed, lastId);
+                    return ProcessInstructionsResult.AsCompleted(numberOfInstructionsProcessed, lastId);
                 }
             }
 
+            /// <inheritdoc />
+            [Obsolete("Use the non-obsolete overload. Scheduled for removal in V17.")]
+            public ProcessInstructionsResult ProcessInstructions(
+                CacheRefresherCollection cacheRefreshers,
+                ServerRole serverRole,
+                CancellationToken cancellationToken,
+                string localIdentity,
+                DateTime lastPruned,
+                int lastId) =>
+                ProcessInstructions(cacheRefreshers, cancellationToken, localIdentity, lastId);
+
             private CacheInstruction CreateCacheInstruction(IEnumerable<RefreshInstruction> instructions, string localIdentity)
                 => new(
                     0,
@@ -486,21 +473,6 @@ private static IJsonCacheRefresher GetJsonRefresher(ICacheRefresher refresher)
 
                 return jsonRefresher;
             }
-
-            /// <summary>
-            ///     Remove old instructions from the database
-            /// </summary>
-            /// <remarks>
-            ///     Always leave the last (most recent) record in the db table, this is so that not all instructions are removed which
-            ///     would cause
-            ///     the site to cold boot if there's been no instruction activity for more than TimeToRetainInstructions.
-            ///     See: http://issues.umbraco.org/issue/U4-7643#comment=67-25085
-            /// </remarks>
-            private void PruneOldInstructions()
-            {
-                DateTime pruneDate = DateTime.UtcNow - _globalSettings.DatabaseServerMessenger.TimeToRetainInstructions;
-                _cacheInstructionRepository.DeleteInstructionsOlderThan(pruneDate);
-            }
         }
     }
 }
diff --git a/src/Umbraco.Infrastructure/Sync/BatchedDatabaseServerMessenger.cs b/src/Umbraco.Infrastructure/Sync/BatchedDatabaseServerMessenger.cs
@@ -16,12 +16,41 @@ namespace Umbraco.Cms.Infrastructure.Sync;
 /// </summary>
 public class BatchedDatabaseServerMessenger : DatabaseServerMessenger
 {
-    private readonly IRequestAccessor _requestAccessor;
     private readonly IRequestCache _requestCache;
 
     /// <summary>
     ///     Initializes a new instance of the <see cref="BatchedDatabaseServerMessenger" /> class.
     /// </summary>
+    public BatchedDatabaseServerMessenger(
+        IMainDom mainDom,
+        CacheRefresherCollection cacheRefreshers,
+        ILogger<BatchedDatabaseServerMessenger> logger,
+        ISyncBootStateAccessor syncBootStateAccessor,
+        IHostingEnvironment hostingEnvironment,
+        ICacheInstructionService cacheInstructionService,
+        IJsonSerializer jsonSerializer,
+        IRequestCache requestCache,
+        LastSyncedFileManager lastSyncedFileManager,
+        IOptionsMonitor<GlobalSettings> globalSettings)
+        : base(
+            mainDom,
+            cacheRefreshers,
+            logger,
+            true,
+            syncBootStateAccessor,
+            hostingEnvironment,
+            cacheInstructionService,
+            jsonSerializer,
+            lastSyncedFileManager,
+            globalSettings)
+    {
+        _requestCache = requestCache;
+    }
+
+    /// <summary>
+    ///     Initializes a new instance of the <see cref="BatchedDatabaseServerMessenger" /> class.
+    /// </summary>
+    [Obsolete("Use the non-obsolete constructor instead. Scheduled for removal in V18.")]
     public BatchedDatabaseServerMessenger(
         IMainDom mainDom,
         CacheRefresherCollection cacheRefreshers,
@@ -35,11 +64,18 @@ public BatchedDatabaseServerMessenger(
         IRequestAccessor requestAccessor,
         LastSyncedFileManager lastSyncedFileManager,
         IOptionsMonitor<GlobalSettings> globalSettings)
-        : base(mainDom, cacheRefreshers, serverRoleAccessor, logger, true, syncBootStateAccessor, hostingEnvironment,
-            cacheInstructionService, jsonSerializer, lastSyncedFileManager, globalSettings)
+        : this(
+            mainDom,
+            cacheRefreshers,
+            logger,
+            syncBootStateAccessor,
+            hostingEnvironment,
+            cacheInstructionService,
+            jsonSerializer,
+            requestCache,
+            lastSyncedFileManager,
+            globalSettings)
     {
-        _requestCache = requestCache;
-        _requestAccessor = requestAccessor;
     }
 
     /// <inheritdoc />
diff --git a/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs b/src/Umbraco.Infrastructure/Sync/DatabaseServerMessenger.cs
@@ -31,11 +31,9 @@ public abstract class DatabaseServerMessenger : ServerMessengerBase, IDisposable
      */
 
     private readonly IMainDom _mainDom;
-    private readonly IServerRoleAccessor _serverRoleAccessor;
     private readonly ISyncBootStateAccessor _syncBootStateAccessor;
     private readonly ManualResetEvent _syncIdle;
     private bool _disposedValue;
-    private DateTime _lastPruned;
     private DateTime _lastSync;
     private bool _syncing;
 
@@ -45,7 +43,6 @@ public abstract class DatabaseServerMessenger : ServerMessengerBase, IDisposable
     protected DatabaseServerMessenger(
         IMainDom mainDom,
         CacheRefresherCollection cacheRefreshers,
-        IServerRoleAccessor serverRoleAccessor,
         ILogger<DatabaseServerMessenger> logger,
         bool distributedEnabled,
         ISyncBootStateAccessor syncBootStateAccessor,
@@ -59,15 +56,14 @@ protected DatabaseServerMessenger(
         _cancellationToken = _cancellationTokenSource.Token;
         _mainDom = mainDom;
         _cacheRefreshers = cacheRefreshers;
-        _serverRoleAccessor = serverRoleAccessor;
         _hostingEnvironment = hostingEnvironment;
         Logger = logger;
         _syncBootStateAccessor = syncBootStateAccessor;
         CacheInstructionService = cacheInstructionService;
         JsonSerializer = jsonSerializer;
         _lastSyncedFileManager = lastSyncedFileManager;
         GlobalSettings = globalSettings.CurrentValue;
-        _lastPruned = _lastSync = DateTime.UtcNow;
+        _lastSync = DateTime.UtcNow;
         _syncIdle = new ManualResetEvent(true);
 
         globalSettings.OnChange(x => GlobalSettings = x);
@@ -84,6 +80,36 @@ protected DatabaseServerMessenger(
         _initialized = new Lazy<SyncBootState?>(InitializeWithMainDom);
     }
 
+    /// <summary>
+    ///     Initializes a new instance of the <see cref="DatabaseServerMessenger" /> class.
+    /// </summary>
+    [Obsolete("Use the non-obsolete constructor. Scheduled for removal in V18.")]
+    protected DatabaseServerMessenger(
+        IMainDom mainDom,
+        CacheRefresherCollection cacheRefreshers,
+        IServerRoleAccessor serverRoleAccessor,
+        ILogger<DatabaseServerMessenger> logger,
+        bool distributedEnabled,
+        ISyncBootStateAccessor syncBootStateAccessor,
+        IHostingEnvironment hostingEnvironment,
+        ICacheInstructionService cacheInstructionService,
+        IJsonSerializer jsonSerializer,
+        LastSyncedFileManager lastSyncedFileManager,
+        IOptionsMonitor<GlobalSettings> globalSettings)
+        : this(
+            mainDom,
+            cacheRefreshers,
+            logger,
+            distributedEnabled,
+            syncBootStateAccessor,
+            hostingEnvironment,
+            cacheInstructionService,
+            jsonSerializer,
+            lastSyncedFileManager,
+            globalSettings)
+    {
+    }
+
     public GlobalSettings GlobalSettings { get; private set; }
 
     protected ILogger<DatabaseServerMessenger> Logger { get; }
@@ -146,17 +172,10 @@ public override void Sync()
         {
             ProcessInstructionsResult result = CacheInstructionService.ProcessInstructions(
                 _cacheRefreshers,
-                _serverRoleAccessor.CurrentServerRole,
                 _cancellationToken,
                 LocalIdentity,
-                _lastPruned,
                 _lastSyncedFileManager.LastSyncedId);
 
-            if (result.InstructionsWerePruned)
-            {
-                _lastPruned = _lastSync;
-            }
-
             if (result.LastId > 0)
             {
                 _lastSyncedFileManager.SaveLastSyncedId(result.LastId);
diff --git a/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Common/DependencyInjection/UmbracoBuilderExtensions.cs
@@ -189,6 +189,7 @@ public static IUmbracoBuilder AddRecurringBackgroundJobs(this IUmbracoBuilder bu
         builder.Services.AddRecurringBackgroundJob<WebhookFiring>();
         builder.Services.AddRecurringBackgroundJob<WebhookLoggingCleanup>();
         builder.Services.AddRecurringBackgroundJob<ReportSiteJob>();
+        builder.Services.AddRecurringBackgroundJob<CacheInstructionsPruningJob>();
 
 
         builder.Services.AddSingleton(RecurringBackgroundJobHostedService.CreateHostedServiceFactory);
diff --git a/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/CacheInstructionServiceTests.cs b/tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Services/CacheInstructionServiceTests.cs
diff --git a/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/CacheInstructionsPruningJobTests.cs b/tests/Umbraco.Tests.UnitTests/Umbraco.Infrastructure/BackgroundJobs/Jobs/CacheInstructionsPruningJobTests.cs
