Skip to content
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
22 changes: 22 additions & 0 deletions src/Umbraco.Core/Configuration/Models/NuCacheSettings.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ public class NuCacheSettings
/// </summary>
internal const bool StaticUsePagedSqlQuery = true;

/// <summary>
/// The default number of content items whose stale NuCache rows are deleted per batch during a
/// content type rebuild. Matches the SQL parameter limit ceiling the delete is capped to.
/// </summary>
internal const int StaticContentTypeRebuildDeleteBatchSize = 2000;

/// <summary>
/// The serializer type that nucache uses to persist documents in the database.
/// </summary>
Expand All @@ -43,4 +49,20 @@ public class NuCacheSettings
/// </summary>
[DefaultValue(StaticUsePagedSqlQuery)]
public bool UsePagedSqlQuery { get; set; } = true;

/// <summary>
/// Gets or sets the number of content items whose stale NuCache rows are deleted per batch during a
/// content type structural rebuild.
/// </summary>
/// <remarks>
/// Deleting in batches avoids a single unbounded DELETE that can escalate to a table lock, bloat the
/// transaction log, or exceed the command timeout on sites with a lot of content. The matching node ids
/// are read from the source tables once, then their rows are deleted in batches of this size, so the
/// value only bounds the per-statement (and, for a deferred rebuild, per-transaction) footprint — it
/// does not cause repeated scans. The effective size is capped at the SQL parameter limit
/// (<see cref="Constants.Sql.MaxParameterCount" />); lower it if brief lock escalation on
/// <c>cmsContentNu</c> during a rebuild is a concern.
/// </remarks>
[DefaultValue(StaticContentTypeRebuildDeleteBatchSize)]
public int ContentTypeRebuildDeleteBatchSize { get; set; } = StaticContentTypeRebuildDeleteBatchSize;
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.Diagnostics;

Check warning on line 1 in src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

❌ Getting worse: Code Duplication

introduced similar code in: CountByObjectType,GetNodeIdsByContentTypes,GetPagedContentNodeIds Avoid duplicated, aka copy-pasted, code inside the module. More duplication lowers the code health.
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using NPoco;
Expand Down Expand Up @@ -444,92 +444,80 @@
}

Guid contentObjectType = Constants.ObjectTypes.Document;

long total = 0;
Dictionary<int, byte>? contentTypeVariations = null;
Dictionary<short, string>? languageMap = null;
Dictionary<int, List<PropertyTypeInfo>>? propertyInfoByContentType = null;

