From 6e08a0396ea5fc65861c88ea40c69747a321bbfb Mon Sep 17 00:00:00 2001 From: Shannon Date: Mon, 31 Aug 2020 23:41:19 +1000 Subject: [PATCH] Umbraco to re-index data on background thread Fixes issue with scopes being disposed or referenced incorrectly due to yield returns as this can capture a scope in the enumerator which will get passed to a background thread in Examine and cause some issues. We saw this issue in netcore but the issue must still exist in v8 but we don't visibly see it for some reason. The other issue is that the ValueSet lookup for content was done 3x when an IContent is saved and it should only be done 2x, one for published, one for unpublished. The other issue is that the data lookups to build a ValueSet are intended to be done on a background thread. This is the case in v7 because the IEnumerable is lazy and passed all the way down to Examine's background thread but this doesn't occur in v8 because we need to iterate/split that value set before it's sent to Examine so the ValueSet is eagerly built within the request. We can easily resolve this by using the background task manager and just pushing a task when IContent/IMedia/IMember is saved. This will return the execution to the UI quicker. --- src/Umbraco.Examine/UmbracoContentIndex.cs | 6 +- src/Umbraco.Web/Scheduling/SimpleTask.cs | 32 +++++++++ src/Umbraco.Web/Search/ExamineComponent.cs | 84 +++++++++++++++------- src/Umbraco.Web/Umbraco.Web.csproj | 1 + 4 files changed, 95 insertions(+), 28 deletions(-) create mode 100644 src/Umbraco.Web/Scheduling/SimpleTask.cs diff --git a/src/Umbraco.Examine/UmbracoContentIndex.cs b/src/Umbraco.Examine/UmbracoContentIndex.cs index e266ca789d..67e430b4e9 100644 --- a/src/Umbraco.Examine/UmbracoContentIndex.cs +++ b/src/Umbraco.Examine/UmbracoContentIndex.cs @@ -62,7 +62,7 @@ namespace Umbraco.Examine /// protected override void PerformIndexItems(IEnumerable values, Action onComplete) { - //We don't want to re-enumerate this list, but we need to split it into 2x enumerables: invalid and valid items. + // We don't want to re-enumerate this list, but we need to split it into 2x enumerables: invalid and valid items. // The Invalid items will be deleted, these are items that have invalid paths (i.e. moved to the recycle bin, etc...) // Then we'll index the Value group all together. // We return 0 or 1 here so we can order the results and do the invalid first and then the valid. @@ -80,7 +80,7 @@ namespace Umbraco.Examine || !validator.ValidateProtectedContent(path, v.Category)) ? 0 : 1; - }); + }).ToList(); var hasDeletes = false; var hasUpdates = false; @@ -99,7 +99,7 @@ namespace Umbraco.Examine { hasUpdates = true; //these are the valid ones, so just index them all at once - base.PerformIndexItems(group, onComplete); + base.PerformIndexItems(group.ToList(), onComplete); } } diff --git a/src/Umbraco.Web/Scheduling/SimpleTask.cs b/src/Umbraco.Web/Scheduling/SimpleTask.cs new file mode 100644 index 0000000000..a8603915b0 --- /dev/null +++ b/src/Umbraco.Web/Scheduling/SimpleTask.cs @@ -0,0 +1,32 @@ +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace Umbraco.Web.Scheduling +{ + /// + /// A simple task that executes a delegate synchronously + /// + internal class SimpleTask : IBackgroundTask + { + private readonly Action _action; + + public SimpleTask(Action action) + { + _action = action; + } + + public bool IsAsync => false; + + public void Run() => _action(); + + public Task RunAsync(CancellationToken token) + { + throw new NotImplementedException(); + } + + public void Dispose() + { + } + } +} diff --git a/src/Umbraco.Web/Search/ExamineComponent.cs b/src/Umbraco.Web/Search/ExamineComponent.cs index 149b4d1436..6f1a7cc960 100644 --- a/src/Umbraco.Web/Search/ExamineComponent.cs +++ b/src/Umbraco.Web/Search/ExamineComponent.cs @@ -17,9 +17,13 @@ using Umbraco.Core.Persistence.DatabaseModelDefinitions; using Examine.LuceneEngine.Directories; using Umbraco.Core.Composing; using System.ComponentModel; +using System.Threading; +using Umbraco.Web.Scheduling; namespace Umbraco.Web.Search { + + public sealed class ExamineComponent : Umbraco.Core.Composing.IComponent { private readonly IExamineManager _examineManager; @@ -34,7 +38,8 @@ namespace Umbraco.Web.Search private readonly IMainDom _mainDom; private readonly IProfilingLogger _logger; private readonly IUmbracoIndexesCreator _indexCreator; - + private readonly BackgroundTaskRunner _indexItemTaskRunner; + // the default enlist priority is 100 // enlist with a lower priority to ensure that anything "default" runs after us @@ -62,6 +67,7 @@ namespace Umbraco.Web.Search _mainDom = mainDom; _logger = profilingLogger; _indexCreator = indexCreator; + _indexItemTaskRunner = new BackgroundTaskRunner(_logger); } public void Initialize() @@ -574,12 +580,18 @@ namespace Umbraco.Web.Search } } + /// + /// An action that will execute at the end of the Scope being completed + /// private abstract class DeferedAction { public virtual void Execute() { } } + /// + /// Re-indexes an item on a background thread + /// private class DeferedReIndexForContent : DeferedAction { private readonly ExamineComponent _examineComponent; @@ -600,21 +612,32 @@ namespace Umbraco.Web.Search public static void Execute(ExamineComponent examineComponent, IContent content, bool isPublished) { - foreach (var index in examineComponent._examineManager.Indexes.OfType() - //filter the indexers - .Where(x => isPublished || !x.PublishedValuesOnly) - .Where(x => x.EnableDefaultEventHandler)) + // perform the ValueSet lookup on a background thread + examineComponent._indexItemTaskRunner.Add(new SimpleTask(() => { - //for content we have a different builder for published vs unpublished - var builder = index.PublishedValuesOnly - ? examineComponent._publishedContentValueSetBuilder - : (IValueSetBuilder)examineComponent._contentValueSetBuilder; + // for content we have a different builder for published vs unpublished + // we don't want to build more value sets than is needed so we'll lazily build 2 one for published one for non-published + var builders = new Dictionary>> + { + [true] = new Lazy>(() => examineComponent._publishedContentValueSetBuilder.GetValueSets(content).ToList()), + [false] = new Lazy>(() => examineComponent._contentValueSetBuilder.GetValueSets(content).ToList()) + }; - index.IndexItems(builder.GetValueSets(content)); - } + foreach (var index in examineComponent._examineManager.Indexes.OfType() + //filter the indexers + .Where(x => isPublished || !x.PublishedValuesOnly) + .Where(x => x.EnableDefaultEventHandler)) + { + var valueSet = builders[index.PublishedValuesOnly].Value; + index.IndexItems(valueSet); + } + })); } } + /// + /// Re-indexes an item on a background thread + /// private class DeferedReIndexForMedia : DeferedAction { private readonly ExamineComponent _examineComponent; @@ -635,18 +658,25 @@ namespace Umbraco.Web.Search public static void Execute(ExamineComponent examineComponent, IMedia media, bool isPublished) { - var valueSet = examineComponent._mediaValueSetBuilder.GetValueSets(media).ToList(); - - foreach (var index in examineComponent._examineManager.Indexes.OfType() - //filter the indexers - .Where(x => isPublished || !x.PublishedValuesOnly) - .Where(x => x.EnableDefaultEventHandler)) + // perform the ValueSet lookup on a background thread + examineComponent._indexItemTaskRunner.Add(new SimpleTask(() => { - index.IndexItems(valueSet); - } + var valueSet = examineComponent._mediaValueSetBuilder.GetValueSets(media).ToList(); + + foreach (var index in examineComponent._examineManager.Indexes.OfType() + //filter the indexers + .Where(x => isPublished || !x.PublishedValuesOnly) + .Where(x => x.EnableDefaultEventHandler)) + { + index.IndexItems(valueSet); + } + })); } } + /// + /// Re-indexes an item on a background thread + /// private class DeferedReIndexForMember : DeferedAction { private readonly ExamineComponent _examineComponent; @@ -665,13 +695,17 @@ namespace Umbraco.Web.Search public static void Execute(ExamineComponent examineComponent, IMember member) { - var valueSet = examineComponent._memberValueSetBuilder.GetValueSets(member).ToList(); - foreach (var index in examineComponent._examineManager.Indexes.OfType() - //filter the indexers - .Where(x => x.EnableDefaultEventHandler)) + // perform the ValueSet lookup on a background thread + examineComponent._indexItemTaskRunner.Add(new SimpleTask(() => { - index.IndexItems(valueSet); - } + var valueSet = examineComponent._memberValueSetBuilder.GetValueSets(member).ToList(); + foreach (var index in examineComponent._examineManager.Indexes.OfType() + //filter the indexers + .Where(x => x.EnableDefaultEventHandler)) + { + index.IndexItems(valueSet); + } + })); } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index ed457b1364..35a00d06c2 100755 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -247,6 +247,7 @@ +