V9/tmp nucache lock (#12149)

* wrapping SetAllFastSortedLocked in try catch

* Potential issue mitigation by retrieving all data from the database at once instead of one at a time while populating the NuCache file.

* Moved content retrieval to within the try-catch block.

* using InGroupsOf() to retrieve content without loading absolutely everything into memory.

* added "old" method signatures to prevent breaking change in ContentStore

* Revert code style cleanups for clarity

Co-authored-by: Sebastiaan Janssen <sebastiaan@umbraco.com>

src/Umbraco.Core/Configuration/Models/NuCacheSettings.cs

@@ -13,6 +13,7 @@ namespace Umbraco.Cms.Core.Configuration.Models
     {
         internal const string StaticNuCacheSerializerType = "MessagePack";
         internal const int StaticSqlPageSize = 1000;
+        internal const int StaticKitBatchSize = 1;
 
         /// <summary>
         /// Gets or sets a value defining the BTree block size.
@@ -31,6 +32,12 @@ namespace Umbraco.Cms.Core.Configuration.Models
         [DefaultValue(StaticSqlPageSize)]
         public int SqlPageSize { get; set; } = StaticSqlPageSize;
 
+        /// <summary>
+        /// The size to use for nucache Kit batches.  Higher value means more content loaded into memory at a time.
+        /// </summary>
+        [DefaultValue(StaticKitBatchSize)]
+        public int KitBatchSize { get; set; } = StaticKitBatchSize;
+
         public bool UnPublishedContentCompression { get; set; } = false;
     }
 }

src/Umbraco.PublishedCache.NuCache/ContentStore.cs

@@ -11,6 +11,7 @@ using Umbraco.Cms.Core.Models.PublishedContent;
 using Umbraco.Cms.Core.PublishedCache;
 using Umbraco.Cms.Core.Scoping;
 using Umbraco.Cms.Infrastructure.PublishedCache.Snap;
