Optimize document and media seeding by looking up from database in batches (#19890)

* Optimize document and media seeding by looking up from database in batches.

* Ensure null values aren't stored in the cache when checking existance.

* Fixed failing integration tests.

* Resolved issue with not writing to the L1 cache on an L2 hit.

* Tidied up and populated XML header comments.

* Address issue raised in code review.

Author: AndyButland

src/Umbraco.PublishedCache.HybridCache/Extensions/HybridCacheExtensions.cs

@@ -0,0 +1,62 @@
+using Microsoft.Extensions.Caching.Hybrid;
+
+namespace Umbraco.Cms.Infrastructure.HybridCache.Extensions;
+
+/// <summary>
+/// Provides extension methods on <see cref="Microsoft.Extensions.Caching.Hybrid.HybridCache"/>.
+/// </summary>
+internal static class HybridCacheExtensions
+{
+    /// <summary>
+    /// Returns true if the cache contains an item with a matching key.
+    /// </summary>
+    /// <param name="cache">An instance of <see cref="Microsoft.Extensions.Caching.Hybrid.HybridCache"/></param>
+    /// <param name="key">The name (key) of the item to search for in the cache.</param>
+    /// <returns>True if the item exists already. False if it doesn't.</returns>
+    /// <remarks>
+    /// Hat-tip: https://github.com/dotnet/aspnetcore/discussions/57191
+    /// Will never add or alter the state of any items in the cache.
+    /// </remarks>
+    public static async Task<bool> ExistsAsync(this Microsoft.Extensions.Caching.Hybrid.HybridCache cache, string key)
+    {
+        (bool exists, _) = await TryGetValueAsync<object>(cache, key);
+        return exists;
+    }
+
+    /// <summary>
+    /// Returns true if the cache contains an item with a matching key, along with the value of the matching cache entry.
+    /// </summary>
+    /// <typeparam name="T">The type of the value of the item in the cache.</typeparam>
+    /// <param name="cache">An instance of <see cref="Microsoft.Extensions.Caching.Hybrid.HybridCache"/></param>
+    /// <param name="key">The name (key) of the item to search for in the cache.</param>
+    /// <returns>A tuple of <see cref="bool"/> and the object (if found) retrieved from the cache.</returns>
+    /// <remarks>
+    /// Hat-tip: https://github.com/dotnet/aspnetcore/discussions/57191
+    /// Will never add or alter the state of any items in the cache.
+    /// </remarks>
+    public static async Task<(bool Exists, T? Value)> TryGetValueAsync<T>(this Microsoft.Extensions.Caching.Hybrid.HybridCache cache, string key)
+    {
+        var exists = true;
+
+        T? result = await cache.GetOrCreateAsync<object, T>(
+            key,
+            null!,
+            (_, _) =>
+            {
+                exists = false;
+                return new ValueTask<T>(default(T)!);
+            },
+            new HybridCacheEntryOptions(),
+            null,
+            CancellationToken.None);
+
+        // In checking for the existence of the item, if not found, we will have created a cache entry with a null value.
+        // So remove it again.
+        if (exists is false)
+        {
+            await cache.RemoveAsync(key);
+        }
+
+        return (exists, result);
+    }
+}

src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs

@@ -146,6 +146,26 @@ public void Rebuild(
         return CreateContentNodeKit(dto, serializer, preview);
     }
 
+    public async Task<IEnumerable<ContentCacheNode>> GetContentSourcesAsync(IEnumerable<Guid> keys, bool preview = false)
+    {
+        Sql<ISqlContext>? sql = SqlContentSourcesSelect()
+            .Append(SqlObjectTypeNotTrashed(SqlContext, Constants.ObjectTypes.Document))
+            .WhereIn<NodeDto>(x => x.UniqueId, keys)
+            .Append(SqlOrderByLevelIdSortOrder(SqlContext));
+
+        List<ContentSourceDto> dtos = await Database.FetchAsync<ContentSourceDto>(sql);
+
+        dtos = dtos
+            .Where(x => x is not null)
+            .Where(x => preview || x.PubDataRaw is not null || x.PubData is not null)
+            .ToList();
+
+        IContentCacheDataSerializer serializer =
+            _contentCacheDataSerializerFactory.Create(ContentCacheDataSerializerEntityType.Document);
+        return dtos
+            .Select(x => CreateContentNodeKit(x, serializer, preview));
+    }
+
     private IEnumerable<ContentSourceDto> GetContentSourceByDocumentTypeKey(IEnumerable<Guid> documentTypeKeys, Guid objectType)
     {
         Guid[] keys = documentTypeKeys.ToArray();
@@ -220,6 +240,25 @@ public IEnumerable<Guid> GetDocumentKeysByContentTypeKeys(IEnumerable<Guid> keys
         return CreateMediaNodeKit(dto, serializer);
     }
 
+    public async Task<IEnumerable<ContentCacheNode>> GetMediaSourcesAsync(IEnumerable<Guid> keys)
+    {
+        Sql<ISqlContext>? sql = SqlMediaSourcesSelect()
+            .Append(SqlObjectTypeNotTrashed(SqlContext, Constants.ObjectTypes.Media))
+            .WhereIn<NodeDto>(x => x.UniqueId, keys)
+            .Append(SqlOrderByLevelIdSortOrder(SqlContext));
+
+        List<ContentSourceDto> dtos = await Database.FetchAsync<ContentSourceDto>(sql);
+
+        dtos = dtos
+            .Where(x => x is not null)
+            .ToList();
+
+        IContentCacheDataSerializer serializer =
+            _contentCacheDataSerializerFactory.Create(ContentCacheDataSerializerEntityType.Media);
+        return dtos
+            .Select(x => CreateMediaNodeKit(x, serializer));
+    }
+
     private async Task OnRepositoryRefreshed(IContentCacheDataSerializer serializer, ContentCacheNode content, bool preview)
     {
         // use a custom SQL to update row version on each update

src/Umbraco.PublishedCache.HybridCache/Persistence/IDatabaseCacheRepository.cs

@@ -5,48 +5,96 @@ namespace Umbraco.Cms.Infrastructure.HybridCache.Persistence;
 
 internal interface IDatabaseCacheRepository
 {
+    /// <summary>
+    /// Deletes the specified content item from the cache database.
+    /// </summary>
     Task DeleteContentItemAsync(int id);
 
+    /// <summary>
+    /// Gets a single cache node for a document key.
+    /// </summary>
     Task<ContentCacheNode?> GetContentSourceAsync(Guid key, bool preview = false);
 
+    /// <summary>
+    /// Gets a collection of cache nodes for a collection of document keys.
+    /// </summary>
+    // TODO (V18): Remove the default implementation on this method.
+    async Task<IEnumerable<ContentCacheNode>> GetContentSourcesAsync(IEnumerable<Guid> keys, bool preview = false)
+    {
+        var contentCacheNodes = new List<ContentCacheNode>();
+        foreach (Guid key in keys)
+        {
+            ContentCacheNode? contentSource = await GetContentSourceAsync(key, preview);
+            if (contentSource is not null)
+            {
+                contentCacheNodes.Add(contentSource);
+            }
+        }
+
+        return contentCacheNodes;
+    }
+
+    /// <summary>
+    /// Gets a single cache node for a media key.
+    /// </summary>
     Task<ContentCacheNode?> GetMediaSourceAsync(Guid key);
 
+    /// <summary>
+    /// Gets a collection of cache nodes for a collection of media keys.
+    /// </summary>
+    // TODO (V18): Remove the default implementation on this method.
+    async Task<IEnumerable<ContentCacheNode>> GetMediaSourcesAsync(IEnumerable<Guid> keys)
+    {
+        var contentCacheNodes = new List<ContentCacheNode>();
+        foreach (Guid key in keys)
+        {
+            ContentCacheNode? contentSource = await GetMediaSourceAsync(key);
+            if (contentSource is not null)
+            {
+                contentCacheNodes.Add(contentSource);
+            }
+        }
 
+        return contentCacheNodes;
+    }
+
+    /// <summary>
+    /// Gets a collection of cache nodes for a collection of content type keys and entity type.
+    /// </summary>
     IEnumerable<ContentCacheNode> GetContentByContentTypeKey(IEnumerable<Guid> keys, ContentCacheDataSerializerEntityType entityType);
 
     /// <summary>
-    /// Gets all content keys of specific document types
+    /// Gets all content keys of specific document types.
     /// </summary>
     /// <param name="keys">The document types to find content using.</param>
+    /// <param name="published">A flag indicating whether to restrict to just published content.</param>
     /// <returns>The keys of all content use specific document types.</returns>
     IEnumerable<Guid> GetDocumentKeysByContentTypeKeys(IEnumerable<Guid> keys, bool published = false);
 
     /// <summary>
-    ///     Refreshes the nucache database row for the given cache node />
+    /// Refreshes the cache for the given document cache node.
     /// </summary>
-    /// <returns><placeholder>A <see cref="Task"/> representing the asynchronous operation.</placeholder></returns>
     Task RefreshContentAsync(ContentCacheNode contentCacheNode, PublishedState publishedState);
 
     /// <summary>
-    ///     Refreshes the nucache database row for the given cache node />
+    /// Refreshes the cache row for the given media cache node.
     /// </summary>
-    /// <returns><placeholder>A <see cref="Task"/> representing the asynchronous operation.</placeholder></returns>
     Task RefreshMediaAsync(ContentCacheNode contentCacheNode);
 
     /// <summary>
-    ///     Rebuilds the caches for content, media and/or members based on the content type ids specified
+    /// Rebuilds the caches for content, media and/or members based on the content type ids specified.
     /// </summary>
     /// <param name="contentTypeIds">
     ///     If not null will process content for the matching content types, if empty will process all
-    ///     content
+    ///     content.
     /// </param>
     /// <param name="mediaTypeIds">
     ///     If not null will process content for the matching media types, if empty will process all
-    ///     media
+    ///     media.
     /// </param>
     /// <param name="memberTypeIds">
     ///     If not null will process content for the matching members types, if empty will process all
-    ///     members
+    ///     members.
     /// </param>
     void Rebuild(
         IReadOnlyCollection<int>? contentTypeIds = null,

src/Umbraco.PublishedCache.HybridCache/Services/DocumentCacheService.cs

@@ -1,4 +1,8 @@
+#if DEBUG
+    using System.Diagnostics;
+#endif
 using Microsoft.Extensions.Caching.Hybrid;
+using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
 using Umbraco.Cms.Core;
 using Umbraco.Cms.Core.Models;
@@ -7,6 +11,7 @@
 using Umbraco.Cms.Core.Scoping;
 using Umbraco.Cms.Core.Services;
 using Umbraco.Cms.Core.Services.Navigation;
+using Umbraco.Cms.Infrastructure.HybridCache.Extensions;
 using Umbraco.Cms.Infrastructure.HybridCache.Factories;
 using Umbraco.Cms.Infrastructure.HybridCache.Persistence;
 using Umbraco.Cms.Infrastructure.HybridCache.Serialization;
@@ -26,8 +31,8 @@ internal sealed class DocumentCacheService : IDocumentCacheService
     private readonly IPublishedModelFactory _publishedModelFactory;
     private readonly IPreviewService _previewService;
     private readonly IPublishStatusQueryService _publishStatusQueryService;
-    private readonly IDocumentNavigationQueryService _documentNavigationQueryService;
     private readonly CacheSettings _cacheSettings;
+    private readonly ILogger<DocumentCacheService> _logger;
     private HashSet<Guid>? _seedKeys;
 
     private HashSet<Guid> SeedKeys
@@ -62,7 +67,7 @@ public DocumentCacheService(
         IPublishedModelFactory publishedModelFactory,
         IPreviewService previewService,
         IPublishStatusQueryService publishStatusQueryService,
-        IDocumentNavigationQueryService documentNavigationQueryService)
+        ILogger<DocumentCacheService> logger)
     {
         _databaseCacheRepository = databaseCacheRepository;
         _idKeyMap = idKeyMap;
@@ -74,8 +79,8 @@ public DocumentCacheService(
         _publishedModelFactory = publishedModelFactory;
         _previewService = previewService;
         _publishStatusQueryService = publishStatusQueryService;
-        _documentNavigationQueryService = documentNavigationQueryService;
         _cacheSettings = cacheSettings.Value;
+        _logger = logger;
     }
 
     public async Task<IPublishedContent?> GetByKeyAsync(Guid key, bool? preview = null)
@@ -185,44 +190,64 @@ public async Task RemoveFromMemoryCacheAsync(Guid key)
 
     public async Task SeedAsync(CancellationToken cancellationToken)
     {
-        foreach (Guid key in SeedKeys)
+#if DEBUG
+        var sw = new Stopwatch();
+        sw.Start();
+#endif
+
+        const int GroupSize = 100;
+        foreach (IEnumerable<Guid> group in SeedKeys.InGroupsOf(GroupSize))
         {
-            if (cancellationToken.IsCancellationRequested)
+            var uncachedKeys = new HashSet<Guid>();
+            foreach (Guid key in group)
             {
-                break;
-            }
+                if (cancellationToken.IsCancellationRequested)
+                {
+                    break;
+                }
 
-            var cacheKey = GetCacheKey(key, false);
+                var cacheKey = GetCacheKey(key, false);
 
-            // We'll use GetOrCreateAsync because it may be in the second level cache, in which case we don't have to re-seed.
-            ContentCacheNode? cachedValue = await _hybridCache.GetOrCreateAsync(
-                cacheKey,
-                async cancel =>
+                var existsInCache = await _hybridCache.ExistsAsync(cacheKey);
+                if (existsInCache is false)
                 {
-                    using ICoreScope scope = _scopeProvider.CreateCoreScope();
+                    uncachedKeys.Add(key);
+                }
+            }
+
+            _logger.LogDebug("Uncached key count {KeyCount}", uncachedKeys.Count);
+
+            if (uncachedKeys.Count == 0)
+            {
+                continue;
+            }
 
-                    ContentCacheNode? cacheNode = await _databaseCacheRepository.GetContentSourceAsync(key);
+            using ICoreScope scope = _scopeProvider.CreateCoreScope();
 
-                    scope.Complete();
+            IEnumerable<ContentCacheNode> cacheNodes = await _databaseCacheRepository.GetContentSourcesAsync(uncachedKeys);
 
-                    // We don't want to seed drafts
-                    if (cacheNode is null || cacheNode.IsDraft)
-                    {
-                        return null;
-                    }
+            scope.Complete();
 
-                    return cacheNode;
-                },
-                GetSeedEntryOptions(),
-                GenerateTags(key),
-                cancellationToken: cancellationToken);
+            _logger.LogDebug("Document nodes to cache {NodeCount}", cacheNodes.Count());
 
-            // If the value is null, it's likely because
-            if (cachedValue is null)
+            foreach (ContentCacheNode cacheNode in cacheNodes)
             {
-                await _hybridCache.RemoveAsync(cacheKey, cancellationToken);
+                var cacheKey = GetCacheKey(cacheNode.Key, false);
+                await _hybridCache.SetAsync(
+                    cacheKey,
+                    cacheNode,
+                    GetSeedEntryOptions(),
+                    GenerateTags(cacheNode.Key),
+                    cancellationToken: cancellationToken);
             }
         }
+
+#if DEBUG
+        sw.Stop();
+        _logger.LogInformation("Document cache seeding completed in {ElapsedMilliseconds} ms with {SeedCount} seed keys.", sw.ElapsedMilliseconds, SeedKeys.Count);
+#else
+        _logger.LogInformation("Document cache seeding completed with {SeedCount} seed keys.", SeedKeys.Count);
+#endif
     }
 
     // Internal for test purposes.
@@ -256,16 +281,7 @@ public async Task<bool> HasContentByIdAsync(int id, bool preview = false)
             return false;
         }
 
-        ContentCacheNode? contentCacheNode = await _hybridCache.GetOrCreateAsync<ContentCacheNode?>(
-            GetCacheKey(keyAttempt.Result, preview), // Unique key to the cache entry
-            cancel => ValueTask.FromResult<ContentCacheNode?>(null));
-
-        if (contentCacheNode is null)
-        {
-            await _hybridCache.RemoveAsync(GetCacheKey(keyAttempt.Result, preview));
-        }
-
-        return contentCacheNode is not null;
+        return await _hybridCache.ExistsAsync(GetCacheKey(keyAttempt.Result, preview));
     }
 
     public async Task RefreshContentAsync(IContent content)