// Delete and pre-fetch in one step, then release locks before the paging loop.
executeStep(() =>
{
RemoveByObjectType(contentObjectType, contentTypeIds);

Sql<ISqlContext> countSql = Sql()
.SelectCount()
.From<NodeDto>()
.InnerJoin<ContentDto>().On<NodeDto, ContentDto>((n, c) => n.NodeId == c.NodeId)
.Where<NodeDto>(x => x.NodeObjectType == contentObjectType);

if (contentTypeIds.Count > 0)
{
countSql = countSql.WhereIn<ContentDto>(x => x.ContentTypeId, contentTypeIds);
}
// Delete stale rows in batches (each its own step); the returned count of affected nodes is the
// number to repopulate, so no separate count query is needed.
long total = RemoveByObjectTypeInBatches(contentObjectType, contentTypeIds, executeStep);

total = Database.ExecuteScalar<long>(countSql);

if (total == 0)
{
return;
}
if (total == 0)
{
return;
}

executeStep(() =>
{
contentTypeVariations = GetContentTypeVariations(contentTypeIds);
languageMap = GetLanguageMap();
propertyInfoByContentType = GetPropertyInfoByContentType(contentTypeIds);
});

if (total == 0)
{
return;
}

long processed = 0;
long pageIndex = 0;

while (processed < total)
{
var pageComplete = false;

executeStep(() =>
{
List<int> nodeIds = GetPagedContentNodeIds(contentObjectType, contentTypeIds, pageIndex, groupSize);
if (nodeIds.Count == 0)
{
return;
}

List<CacheRebuildDocumentDto> contentDtos = GetDocumentMetadataForNodes(nodeIds);
List<CacheRebuildPropertyDto> propertyDtos = GetPropertyDataForNodes(nodeIds);
List<CacheRebuildCultureDto> cultureDtos = GetCultureDataForNodes(nodeIds);
List<CacheRebuildDocumentCultureDto> documentCultureDtos = GetDocumentCultureDataForNodes(nodeIds);

var items = contentDtos
.AsParallel()
.WithDegreeOfParallelism(Environment.ProcessorCount)
.SelectMany(content => BuildCacheDtosForDocument(
content,
propertyDtos,
cultureDtos,
documentCultureDtos,
contentTypeVariations!,
languageMap!,
propertyInfoByContentType!,
serializer))
.ToList();

// Use "insert where not exists" to skip rows that a foreground content save
// may have written between the initial bulk delete and this page. This ensures
// the foreground's fresher data is preserved rather than overwritten.
BulkInsertSkipExisting(items);

processed += nodeIds.Count;
pageComplete = true;
});

// The break IS reachable: the executeStep delegate's early return (a page yielding no nodes — e.g.
// content removed concurrently, leaving the total row count stale) leaves pageComplete false, which
// guards against looping indefinitely. SonarLint cannot see through the delegate, so it incorrectly
// reports the condition as always false.
#pragma warning disable S2583 // Conditionally executed code should be reachable
if (!pageComplete)
{
break;
}
#pragma warning restore S2583

Check notice on line 520 in src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Code Health Review (v17/dev)

✅ No longer an issue: Large Method

RebuildContentDbCache is no longer above the threshold for lines of code Large functions with many lines of code are generally harder to understand and lower the code health. Avoid adding more lines to this function.

pageIndex++;
}
Expand Down Expand Up @@ -1023,39 +1011,22 @@

Guid mediaObjectType = Constants.ObjectTypes.Media;

long total = 0;
Dictionary<int, List<string>>? propertyAliasesByContentType = null;

executeStep(() =>
{
RemoveByObjectType(mediaObjectType, contentTypeIds);

Sql<ISqlContext> countSql = Sql()
.SelectCount()
.From<NodeDto>()
.InnerJoin<ContentDto>().On<NodeDto, ContentDto>((n, c) => n.NodeId == c.NodeId)
.Where<NodeDto>(x => x.NodeObjectType == mediaObjectType);

if (contentTypeIds.Count > 0)
{
countSql = countSql.WhereIn<ContentDto>(x => x.ContentTypeId, contentTypeIds);
}

total = Database.ExecuteScalar<long>(countSql);

if (total == 0)
{
return;
}

propertyAliasesByContentType = GetPropertyAliasesByContentType(contentTypeIds);
});
// Delete stale rows in batches (each its own step); the returned count of affected nodes is the
// number to repopulate, so no separate count query is needed.
long total = RemoveByObjectTypeInBatches(mediaObjectType, contentTypeIds, executeStep);

if (total == 0)
{
return;
}

executeStep(() =>
{
propertyAliasesByContentType = GetPropertyAliasesByContentType(contentTypeIds);
});

long processed = 0;
long pageIndex = 0;

Expand Down Expand Up @@ -1086,10 +1057,16 @@
pageComplete = true;
});

// The break IS reachable: the executeStep delegate's early return (a page yielding no nodes — e.g.
// content removed concurrently, leaving the total row count stale) leaves pageComplete false, which
// guards against looping indefinitely. SonarLint cannot see through the delegate, so it incorrectly
// reports the condition as always false.
#pragma warning disable S2583 // Conditionally executed code should be reachable
if (!pageComplete)
{
break;
}
#pragma warning restore S2583

pageIndex++;
}
Expand Down Expand Up @@ -1344,38 +1321,22 @@

Guid memberObjectType = Constants.ObjectTypes.Member;

long total = 0;
Dictionary<int, List<string>>? propertyAliasesByContentType = null;

executeStep(() =>
{
RemoveByObjectType(memberObjectType, contentTypeIds);

Sql<ISqlContext> countSql = Sql()
.SelectCount()
.From<NodeDto>()
.InnerJoin<ContentDto>().On<NodeDto, ContentDto>((n, c) => n.NodeId == c.NodeId)
.Where<NodeDto>(x => x.NodeObjectType == memberObjectType);

if (contentTypeIds.Count > 0)
{
countSql = countSql.WhereIn<ContentDto>(x => x.ContentTypeId, contentTypeIds);
}

total = Database.ExecuteScalar<long>(countSql);
if (total == 0)
{
return;
}

propertyAliasesByContentType = GetPropertyAliasesByContentType(contentTypeIds);
});
// Delete stale rows in batches (each its own step); the returned count of affected nodes is the
// number to repopulate, so no separate count query is needed.
long total = RemoveByObjectTypeInBatches(memberObjectType, contentTypeIds, executeStep);

if (total == 0)
{
return;
}

executeStep(() =>
{
propertyAliasesByContentType = GetPropertyAliasesByContentType(contentTypeIds);
});

long processed = 0;
long pageIndex = 0;

Expand Down Expand Up @@ -1406,15 +1367,133 @@
pageComplete = true;
});

// The break IS reachable: the executeStep delegate's early return (a page yielding no nodes — e.g.
// content removed concurrently, leaving the total row count stale) leaves pageComplete false, which
// guards against looping indefinitely. SonarLint cannot see through the delegate, so it incorrectly
// reports the condition as always false.
#pragma warning disable S2583 // Conditionally executed code should be reachable
if (!pageComplete)
{
break;
}
#pragma warning restore S2583