+using Umbraco.Extensions;
 
 namespace Umbraco.Cms.Infrastructure.PublishedCache
 {
@@ -696,7 +697,36 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache
         /// <exception cref="InvalidOperationException">
         /// Thrown if this method is not called within a write lock
         /// </exception>
+        [Obsolete("Use the overload that takes a 'kitGroupSize' parameter instead")]
         public bool SetAllFastSortedLocked(IEnumerable<ContentNodeKit> kits, bool fromDb)
+        {
+            return SetAllFastSortedLocked(kits, 1, fromDb);
+        }
+
+        /// <summary>
+        /// Builds all kits on startup using a fast forward only cursor
+        /// </summary>
+        /// <param name="kits">
+        /// All kits sorted by Level + Parent Id + Sort order
+        /// </param>
+        /// <param name="kitGroupSize"></param>
+        /// <param name="fromDb">True if the data is coming from the database (not the local cache db)</param>
+        /// <returns></returns>
+        /// <remarks>
+        /// <para>
+        /// This requires that the collection is sorted by Level + ParentId + Sort Order.
+        /// This should be used only on a site startup as the first generations.
+        /// This CANNOT be used after startup since it bypasses all checks for Generations.
+        /// </para>
+        /// <para>
+        /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock
+        /// otherwise an exception will occur.
+        /// </para>
+        /// </remarks>
+        /// <exception cref="InvalidOperationException">
+        /// Thrown if this method is not called within a write lock
+        /// </exception>
+        public bool SetAllFastSortedLocked(IEnumerable<ContentNodeKit> kits, int kitGroupSize, bool fromDb)
         {
             EnsureLocked();
 
@@ -714,7 +744,19 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache
             ContentNode previousNode = null;
             ContentNode parent = null;
 
-            foreach (var kit in kits)
+            // By using InGroupsOf() here we are forcing the database query result extraction to retrieve items in batches,
+            // reducing the possibility of a database timeout (ThreadAbortException) on large datasets.
+            // This in turn reduces the possibility that the NuCache file will remain locked, because an exception
+            // here results in the calling method to not release the lock.
+
+            // However the larger the batck size, the more content loaded into memory.  So by default, this is set to 1 and can be increased by setting
+            // the configuration setting Umbraco:CMS:NuCache:KitPageSize to a higher value.
+
+            // If we are not loading from the database, then we can ignore this restriction.
+
+            foreach (var kitGroup in kits.InGroupsOf(!fromDb || kitGroupSize < 1 ? 1 : kitGroupSize))
+            {
+                foreach (var kit in kitGroup)
                 {
                     if (!BuildKit(kit, out var parentLink))
                     {
@@ -763,6 +805,7 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache
 
                     _contentKeyToIdMap[kit.Node.Uid] = kit.Node.Id;
                 }
+            }
 
             return ok;
         }
@@ -779,7 +822,27 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache
         /// <exception cref="InvalidOperationException">
         /// Thrown if this method is not called within a write lock
         /// </exception>
+        [Obsolete("Use the overload that takes the 'kitGroupSize' and 'fromDb' parameters instead")]
         public bool SetAllLocked(IEnumerable<ContentNodeKit> kits)
+        {
+            return SetAllLocked(kits, 1, false);
+        }
+
+        /// <summary>
+        /// Set all data for a collection of <see cref="ContentNodeKit"/>
+        /// </summary>
+        /// <param name="kits"></param>
+        /// <param name="kitGroupSize"></param>
+        /// <param name="fromDb">True if the data is coming from the database (not the local cache db)</param>
+        /// <returns></returns>
+        /// <remarks>
+        /// This methods MUST be called from within a write lock, normally wrapped within GetScopedWriteLock
+        /// otherwise an exception will occur.
+        /// </remarks>
+        /// <exception cref="InvalidOperationException">
+        /// Thrown if this method is not called within a write lock
+        /// </exception>
+        public bool SetAllLocked(IEnumerable<ContentNodeKit> kits, int kitGroupSize, bool fromDb)
         {
             EnsureLocked();
 
@@ -792,7 +855,18 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache
             //ClearLocked(_contentTypesById);
             //ClearLocked(_contentTypesByAlias);
 
-            foreach (var kit in kits)
+            // By using InGroupsOf() here we are forcing the database query result extraction to retrieve items in batches,
+            // reducing the possibility of a database timeout (ThreadAbortException) on large datasets.
+            // This in turn reduces the possibility that the NuCache file will remain locked, because an exception
+            // here results in the calling method to not release the lock.
+
+            // However the larger the batck size, the more content loaded into memory.  So by default, this is set to 1 and can be increased by setting
+            // the configuration setting Umbraco:CMS:NuCache:KitPageSize to a higher value.
+
+            // If we are not loading from the database, then we can ignore this restriction.
+            foreach (var kitGroup in kits.InGroupsOf(!fromDb || kitGroupSize < 1 ? 1 : kitGroupSize))
+            {
+                foreach (var kit in kitGroup)
                 {
                     if (!BuildKit(kit, out var parent))
                     {
@@ -812,6 +886,7 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache
 
                     _contentKeyToIdMap[kit.Node.Uid] = kit.Node.Id;
                 }
+            }
 
             return ok;
         }

src/Umbraco.PublishedCache.NuCache/PublishedSnapshotService.cs

@@ -337,8 +337,19 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache
                 _localContentDb?.Clear();
 
                 // IMPORTANT GetAllContentSources sorts kits by level + parentId + sortOrder
+
+                try
+                {
                     var kits = _publishedContentService.GetAllContentSources();
-                return onStartup ? _contentStore.SetAllFastSortedLocked(kits, true) : _contentStore.SetAllLocked(kits);
+                    return onStartup ? _contentStore.SetAllFastSortedLocked(kits, _config.KitBatchSize, true) : _contentStore.SetAllLocked(kits, _config.KitBatchSize, true);
+                }
+                catch (ThreadAbortException tae)
+                {
+                    // Caught a ThreadAbortException, most likely from a database timeout.
+                    // If we don't catch it here, the whole local cache can remain locked causing widespread panic (see above comment).
+                    _logger.LogWarning(tae, tae.Message);
+                }
+                return false;
             }
         }
 
@@ -385,8 +396,20 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache
 
                 _logger.LogDebug("Loading media from database...");
                 // IMPORTANT GetAllMediaSources sorts kits by level + parentId + sortOrder
+
+                try
+                {
+
                     var kits = _publishedContentService.GetAllMediaSources();
-                return onStartup ? _mediaStore.SetAllFastSortedLocked(kits, true) : _mediaStore.SetAllLocked(kits);
+                    return onStartup ? _mediaStore.SetAllFastSortedLocked(kits, _config.KitBatchSize, true) : _mediaStore.SetAllLocked(kits, _config.KitBatchSize, true);
+                }
+                catch (ThreadAbortException tae)
+                {
+                    // Caught a ThreadAbortException, most likely from a database timeout.
+                    // If we don't catch it here, the whole local cache can remain locked causing widespread panic (see above comment).
+                    _logger.LogWarning(tae, tae.Message);
+                }
+                return false;
             }
         }
 
@@ -434,7 +457,7 @@ namespace Umbraco.Cms.Infrastructure.PublishedCache
                 return false;
             }
 
-            return onStartup ? store.SetAllFastSortedLocked(kits, false) : store.SetAllLocked(kits);
+            return onStartup ? store.SetAllFastSortedLocked(kits, _config.KitBatchSize, false) : store.SetAllLocked(kits, _config.KitBatchSize, false);
         }
 
         private void LockAndLoadDomains()