Skip to content

Move database cache rebuild to a background task with polling (13) #18922

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions src/Umbraco.Core/PublishedCache/IPublishedSnapshotService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ public interface IPublishedSnapshotService : IDisposable
/// </remarks>
IPublishedSnapshot CreatePublishedSnapshot(string? previewToken);

/// <summary>
/// Indicates if the database cache is in the process of being rebuilt.
/// </summary>
/// <returns></returns>
bool IsRebuilding() => false;

/// <summary>
/// Rebuilds internal database caches (but does not reload).
/// </summary>
Expand Down Expand Up @@ -61,6 +67,38 @@ void Rebuild(
IReadOnlyCollection<int>? mediaTypeIds = null,
IReadOnlyCollection<int>? memberTypeIds = null);

/// <summary>
/// Rebuilds internal database caches (but does not reload).
/// </summary>
/// <param name="contentTypeIds">
/// If not null will process content for the matching content types, if empty will process all
/// content
/// </param>
/// <param name="mediaTypeIds">
/// If not null will process content for the matching media types, if empty will process all
/// media
/// </param>
/// <param name="memberTypeIds">
/// If not null will process content for the matching members types, if empty will process all
/// members
/// </param>
/// <param name="useBackgroundThread">Flag indicating whether to use a background thread for the operation and immediately return to the caller.</param>
/// <remarks>
/// <para>
/// Forces the snapshot service to rebuild its internal database caches. For instance, some caches
/// may rely on a database table to store pre-serialized version of documents.
/// </para>
/// <para>
/// This does *not* reload the caches. Caches need to be reloaded, for instance via
/// <see cref="DistributedCache" /> RefreshAllPublishedSnapshot method.
/// </para>
/// </remarks>
void Rebuild(
bool useBackgroundThread,
IReadOnlyCollection<int>? contentTypeIds = null,
IReadOnlyCollection<int>? mediaTypeIds = null,
IReadOnlyCollection<int>? memberTypeIds = null) => Rebuild(contentTypeIds, mediaTypeIds, memberTypeIds);