pageIndex++;
}
}

/// <summary>
/// Deletes the <c>cmsContentNu</c> rows for the given object type in batches, running each batch through
/// the supplied <paramref name="executeStep" /> delegate.
/// </summary>
/// <remarks>
/// Batching avoids a single unbounded DELETE that can escalate to a table lock, bloat the transaction
/// log, and (for a content type backing a lot of content) exceed the command timeout. When the caller's
/// <paramref name="executeStep" /> opens a fresh scope per step (the deferred rebuild path), each batch
/// commits in its own transaction, so the locks and log space it uses are released between batches
/// instead of accumulating across the whole delete. When it runs inline (the immediate path) the batches
/// share the ambient transaction, but each DELETE statement is still individually bounded.
/// </remarks>
/// <returns>The number of content nodes affected, i.e. the number that will need repopulating.</returns>
private long RemoveByObjectTypeInBatches(Guid objectType, IReadOnlyCollection<int> contentTypeIds, Action<Action> executeStep)
{
// A full clear (no content type filter) is only used by full rebuilds, where the table has typically
// already been truncated, so keep it as a single statement.
if (contentTypeIds.Count == 0)
{
long allCount = 0;
executeStep(() =>
{
RemoveByObjectType(objectType, contentTypeIds);
allCount = CountByObjectType(objectType, contentTypeIds);
});
return allCount;
}

// The node ids come from the (stable) source tables, not cmsContentNu, so fetching them once up front
// is safe: concurrent foreground saves during a deferred rebuild only add/remove rows we either
// correctly skip (new rows are preserved) or harmlessly no-op on (already-removed rows).
List<int> nodeIds = [];
executeStep(() => nodeIds = GetNodeIdsByContentTypes(objectType, contentTypeIds));

var batchSize = Math.Clamp(_nucacheSettings.Value.ContentTypeRebuildDeleteBatchSize, 1, Constants.Sql.MaxParameterCount);
var totalBatches = (int)Math.Ceiling(nodeIds.Count / (double)batchSize);

if (_logger.IsEnabled(LogLevel.Debug))
{
_logger.LogDebug(
"Rebuild: deleting cmsContentNu rows for object type {ObjectType} — {NodeCount} node(s) in {BatchCount} batch(es) of up to {BatchSize}.",
objectType,
nodeIds.Count,
totalBatches,
batchSize);
}

var batchNumber = 0;
foreach (IEnumerable<int> batch in nodeIds.InGroupsOf(batchSize))
{
var nodeIdBatch = batch.ToArray();
var currentBatchNumber = ++batchNumber;
executeStep(() =>
{
DeleteContentNuByNodeIds(nodeIdBatch);
if (_logger.IsEnabled(LogLevel.Debug))
{
_logger.LogDebug(
"Rebuild: deleted cmsContentNu batch {BatchNumber}/{BatchCount} ({NodeCount} node(s)) for object type {ObjectType}.",
currentBatchNumber,
totalBatches,
nodeIdBatch.Length,
objectType);
}
});
}

return nodeIds.Count;
}

private List<int> GetNodeIdsByContentTypes(Guid objectType, IReadOnlyCollection<int> contentTypeIds)
{
Sql<ISqlContext> sql = Sql()
.Select<NodeDto>(x => x.NodeId)
.From<NodeDto>()
.InnerJoin<ContentDto>().On<NodeDto, ContentDto>((n, c) => n.NodeId == c.NodeId)
.Where<NodeDto>(x => x.NodeObjectType == objectType)
.WhereIn<ContentDto>(x => x.ContentTypeId, contentTypeIds);

return Database.Fetch<int>(sql);
}

private long CountByObjectType(Guid objectType, IReadOnlyCollection<int> contentTypeIds)
{
Sql<ISqlContext> sql = Sql()
.SelectCount()
.From<NodeDto>()
.InnerJoin<ContentDto>().On<NodeDto, ContentDto>((n, c) => n.NodeId == c.NodeId)
.Where<NodeDto>(x => x.NodeObjectType == objectType);

if (contentTypeIds.Count > 0)
{
sql = sql.WhereIn<ContentDto>(x => x.ContentTypeId, contentTypeIds);
}

return Database.ExecuteScalar<long>(sql);
}

private void DeleteContentNuByNodeIds(int[] nodeIds)
{
if (nodeIds.Length == 0)
{
return;
}

Sql<ISqlContext> sql = Sql()
.Delete<ContentNuDto>()
.WhereIn<ContentNuDto>(x => x.NodeId, nodeIds);

Database.Execute(sql);
}

private void RemoveByObjectType(Guid objectType, IReadOnlyCollection<int> contentTypeIds)
{
// If the provided contentTypeIds collection is empty, remove all records for the provided object type.
Expand Down
Loading
Loading