From 71448eafe681292fa9cba64a8ba36a673953ff99 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 9 Sep 2021 16:04:18 -0600 Subject: [PATCH] Fixes empty recycle bin performance with indexing Currently when the recycle bin is empty, it is going to individually delete each item from the index. This is going to cause tons of allocations in Umbraco for DeferedDeleteIndex objects for each item and then down within Examine is going to process each one individually instead of just doing it in bulk. There will be a lot of allocations made there too along with a bunch of extra and unecessary threads. --- .../Examine/ExamineUmbracoIndexingHandler.cs | 47 +++++++++++++++++-- .../Search/IUmbracoIndexingHandler.cs | 12 ++++- .../IndexingNotificationHandler.Content.cs | 29 ++++++++++-- .../IndexingNotificationHandler.Media.cs | 31 ++++++++++-- 4 files changed, 109 insertions(+), 10 deletions(-) diff --git a/src/Umbraco.Infrastructure/Examine/ExamineUmbracoIndexingHandler.cs b/src/Umbraco.Infrastructure/Examine/ExamineUmbracoIndexingHandler.cs index d19db3b285..5afe010b82 100644 --- a/src/Umbraco.Infrastructure/Examine/ExamineUmbracoIndexingHandler.cs +++ b/src/Umbraco.Infrastructure/Examine/ExamineUmbracoIndexingHandler.cs @@ -118,6 +118,20 @@ namespace Umbraco.Cms.Infrastructure.Examine } } + /// + public void DeleteIndexForEntities(IReadOnlyCollection entityIds, bool keepIfUnpublished) + { + var actions = DeferedActions.Get(_scopeProvider); + if (actions != null) + { + actions.Add(new DeferedDeleteIndex(this, entityIds, keepIfUnpublished)); + } + else + { + DeferedDeleteIndex.Execute(this, entityIds, keepIfUnpublished); + } + } + /// public void ReIndexForContent(IContent sender, bool isPublished) { @@ -371,6 +385,7 @@ namespace Umbraco.Cms.Infrastructure.Examine { private readonly ExamineUmbracoIndexingHandler _examineUmbracoIndexingHandler; private readonly int _id; + private readonly IReadOnlyCollection _ids; private readonly bool _keepIfUnpublished; public DeferedDeleteIndex(ExamineUmbracoIndexingHandler examineUmbracoIndexingHandler, int id, bool keepIfUnpublished) @@ -380,16 +395,42 @@ namespace Umbraco.Cms.Infrastructure.Examine _keepIfUnpublished = keepIfUnpublished; } - public override void Execute() => Execute(_examineUmbracoIndexingHandler, _id, _keepIfUnpublished); + public DeferedDeleteIndex(ExamineUmbracoIndexingHandler examineUmbracoIndexingHandler, IReadOnlyCollection ids, bool keepIfUnpublished) + { + _examineUmbracoIndexingHandler = examineUmbracoIndexingHandler; + _ids = ids; + _keepIfUnpublished = keepIfUnpublished; + } + + public override void Execute() + { + if (_ids is null) + { + Execute(_examineUmbracoIndexingHandler, _id, _keepIfUnpublished); + } + else + { + Execute(_examineUmbracoIndexingHandler, _ids, _keepIfUnpublished); + } + } public static void Execute(ExamineUmbracoIndexingHandler examineUmbracoIndexingHandler, int id, bool keepIfUnpublished) { - var strId = id.ToString(CultureInfo.InvariantCulture); foreach (var index in examineUmbracoIndexingHandler._examineManager.Indexes.OfType() .Where(x => x.PublishedValuesOnly || !keepIfUnpublished) .Where(x => x.EnableDefaultEventHandler)) { - index.DeleteFromIndex(strId); + index.DeleteFromIndex(id.ToString(CultureInfo.InvariantCulture)); + } + } + + public static void Execute(ExamineUmbracoIndexingHandler examineUmbracoIndexingHandler, IReadOnlyCollection ids, bool keepIfUnpublished) + { + foreach (var index in examineUmbracoIndexingHandler._examineManager.Indexes.OfType() + .Where(x => x.PublishedValuesOnly || !keepIfUnpublished) + .Where(x => x.EnableDefaultEventHandler)) + { + index.DeleteFromIndex(ids.Select(x => x.ToString(CultureInfo.InvariantCulture))); } } } diff --git a/src/Umbraco.Infrastructure/Search/IUmbracoIndexingHandler.cs b/src/Umbraco.Infrastructure/Search/IUmbracoIndexingHandler.cs index 24c82c055d..7fe37d7152 100644 --- a/src/Umbraco.Infrastructure/Search/IUmbracoIndexingHandler.cs +++ b/src/Umbraco.Infrastructure/Search/IUmbracoIndexingHandler.cs @@ -25,7 +25,7 @@ namespace Umbraco.Cms.Infrastructure.Search void DeleteDocumentsForContentTypes(IReadOnlyCollection removedContentTypes); /// - /// Remove items from an index + /// Remove an item from an index /// /// /// @@ -33,5 +33,15 @@ namespace Umbraco.Cms.Infrastructure.Search /// If false it will delete this from all indexes regardless. /// void DeleteIndexForEntity(int entityId, bool keepIfUnpublished); + + /// + /// Remove items from an index + /// + /// + /// + /// If true, indicates that we will only delete this item from indexes that don't support unpublished content. + /// If false it will delete this from all indexes regardless. + /// + void DeleteIndexForEntities(IReadOnlyCollection entityIds, bool keepIfUnpublished); } } diff --git a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs index ebebdb7f34..c80e61af0e 100644 --- a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs +++ b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Content.cs @@ -44,13 +44,21 @@ namespace Umbraco.Cms.Infrastructure.Search throw new NotSupportedException(); } + // Used to track permanent deletions so we can bulk delete from the index + // when needed. For example, when emptying the recycle bin, else it will + // individually update the index which will be much slower. + HashSet deleteBatch = null; + foreach (var payload in (ContentCacheRefresher.JsonPayload[])args.MessageObject) { if (payload.ChangeTypes.HasType(TreeChangeTypes.Remove)) { - // delete content entirely (with descendants) - // false: remove entirely from all indexes - _umbracoIndexingHandler.DeleteIndexForEntity(payload.Id, false); + if (deleteBatch == null) + { + deleteBatch = new HashSet(); + } + + deleteBatch.Add(payload.Id); } else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll)) { @@ -62,6 +70,15 @@ namespace Umbraco.Cms.Infrastructure.Search } else // RefreshNode or RefreshBranch (maybe trashed) { + if (deleteBatch != null && deleteBatch.Contains(payload.Id)) + { + // the same node has already been deleted, to ensure ordering is + // handled, we'll need to execute all queued deleted items now + // and reset the deleted items list. + _umbracoIndexingHandler.DeleteIndexForEntities(deleteBatch, false); + deleteBatch = null; + } + // don't try to be too clever - refresh entirely // there has to be race conditions in there ;-( @@ -132,6 +149,12 @@ namespace Umbraco.Cms.Infrastructure.Search // // BUT ... pretty sure it is! see test "Index_Delete_Index_Item_Ensure_Heirarchy_Removed" } + + if (deleteBatch != null) + { + // process the delete batch + _umbracoIndexingHandler.DeleteIndexForEntities(deleteBatch, false); + } } } } diff --git a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Media.cs b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Media.cs index 8b37d047de..4ac1d1ca09 100644 --- a/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Media.cs +++ b/src/Umbraco.Infrastructure/Search/IndexingNotificationHandler.Media.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using Umbraco.Cms.Core.Cache; using Umbraco.Cms.Core.Events; using Umbraco.Cms.Core.Notifications; @@ -37,12 +38,21 @@ namespace Umbraco.Cms.Infrastructure.Search throw new NotSupportedException(); } + // Used to track permanent deletions so we can bulk delete from the index + // when needed. For example, when emptying the recycle bin, else it will + // individually update the index which will be much slower. + HashSet deleteBatch = null; + foreach (var payload in (MediaCacheRefresher.JsonPayload[])args.MessageObject) { if (payload.ChangeTypes.HasType(TreeChangeTypes.Remove)) { - // remove from *all* indexes - _umbracoIndexingHandler.DeleteIndexForEntity(payload.Id, false); + if (deleteBatch == null) + { + deleteBatch = new HashSet(); + } + + deleteBatch.Add(payload.Id); } else if (payload.ChangeTypes.HasType(TreeChangeTypes.RefreshAll)) { @@ -52,6 +62,15 @@ namespace Umbraco.Cms.Infrastructure.Search } else // RefreshNode or RefreshBranch (maybe trashed) { + if (deleteBatch != null && deleteBatch.Contains(payload.Id)) + { + // the same node has already been deleted, to ensure ordering is + // handled, we'll need to execute all queued deleted items now + // and reset the deleted items list. + _umbracoIndexingHandler.DeleteIndexForEntities(deleteBatch, false); + deleteBatch = null; + } + var media = _mediaService.GetById(payload.Id); if (media == null) { @@ -83,7 +102,13 @@ namespace Umbraco.Cms.Infrastructure.Search } } } - } + } + } + + if (deleteBatch != null) + { + // process the delete batch + _umbracoIndexingHandler.DeleteIndexForEntities(deleteBatch, false); } } }