/// <summary>
/// Rebuilds all internal database caches (but does not reload).
Expand All @@ -77,6 +115,22 @@ void Rebuild(
/// </remarks>
void RebuildAll() => Rebuild(Array.Empty<int>(), Array.Empty<int>(), Array.Empty<int>());

/// <summary>
/// Rebuilds all internal database caches (but does not reload).
/// </summary>
/// <param name="useBackgroundThread">Flag indicating whether to use a background thread for the operation and immediately return to the caller.</param>
/// <remarks>
/// <para>
/// Forces the snapshot service to rebuild its internal database caches. For instance, some caches
/// may rely on a database table to store pre-serialized version of documents.
/// </para>
/// <para>
/// This does *not* reload the caches. Caches need to be reloaded, for instance via
/// <see cref="DistributedCache" /> RefreshAllPublishedSnapshot method.
/// </para>
/// </remarks>
void RebuildAll(bool useBackgroundThread) => Rebuild(useBackgroundThread, Array.Empty<int>(), Array.Empty<int>(), Array.Empty<int>());

/* An IPublishedCachesService implementation can rely on transaction-level events to update
* its internal, database-level data, as these events are purely internal. However, it cannot
* rely on cache refreshers CacheUpdated events to update itself, as these events are external
Expand Down
68 changes: 66 additions & 2 deletions src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using CSharpTest.Net.Collections;

Check notice on line 1 in src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

✅ Getting better: Primitive Obsession

The ratio of primitive types in function arguments decreases from 33.33% to 30.77%, threshold = 30.0%. The functions in this file have too many primitive types (e.g. int, double, float) in their function argument lists. Using many primitive types lead to the code smell Primitive Obsession. Avoid adding more primitive arguments.
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Umbraco.Cms.Core;
Expand All @@ -15,6 +15,7 @@
using Umbraco.Cms.Core.Services;
using Umbraco.Cms.Core.Services.Changes;
using Umbraco.Cms.Core.Sync;
using Umbraco.Cms.Infrastructure.HostedServices;
using Umbraco.Cms.Infrastructure.PublishedCache.DataSource;
using Umbraco.Cms.Infrastructure.PublishedCache.Persistence;
using Umbraco.Extensions;
Expand All @@ -31,6 +32,9 @@
// means faster execution, but uses memory - not sure if we want it
// so making it configurable.
public static readonly bool FullCacheWhenPreviewing = true;

private const string IsRebuildingDatabaseCacheRuntimeCacheKey = "temp_database_cache_rebuild_op";

private readonly NuCacheSettings _config;
private readonly ContentDataSerializer _contentDataSerializer;
private readonly IDefaultCultureAccessor _defaultCultureAccessor;
Expand All @@ -51,6 +55,8 @@
private readonly object _storesLock = new();
private readonly ISyncBootStateAccessor _syncBootStateAccessor;
private readonly IVariationContextAccessor _variationContextAccessor;
private readonly IBackgroundTaskQueue _backgroundTaskQueue;
private readonly IAppPolicyCache _runtimeCache;

private long _contentGen;

Expand Down Expand Up @@ -91,7 +97,9 @@
IPublishedModelFactory publishedModelFactory,
IHostingEnvironment hostingEnvironment,
IOptions<NuCacheSettings> config,
ContentDataSerializer contentDataSerializer)
ContentDataSerializer contentDataSerializer,
IBackgroundTaskQueue backgroundTaskQueue,
AppCaches appCaches)
{
_options = options;
_syncBootStateAccessor = syncBootStateAccessor;
Expand All @@ -111,6 +119,8 @@
_contentDataSerializer = contentDataSerializer;
_config = config.Value;
_publishedModelFactory = publishedModelFactory;
_backgroundTaskQueue = backgroundTaskQueue;
_runtimeCache = appCaches.RuntimeCache;
}

protected PublishedSnapshot? CurrentPublishedSnapshot
Expand Down Expand Up @@ -349,12 +359,66 @@
return new PublishedSnapshot(this, preview);
}

/// <inheritdoc />
public bool IsRebuilding() => _runtimeCache.Get(IsRebuildingDatabaseCacheRuntimeCacheKey) is not null;

/// <inheritdoc />
public void Rebuild(
IReadOnlyCollection<int>? contentTypeIds = null,
IReadOnlyCollection<int>? mediaTypeIds = null,
IReadOnlyCollection<int>? memberTypeIds = null)
=> _publishedContentService.Rebuild(contentTypeIds, mediaTypeIds, memberTypeIds);
=> Rebuild(false, contentTypeIds, mediaTypeIds, memberTypeIds);

/// <inheritdoc />
public void Rebuild(
bool useBackgroundThread,
IReadOnlyCollection<int>? contentTypeIds = null,
IReadOnlyCollection<int>? mediaTypeIds = null,
IReadOnlyCollection<int>? memberTypeIds = null)
{
if (useBackgroundThread)
{
_logger.LogInformation("Starting async background thread for rebuilding database cache.");

_backgroundTaskQueue.QueueBackgroundWorkItem(
cancellationToken =>
{
// Do not flow AsyncLocal to the child thread
using (ExecutionContext.SuppressFlow())
{
Task.Run(() => PerformRebuild(contentTypeIds, mediaTypeIds, memberTypeIds));

// immediately return so the request isn't waiting.
return Task.CompletedTask;
}
});
}
else
{
PerformRebuild(contentTypeIds, mediaTypeIds, memberTypeIds);
}
}

private void PerformRebuild(
IReadOnlyCollection<int>? contentTypeIds = null,
IReadOnlyCollection<int>? mediaTypeIds = null,
IReadOnlyCollection<int>? memberTypeIds = null)
{
try
{
SetIsRebuilding();

_publishedContentService.Rebuild(contentTypeIds, mediaTypeIds, memberTypeIds);
}
finally
{
ClearIsRebuilding();
}
}

private void SetIsRebuilding() => _runtimeCache.Insert(IsRebuildingDatabaseCacheRuntimeCacheKey, () => "tempValue", TimeSpan.FromMinutes(10));

private void ClearIsRebuilding() => _runtimeCache.Clear(IsRebuildingDatabaseCacheRuntimeCacheKey);

public async Task CollectAsync()
{
Expand Down
5 changes: 5 additions & 0 deletions src/Umbraco.PublishedCache.NuCache/PublishedSnapshotStatus.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,11 @@
$"The current {typeof(IPublishedSnapshotService)} is not the default type. A status cannot be determined.";
}

if (_service.IsRebuilding())
{
return "Rebuild in progress. Please wait.";
}

Check warning on line 36 in src/Umbraco.PublishedCache.NuCache/PublishedSnapshotStatus.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

❌ Getting worse: Complex Method

GetStatus increases in cyclomatic complexity from 11 to 12, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
// TODO: This should be private
_service.EnsureCaches();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,32 @@ public PublishedSnapshotCacheStatusController(
[HttpPost]
public string RebuildDbCache()
{
//Rebuild All
if (_publishedSnapshotService.IsRebuilding())
{
return "Rebuild already in progress.";
}

_publishedSnapshotService.RebuildAll();
return _publishedSnapshotStatus.GetStatus();
}

/// <summary>
/// Gets a status report
/// Rebuilds the Database cache on a background thread.
/// </summary>
[HttpPost]
public IActionResult RebuildDbCacheInBackground()
{
if (_publishedSnapshotService.IsRebuilding())
{
return BadRequest("Rebuild already in progress.");
}

_publishedSnapshotService.RebuildAll(true);
return Ok();
}

/// <summary>
/// Gets a status report.
/// </summary>
[HttpGet]
public string GetStatus() => _publishedSnapshotStatus.GetStatus();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
function publishedSnapshotCacheController($scope, $http, umbRequestHelper, localizationService, overlayService) {
function publishedSnapshotCacheController($scope, $http, umbRequestHelper, localizationService, overlayService) {

Check notice on line 1 in src/Umbraco.Web.UI.Client/src/views/dashboard/settings/publishedsnapshotcache.controller.js

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (v13/dev)

✅ Getting better: Code Duplication

reduced similar code in: publishedSnapshotCacheController.performRebuild,publishedSnapshotCacheController.performReload. Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.

var vm = this;

Expand Down Expand Up @@ -94,12 +94,23 @@
vm.working = true;

umbRequestHelper.resourcePromise(
$http.post(umbRequestHelper.getApiUrl("publishedSnapshotCacheStatusBaseUrl", "RebuildDbCache")),
'Failed to rebuild the cache.')
.then(function (result) {
vm.working = false;
vm.status = result;
});
$http.post(umbRequestHelper.getApiUrl("publishedSnapshotCacheStatusBaseUrl", "RebuildDbCacheInBackground")), "Failed to queue the rebuild task.")
.then(function () {
const interval = setInterval(function () {
$http.get(umbRequestHelper.getApiUrl("publishedSnapshotCacheStatusBaseUrl", "GetStatus"))
.then(function (result) {
if (!result.data.toString().startsWith("Rebuild in progress")) {
vm.working = false;
vm.status = result.data;
clearInterval(interval);
}
}, function () {
vm.working = false;
vm.status = "Could not retrieve rebuild cache status";
});

}, 2000);
});
}

function init() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.Events;
using Umbraco.Cms.Core.Hosting;
Expand All @@ -22,6 +23,7 @@
using Umbraco.Cms.Core.Strings;
using Umbraco.Cms.Core.Sync;
using Umbraco.Cms.Core.Web;
using Umbraco.Cms.Infrastructure.HostedServices;
using Umbraco.Cms.Infrastructure.PublishedCache;
using Umbraco.Cms.Infrastructure.PublishedCache.DataSource;
using Umbraco.Cms.Infrastructure.Serialization;
Expand Down Expand Up @@ -280,7 +282,9 @@ protected void InitializedCache(
PublishedModelFactory,
TestHelper.GetHostingEnvironment(),
Options.Create(nuCacheSettings),
new ContentDataSerializer(new DictionaryOfPropertyDataSerializer()));
new ContentDataSerializer(new DictionaryOfPropertyDataSerializer()),
Mock.Of<IBackgroundTaskQueue>(),
AppCaches.NoCache);

// invariant is the current default
VariationContextAccessor.VariationContext = new VariationContext();
Expand Down
Loading