-
Notifications
You must be signed in to change notification settings - Fork 2.8k
V16: Cache Version Mechanism #19747
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
base: v16/feature/load-balancing-isolated-caches
Are you sure you want to change the base?
V16: Cache Version Mechanism #19747
Changes from all commits
0d36ca0
6e1e691
71a3506
7e277d9
1c4a8ae
82816e1
28bb9d3
905a64d
0d15c75
e315163
bc64913
6ec47fd
7e79947
f976e52
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
namespace Umbraco.Cms.Core.Cache; | ||
|
||
/// <summary> | ||
/// Provides methods to manage and validate cache versioning for repository entities, | ||
/// ensuring cache consistency with the underlying database. | ||
/// </summary> | ||
public interface IRepositoryCacheVersionService | ||
{ | ||
/// <summary> | ||
/// Validates if the cache is synced with the database. | ||
/// </summary> | ||
/// <typeparam name="TEntity">The type of the cached entity.</typeparam> | ||
/// <returns>True if cache is synced, false if cache needs fast-forwarding.</returns> | ||
Task<bool> IsCacheSyncedAsync<TEntity>() | ||
where TEntity : class; | ||
|
||
/// <summary> | ||
/// Registers a cache update for the specified entity type. | ||
/// </summary> | ||
/// <typeparam name="TEntity">The type of the cached entity.</typeparam> | ||
Task SetCacheUpdatedAsync<TEntity>() | ||
where TEntity : class; | ||
|
||
/// <summary> | ||
/// Registers that the cache has been synced with the database | ||
/// </summary> | ||
Task SetCachesSyncedAsync(); | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,118 @@ | ||||||
using System.Collections.Concurrent; | ||||||
using Microsoft.Extensions.Logging; | ||||||
using Umbraco.Cms.Core.Models; | ||||||
using Umbraco.Cms.Core.Persistence.Repositories; | ||||||
using Umbraco.Cms.Core.Scoping; | ||||||
|
||||||
namespace Umbraco.Cms.Core.Cache; | ||||||
|
||||||
/// <inheritdoc /> | ||||||
internal class RepositoryCacheVersionService : IRepositoryCacheVersionService | ||||||
{ | ||||||
private readonly ICoreScopeProvider _scopeProvider; | ||||||
private readonly IRepositoryCacheVersionRepository _repositoryCacheVersionRepository; | ||||||
private readonly ILogger<RepositoryCacheVersionService> _logger; | ||||||
private readonly ConcurrentDictionary<string, Guid> _cacheVersions = new(); | ||||||
|
||||||
public RepositoryCacheVersionService( | ||||||
ICoreScopeProvider scopeProvider, | ||||||
IRepositoryCacheVersionRepository repositoryCacheVersionRepository, | ||||||
ILogger<RepositoryCacheVersionService> logger) | ||||||
{ | ||||||
_scopeProvider = scopeProvider; | ||||||
_repositoryCacheVersionRepository = repositoryCacheVersionRepository; | ||||||
_logger = logger; | ||||||
} | ||||||
|
||||||
/// <inheritdoc /> | ||||||
public async Task<bool> IsCacheSyncedAsync<TEntity>() | ||||||
where TEntity : class | ||||||
{ | ||||||
_logger.LogDebug("Checking if cache for {EntityType} is synced", typeof(TEntity).Name); | ||||||
|
||||||
// We have to take a read lock to ensure the cache is not being updated while we check the version | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
using ICoreScope scope = _scopeProvider.CreateCoreScope(autoComplete: true); | ||||||
scope.ReadLock(Constants.Locks.CacheVersion); | ||||||
|
||||||
var cacheKey = GetCacheKey<TEntity>(); | ||||||
|
||||||
RepositoryCacheVersion? databaseVersion = await _repositoryCacheVersionRepository.GetAsync(cacheKey); | ||||||
|
||||||
if (databaseVersion?.Version is null) | ||||||
{ | ||||||
_logger.LogDebug("Cache for {EntityType} has no version in the database, considering it synced", typeof(TEntity).Name); | ||||||
|
||||||
// If the database version is null, it means the cache has never been initialized, so we consider it synced. | ||||||
return true; | ||||||
} | ||||||
|
||||||
if (_cacheVersions.TryGetValue(cacheKey, out Guid localVersion) is false) | ||||||
{ | ||||||
_logger.LogDebug("Cache for {EntityType} is not initialized, considering it synced", typeof(TEntity).Name); | ||||||
|
||||||
// We're not initialized yet, so cache is empty, which means cache is synced. | ||||||
// Since the cache is most likely no longer empty, we should set the cache version to the database version. | ||||||
_cacheVersions[cacheKey] = Guid.Parse(databaseVersion.Version); | ||||||
return true; | ||||||
} | ||||||
|
||||||
// We could've parsed this in the repository layer; however, the fact that we are using a Guid is an implementation detail. | ||||||
if (localVersion != Guid.Parse(databaseVersion.Version)) | ||||||
{ | ||||||
_logger.LogDebug( | ||||||
"Cache for {EntityType} is not synced: local version {LocalVersion} does not match database version {DatabaseVersion}", | ||||||
typeof(TEntity).Name, | ||||||
localVersion, | ||||||
databaseVersion.Version); | ||||||
return false; | ||||||
} | ||||||
|
||||||
_logger.LogDebug("Cache for {EntityType} is synced", typeof(TEntity).Name); | ||||||
return true; | ||||||
} | ||||||
|
||||||
/// <inheritdoc /> | ||||||
public async Task SetCacheUpdatedAsync<TEntity>() | ||||||
where TEntity : class | ||||||
{ | ||||||
using ICoreScope scope = _scopeProvider.CreateCoreScope(); | ||||||
|
||||||
// We have to take a write lock to ensure the cache is not being read while we update the version | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
scope.WriteLock(Constants.Locks.CacheVersion); | ||||||
|
||||||
var cacheKey = GetCacheKey<TEntity>(); | ||||||
var newVersion = Guid.NewGuid(); | ||||||
|
||||||
_logger.LogDebug("Setting cache for {EntityType} to version {Version}", typeof(TEntity).Name, newVersion); | ||||||
await _repositoryCacheVersionRepository.SaveAsync(new RepositoryCacheVersion { Identifier = cacheKey, Version = newVersion.ToString() }); | ||||||
_cacheVersions[cacheKey] = newVersion; | ||||||
|
||||||
scope.Complete(); | ||||||
} | ||||||
|
||||||
/// <inheritdoc /> | ||||||
public async Task SetCachesSyncedAsync() | ||||||
{ | ||||||
using ICoreScope scope = _scopeProvider.CreateCoreScope(); | ||||||
scope.ReadLock(Constants.Locks.CacheVersion); | ||||||
|
||||||
// We always sync all caches versions, so it's safe to assume all caches are synced at this point. | ||||||
IEnumerable<RepositoryCacheVersion> cacheVersions = await _repositoryCacheVersionRepository.GetAllAsync(); | ||||||
|
||||||
foreach (RepositoryCacheVersion version in cacheVersions) | ||||||
{ | ||||||
if (version.Version is null) | ||||||
{ | ||||||
continue; | ||||||
} | ||||||
|
||||||
_cacheVersions[version.Identifier] = Guid.Parse(version.Version); | ||||||
} | ||||||
|
||||||
scope.Complete(); | ||||||
} | ||||||
|
||||||
internal string GetCacheKey<TEntity>() | ||||||
where TEntity : class => | ||||||
typeof(TEntity).Name; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we consider full name here? Just thinking for packages, if something else has an |
||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
namespace Umbraco.Cms.Core.Cache; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should consider documentation for this - i.e. how to register it and when it should be used. Sounds like we are saying that if you don't have a load balanced setup, you could choose to register this implementation instead and get some small performance boost. |
||
|
||
/// <summary> | ||
/// A simple cache version service that assumes the cache is always in sync. | ||
/// <remarks> | ||
/// This is useful in scenarios where you have a single server setup and do not need to manage cache synchronization across multiple servers. | ||
/// </remarks> | ||
/// </summary> | ||
public class SingleServerCacheVersionService : IRepositoryCacheVersionService | ||
{ | ||
/// <inheritdoc /> | ||
public Task<bool> IsCacheSyncedAsync<TEntity>() | ||
where TEntity : class | ||
=> Task.FromResult(true); | ||
|
||
/// <inheritdoc /> | ||
public Task SetCacheUpdatedAsync<TEntity>() | ||
where TEntity : class | ||
=> Task.CompletedTask; | ||
|
||
/// <inheritdoc /> | ||
public Task SetCachesSyncedAsync() => Task.CompletedTask; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
namespace Umbraco.Cms.Core.Models; | ||
|
||
/// <summary> | ||
/// Represents a version of a repository cache. | ||
/// </summary> | ||
public class RepositoryCacheVersion | ||
{ | ||
/// <summary> | ||
/// The unique identifier for the cache. | ||
/// </summary> | ||
public required string Identifier { get; init; } | ||
|
||
/// <summary> | ||
/// The identifier of the version of the cache. | ||
/// </summary> | ||
public required string? Version { get; init; } | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -80,5 +80,10 @@ public static class Locks | |||||
/// All webhook logs. | ||||||
/// </summary> | ||||||
public const int WebhookLogs = -343; | ||||||
|
||||||
/// <summary> | ||||||
/// The cache version. | ||||||
/// </summary> | ||||||
public const int CacheVersion = -344; | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Updated just so this doesn't clash with what Laura has proposed in #19688. |
||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
using Umbraco.Cms.Core.Models; | ||
|
||
namespace Umbraco.Cms.Core.Persistence.Repositories; | ||
|
||
/// <summary> | ||
/// Defines methods for accessing and persisting <see cref="RepositoryCacheVersion"/> entities. | ||
/// </summary> | ||
public interface IRepositoryCacheVersionRepository : IRepository | ||
{ | ||
/// <summary> | ||
/// Gets a <see cref="RepositoryCacheVersion"/> by its identifier. | ||
/// </summary> | ||
/// <param name="identifier">The unique identifier of the cache version.</param> | ||
/// <returns> | ||
/// A <see cref="RepositoryCacheVersion"/> if found; otherwise, <c>null</c>. | ||
/// </returns> | ||
Task<RepositoryCacheVersion?> GetAsync(string identifier); | ||
|
||
/// <summary> | ||
/// Gets all <see cref="RepositoryCacheVersion"/> entities. | ||
/// </summary> | ||
/// <returns> | ||
/// An <see cref="IEnumerable{RepositoryCacheVersion}"/> containing all cache versions. | ||
/// </returns> | ||
Task<IEnumerable<RepositoryCacheVersion>> GetAllAsync(); | ||
|
||
/// <summary> | ||
/// Saves the specified <see cref="RepositoryCacheVersion"/>. | ||
/// </summary> | ||
/// <param name="repositoryCacheVersion">The cache version entity to save.</param> | ||
Task SaveAsync(RepositoryCacheVersion repositoryCacheVersion); | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,8 @@ | ||||||
// Copyright (c) Umbraco. | ||||||
// See LICENSE for more details. | ||||||
|
||||||
using Microsoft.Extensions.DependencyInjection; | ||||||
using Umbraco.Cms.Core.DependencyInjection; | ||||||
using Umbraco.Cms.Core.Models.Entities; | ||||||
using Umbraco.Cms.Infrastructure.Scoping; | ||||||
using Umbraco.Extensions; | ||||||
|
@@ -24,10 +26,27 @@ public class DefaultRepositoryCachePolicy<TEntity, TId> : RepositoryCachePolicyB | |||||
private static readonly TEntity[] _emptyEntities = new TEntity[0]; // const | ||||||
private readonly RepositoryCachePolicyOptions _options; | ||||||
|
||||||
public DefaultRepositoryCachePolicy(IAppPolicyCache cache, IScopeAccessor scopeAccessor, RepositoryCachePolicyOptions options) | ||||||
: base(cache, scopeAccessor) => | ||||||
public DefaultRepositoryCachePolicy( | ||||||
IAppPolicyCache cache, | ||||||
IScopeAccessor scopeAccessor, | ||||||
RepositoryCachePolicyOptions options, | ||||||
IRepositoryCacheVersionService repositoryCacheVersionService) | ||||||
: base(cache, scopeAccessor, repositoryCacheVersionService) => | ||||||
_options = options ?? throw new ArgumentNullException(nameof(options)); | ||||||
|
||||||
[Obsolete("Use the constructor with RepositoryCachePolicyOptions parameter instead.")] | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Original message mentions the wrong parameter, but I've suggested something more generic and with an indication of removal. |
||||||
public DefaultRepositoryCachePolicy( | ||||||
IAppPolicyCache cache, | ||||||
IScopeAccessor scopeAccessor, | ||||||
RepositoryCachePolicyOptions options) | ||||||
: this( | ||||||
cache, | ||||||
scopeAccessor, | ||||||
options, | ||||||
StaticServiceProvider.Instance.GetRequiredService<IRepositoryCacheVersionService>()) | ||||||
{ | ||||||
} | ||||||
|
||||||
protected string EntityTypeCacheKey { get; } = $"uRepo_{typeof(TEntity).Name}_"; | ||||||
|
||||||
/// <inheritdoc /> | ||||||
|
@@ -98,6 +117,10 @@ public override void Update(TEntity entity, Action<TEntity> persistUpdated) | |||||
|
||||||
throw; | ||||||
} | ||||||
|
||||||
// We've changed the entity, register cache change for other servers. | ||||||
// We assume that if something goes wrong, we'll roll back, so don't need to register the change. | ||||||
RegisterCacheChange(); | ||||||
} | ||||||
|
||||||
/// <inheritdoc /> | ||||||
|
@@ -122,11 +145,16 @@ public override void Delete(TEntity entity, Action<TEntity> persistDeleted) | |||||
// if there's a GetAllCacheAllowZeroCount cache, ensure it is cleared | ||||||
Cache.Clear(EntityTypeCacheKey); | ||||||
} | ||||||
|
||||||
// We've removed an entity, register cache change for other servers. | ||||||
RegisterCacheChange(); | ||||||
} | ||||||
|
||||||
/// <inheritdoc /> | ||||||
public override TEntity? Get(TId? id, Func<TId?, TEntity?> performGet, Func<TId[]?, IEnumerable<TEntity>?> performGetAll) | ||||||
{ | ||||||
EnsureCacheIsSynced(); | ||||||
|
||||||
var cacheKey = GetEntityCacheKey(id); | ||||||
|
||||||
TEntity? fromCache = Cache.GetCacheItem<TEntity>(cacheKey); | ||||||
|
@@ -163,13 +191,15 @@ public override void Delete(TEntity entity, Action<TEntity> persistDeleted) | |||||
/// <inheritdoc /> | ||||||
public override TEntity? GetCached(TId id) | ||||||
{ | ||||||
EnsureCacheIsSynced(); | ||||||
var cacheKey = GetEntityCacheKey(id); | ||||||
return Cache.GetCacheItem<TEntity>(cacheKey); | ||||||
} | ||||||
|
||||||
/// <inheritdoc /> | ||||||
public override bool Exists(TId id, Func<TId, bool> performExists, Func<TId[], IEnumerable<TEntity>?> performGetAll) | ||||||
{ | ||||||
EnsureCacheIsSynced(); | ||||||
// if found in cache the return else check | ||||||
var cacheKey = GetEntityCacheKey(id); | ||||||
TEntity? fromCache = Cache.GetCacheItem<TEntity>(cacheKey); | ||||||
|
@@ -179,6 +209,7 @@ public override bool Exists(TId id, Func<TId, bool> performExists, Func<TId[], I | |||||
/// <inheritdoc /> | ||||||
public override TEntity[] GetAll(TId[]? ids, Func<TId[]?, IEnumerable<TEntity>?> performGetAll) | ||||||
{ | ||||||
EnsureCacheIsSynced(); | ||||||
if (ids?.Length > 0) | ||||||
{ | ||||||
// try to get each entity from the cache | ||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.