From bff9dfedcc367492bc4f60e6f26b940fb2fe5c69 Mon Sep 17 00:00:00 2001 From: Shannon Date: Thu, 29 Jan 2015 12:45:44 +1100 Subject: [PATCH] Updates BackgroundTaskRunner to support async operations. Updates the content xml cache file persisting to use a IBackgroundTask instead of the strange queuing for persistence - which was super strange because in many cases another request thread will actually be the thread that is persisting the xml file than the request thread that requested it created. This implementation is far better, the xml file will be persisted on a non request thread and will handle multiple documents being published at the same time guaranteeing that the latest published version is the one persisted. The file persistence is also web aware (due to how BackgroundTaskRunner works) so during app shutdown the file will still be written if it's currently being processed. --- src/Umbraco.Core/XmlExtensions.cs | 28 ++++ .../Scheduling/BackgroundTaskRunnerTests.cs | 10 ++ .../XmlCacheFilePersister.cs | 104 ++++++++++++ .../Scheduling/BackgroundTaskRunner.cs | 32 ++-- src/Umbraco.Web/Scheduling/IBackgroundTask.cs | 3 + src/Umbraco.Web/Scheduling/LogScrubber.cs | 11 ++ .../Scheduling/ScheduledPublishing.cs | 11 ++ src/Umbraco.Web/Scheduling/ScheduledTasks.cs | 11 ++ src/Umbraco.Web/Umbraco.Web.csproj | 1 + src/Umbraco.Web/UmbracoModule.cs | 28 +--- .../umbraco.presentation/content.cs | 149 +++--------------- .../umbraco.presentation/requestModule.cs | 8 +- 12 files changed, 227 insertions(+), 169 deletions(-) create mode 100644 src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs diff --git a/src/Umbraco.Core/XmlExtensions.cs b/src/Umbraco.Core/XmlExtensions.cs index 02ebc07490..8784cbfb30 100644 --- a/src/Umbraco.Core/XmlExtensions.cs +++ b/src/Umbraco.Core/XmlExtensions.cs @@ -1,6 +1,9 @@ using System; using System.Collections.Generic; +using System.IO; using System.Linq; +using System.Text; +using System.Threading.Tasks; using System.Xml; using System.Xml.Linq; using System.Xml.XPath; @@ -13,6 +16,31 @@ namespace Umbraco.Core /// internal static class XmlExtensions { + /// + /// Saves the xml document async + /// + /// + /// + /// + public static async Task SaveAsync(this XmlDocument xdoc, string filename) + { + if (xdoc.DocumentElement == null) + throw new XmlException("Cannot save xml document, there is no root element"); + + using (var fs = new FileStream(filename, FileMode.Create, FileAccess.Write, FileShare.Read, bufferSize: 4096, useAsync: true)) + using (var xmlWriter = XmlWriter.Create(fs, new XmlWriterSettings + { + Async = true, + Encoding = Encoding.UTF8, + Indent = true + })) + { + //NOTE: There are no nice methods to write it async, only flushing it async. We + // could implement this ourselves but it'd be a very manual process. + xdoc.WriteTo(xmlWriter); + await xmlWriter.FlushAsync(); + } + } public static bool HasAttribute(this XmlAttributeCollection attributes, string attributeName) { diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index 8b1981bc9e..7cc763e534 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -265,6 +265,16 @@ namespace Umbraco.Tests.Scheduling public Guid UniqueId { get; protected set; } public abstract void Run(); + public Task RunAsync() + { + throw new NotImplementedException(); + } + + public bool IsAsync + { + get { return false; } + } + public abstract void Cancel(); public DateTime Queued { get; set; } diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs new file mode 100644 index 0000000000..482a666538 --- /dev/null +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs @@ -0,0 +1,104 @@ +using System; +using System.Diagnostics; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using System.Xml; +using Umbraco.Core; +using Umbraco.Core.Logging; +using Umbraco.Web.Scheduling; + +namespace Umbraco.Web.PublishedCache.XmlPublishedCache +{ + /// + /// This is the background task runner that persists the xml file to the file system + /// + /// + /// This is used so that all file saving is done on a web aware worker background thread and all logic is performed async so this + /// process will not interfere with any web requests threads. This is also done as to not require any global locks and to ensure that + /// if multiple threads are performing publishing tasks that the file will be persisted in accordance with the final resulting + /// xml structure since the file writes are queued. + /// + internal class XmlCacheFilePersister : DisposableObject, IBackgroundTask + { + private readonly XmlDocument _xDoc; + private readonly string _xmlFileName; + private readonly ProfilingLogger _logger; + + public XmlCacheFilePersister(XmlDocument xDoc, string xmlFileName, ProfilingLogger logger) + { + _xDoc = xDoc; + _xmlFileName = xmlFileName; + _logger = logger; + } + + public async Task RunAsync() + { + await PersistXmlToFileAsync(_xDoc); + } + + public bool IsAsync + { + get { return true; } + } + + /// + /// Persist a XmlDocument to the Disk Cache + /// + /// + internal async Task PersistXmlToFileAsync(XmlDocument xmlDoc) + { + if (xmlDoc != null) + { + using (_logger.DebugDuration( + string.Format("Saving content to disk on thread '{0}' (Threadpool? {1})", Thread.CurrentThread.Name, Thread.CurrentThread.IsThreadPoolThread), + string.Format("Saved content to disk on thread '{0}' (Threadpool? {1})", Thread.CurrentThread.Name, Thread.CurrentThread.IsThreadPoolThread))) + { + try + { + // Try to create directory for cache path if it doesn't yet exist + var directoryName = Path.GetDirectoryName(_xmlFileName); + // create dir if it is not there, if it's there, this will proceed as normal + Directory.CreateDirectory(directoryName); + + await xmlDoc.SaveAsync(_xmlFileName); + } + catch (Exception ee) + { + // If for whatever reason something goes wrong here, invalidate disk cache + DeleteXmlCache(); + + LogHelper.Error("Error saving content to disk", ee); + } + } + + + } + } + + private void DeleteXmlCache() + { + if (File.Exists(_xmlFileName) == false) return; + + // Reset file attributes, to make sure we can delete file + try + { + File.SetAttributes(_xmlFileName, FileAttributes.Normal); + } + finally + { + File.Delete(_xmlFileName); + } + } + + protected override void DisposeResources() + { + } + + public void Run() + { + throw new NotImplementedException(); + } + + } +} \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index a0c6720a88..60eef43b17 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -146,7 +146,8 @@ namespace Umbraco.Web.Scheduling T remainingTask; while (_tasks.TryTake(out remainingTask)) { - ConsumeTaskInternal(remainingTask); + ConsumeTaskInternalAsync(remainingTask) + .Wait(); //block until it completes } } @@ -178,7 +179,7 @@ namespace Umbraco.Web.Scheduling var token = _tokenSource.Token; _consumer = Task.Factory.StartNew(() => - StartThread(token), + StartThreadAsync(token), token, _dedicatedThread ? TaskCreationOptions.LongRunning : TaskCreationOptions.None, TaskScheduler.Default); @@ -197,7 +198,7 @@ namespace Umbraco.Web.Scheduling /// Invokes a new worker thread to consume tasks /// /// - private void StartThread(CancellationToken token) + private async Task StartThreadAsync(CancellationToken token) { // Was cancellation already requested? if (token.IsCancellationRequested) @@ -206,14 +207,14 @@ namespace Umbraco.Web.Scheduling token.ThrowIfCancellationRequested(); } - TakeAndConsumeTask(token); + await TakeAndConsumeTaskAsync(token); } /// /// Trys to get a task from the queue, if there isn't one it will wait a second and try again /// /// - private void TakeAndConsumeTask(CancellationToken token) + private async Task TakeAndConsumeTaskAsync(CancellationToken token) { if (token.IsCancellationRequested) { @@ -235,25 +236,25 @@ namespace Umbraco.Web.Scheduling // cancel when we shutdown foreach (var t in _tasks.GetConsumingEnumerable(token)) { - ConsumeTaskCancellable(t, token); + await ConsumeTaskCancellableAsync(t, token); } //recurse and keep going - TakeAndConsumeTask(token); + await TakeAndConsumeTaskAsync(token); } else { T repositoryTask; while (_tasks.TryTake(out repositoryTask)) { - ConsumeTaskCancellable(repositoryTask, token); + await ConsumeTaskCancellableAsync(repositoryTask, token); } //the task will end here } } - internal void ConsumeTaskCancellable(T task, CancellationToken token) + internal async Task ConsumeTaskCancellableAsync(T task, CancellationToken token) { if (token.IsCancellationRequested) { @@ -266,10 +267,10 @@ namespace Umbraco.Web.Scheduling token.ThrowIfCancellationRequested(); } - ConsumeTaskInternal(task); + await ConsumeTaskInternalAsync(task); } - private void ConsumeTaskInternal(T task) + private async Task ConsumeTaskInternalAsync(T task) { try { @@ -279,7 +280,14 @@ namespace Umbraco.Web.Scheduling { using (task) { - task.Run(); + if (task.IsAsync) + { + await task.RunAsync(); + } + else + { + task.Run(); + } } } catch (Exception e) diff --git a/src/Umbraco.Web/Scheduling/IBackgroundTask.cs b/src/Umbraco.Web/Scheduling/IBackgroundTask.cs index 343f076b2a..48522aeb5f 100644 --- a/src/Umbraco.Web/Scheduling/IBackgroundTask.cs +++ b/src/Umbraco.Web/Scheduling/IBackgroundTask.cs @@ -1,9 +1,12 @@ using System; +using System.Threading.Tasks; namespace Umbraco.Web.Scheduling { internal interface IBackgroundTask : IDisposable { void Run(); + Task RunAsync(); + bool IsAsync { get; } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/LogScrubber.cs b/src/Umbraco.Web/Scheduling/LogScrubber.cs index 1c7a8f3537..2edbe80726 100644 --- a/src/Umbraco.Web/Scheduling/LogScrubber.cs +++ b/src/Umbraco.Web/Scheduling/LogScrubber.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using System.Web; using System.Web.Caching; using umbraco.BusinessLogic; @@ -49,5 +50,15 @@ namespace Umbraco.Web.Scheduling Log.CleanLogs(GetLogScrubbingMaximumAge(_settings)); } } + + public Task RunAsync() + { + throw new NotImplementedException(); + } + + public bool IsAsync + { + get { return false; } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs index 8a64098764..cacb4e133d 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs @@ -2,6 +2,7 @@ using System; using System.Diagnostics; using System.Net; using System.Text; +using System.Threading.Tasks; using Umbraco.Core; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Logging; @@ -75,5 +76,15 @@ namespace Umbraco.Web.Scheduling } } } + + public Task RunAsync() + { + throw new NotImplementedException(); + } + + public bool IsAsync + { + get { return false; } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs index 94c035631f..ddcb9ea533 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs @@ -2,6 +2,7 @@ using System; using System.Collections; using System.Linq; using System.Net; +using System.Threading.Tasks; using System.Xml; using Umbraco.Core.Configuration; using Umbraco.Core.Configuration.UmbracoSettings; @@ -106,5 +107,15 @@ namespace Umbraco.Web.Scheduling } } } + + public Task RunAsync() + { + throw new NotImplementedException(); + } + + public bool IsAsync + { + get { return false; } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index a3e5122a06..68ac552b0b 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -553,6 +553,7 @@ ASPXCodeBehind + True True diff --git a/src/Umbraco.Web/UmbracoModule.cs b/src/Umbraco.Web/UmbracoModule.cs index 633df5c6d5..f2d216f2fd 100644 --- a/src/Umbraco.Web/UmbracoModule.cs +++ b/src/Umbraco.Web/UmbracoModule.cs @@ -5,6 +5,7 @@ using System.IO; using System.Linq; using System.Security.Principal; using System.Threading; +using System.Threading.Tasks; using System.Web; using System.Web.Mvc; using System.Web.Routing; @@ -529,24 +530,8 @@ namespace Umbraco.Web urlRouting.PostResolveRequestCache(context); } - /// - /// Checks if the xml cache file needs to be updated/persisted - /// - /// - /// - /// TODO: This needs an overhaul, see the error report created here: - /// https://docs.google.com/document/d/1neGE3q3grB4lVJfgID1keWY2v9JYqf-pw75sxUUJiyo/edit - /// - static void PersistXmlCache(HttpContextBase httpContext) - { - if (content.Instance.IsXmlQueuedForPersistenceToFile) - { - content.Instance.RemoveXmlFilePersistenceQueue(); - content.Instance.PersistXmlToFile(); - } - } - - /// + + /// /// Any object that is in the HttpContext.Items collection that is IDisposable will get disposed on the end of the request /// /// @@ -613,13 +598,6 @@ namespace Umbraco.Web ProcessRequest(new HttpContextWrapper(httpContext)); }; - // used to check if the xml cache file needs to be updated/persisted - app.PostRequestHandlerExecute += (sender, e) => - { - var httpContext = ((HttpApplication)sender).Context; - PersistXmlCache(new HttpContextWrapper(httpContext)); - }; - app.EndRequest += (sender, args) => { var httpContext = ((HttpApplication)sender).Context; diff --git a/src/Umbraco.Web/umbraco.presentation/content.cs b/src/Umbraco.Web/umbraco.presentation/content.cs index e2773a7112..63c0652edf 100644 --- a/src/Umbraco.Web/umbraco.presentation/content.cs +++ b/src/Umbraco.Web/umbraco.presentation/content.cs @@ -1,33 +1,28 @@ using System; -using System.Collections; using System.Collections.Generic; using System.Diagnostics; using System.Globalization; using System.IO; -using System.Linq; using System.Threading; +using System.Threading.Tasks; using System.Web; using System.Xml; -using System.Xml.XPath; -using umbraco.cms.presentation; using Umbraco.Core; using Umbraco.Core.Cache; using Umbraco.Core.Configuration; using Umbraco.Core.IO; using Umbraco.Core.Logging; using umbraco.BusinessLogic; -using umbraco.BusinessLogic.Actions; -using umbraco.BusinessLogic.Utils; using umbraco.cms.businesslogic; -using umbraco.cms.businesslogic.cache; using umbraco.cms.businesslogic.web; using Umbraco.Core.Models; +using Umbraco.Core.Profiling; using umbraco.DataLayer; using umbraco.presentation.nodeFactory; using Umbraco.Web; -using Action = umbraco.BusinessLogic.Actions.Action; +using Umbraco.Web.PublishedCache.XmlPublishedCache; +using Umbraco.Web.Scheduling; using Node = umbraco.NodeFactory.Node; -using Umbraco.Core; using File = System.IO.File; namespace umbraco @@ -37,6 +32,8 @@ namespace umbraco /// public class content { + private static readonly BackgroundTaskRunner FilePersister = new BackgroundTaskRunner(dedicatedThread: true); + #region Declarations // Sync access to disk file @@ -75,7 +72,6 @@ namespace umbraco #endregion - #region Singleton private static readonly Lazy LazyInstance = new Lazy(() => new content()); @@ -320,8 +316,8 @@ namespace umbraco // and clear the queue in case is this a web request, we don't want it reprocessing. if (UmbracoConfig.For.UmbracoSettings().Content.XmlCacheEnabled && UmbracoConfig.For.UmbracoSettings().Content.ContinouslyUpdateXmlDiskCache) { - RemoveXmlFilePersistenceQueue(); - PersistXmlToFile(xmlDoc); + FilePersister.Add(new XmlCacheFilePersister(xmlDoc, UmbracoXmlDiskCacheFileName , + new ProfilingLogger(LoggerResolver.Current.Logger, ProfilerResolver.Current.Profiler))); } } } @@ -929,54 +925,6 @@ namespace umbraco #region Protected & Private methods - internal const string PersistenceFlagContextKey = "vnc38ykjnkjdnk2jt98ygkxjng"; - - /// - /// Removes the flag that queues the file for persistence - /// - internal void RemoveXmlFilePersistenceQueue() - { - if (UmbracoContext.Current != null && UmbracoContext.Current.HttpContext != null) - { - UmbracoContext.Current.HttpContext.Application.Lock(); - UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey] = null; - UmbracoContext.Current.HttpContext.Application.UnLock(); - } - } - - internal bool IsXmlQueuedForPersistenceToFile - { - get - { - if (UmbracoContext.Current != null && UmbracoContext.Current.HttpContext != null) - { - bool val = UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey] != null; - if (val) - { - DateTime persistenceTime = DateTime.MinValue; - try - { - persistenceTime = (DateTime)UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey]; - if (persistenceTime > GetCacheFileUpdateTime()) - { - return true; - } - else - { - RemoveXmlFilePersistenceQueue(); - } - } - catch (Exception ex) - { - // Nothing to catch here - we'll just persist - LogHelper.Error("An error occurred checking if xml file is queued for persistence", ex); - } - } - } - return false; - } - } - /// /// Invalidates the disk content cache file. Effectively just deletes it. /// @@ -1248,51 +1196,26 @@ order by umbracoNode.level, umbracoNode.sortOrder"; } } + [Obsolete("This method should not be used, xml file persistence is done in a queue using a BackgroundTaskRunner")] public void PersistXmlToFile() - { - PersistXmlToFile(_xmlContent); - } - - /// - /// Persist a XmlDocument to the Disk Cache - /// - /// - internal void PersistXmlToFile(XmlDocument xmlDoc) { lock (ReaderWriterSyncLock) { - if (xmlDoc != null) - { - LogHelper.Debug("Saving content to disk on thread '{0}' (Threadpool? {1})", - () => Thread.CurrentThread.Name, - () => Thread.CurrentThread.IsThreadPoolThread); - + if (_xmlContent != null) + { try { - Stopwatch stopWatch = Stopwatch.StartNew(); + // create directory for cache path if it doesn't yet exist + var directoryName = Path.GetDirectoryName(UmbracoXmlDiskCacheFileName); + Directory.CreateDirectory(directoryName); - DeleteXmlCache(); - - // Try to create directory for cache path if it doesn't yet exist - string directoryName = Path.GetDirectoryName(UmbracoXmlDiskCacheFileName); - if (!File.Exists(UmbracoXmlDiskCacheFileName) && !Directory.Exists(directoryName)) - { - // We're already in a try-catch and saving will fail if this does, so don't need another - Directory.CreateDirectory(directoryName); - } - - xmlDoc.Save(UmbracoXmlDiskCacheFileName); - - LogHelper.Debug("Saved content on thread '{0}' in {1} (Threadpool? {2})", - () => Thread.CurrentThread.Name, - () => stopWatch.Elapsed, - () => Thread.CurrentThread.IsThreadPoolThread); + _xmlContent.Save(UmbracoXmlDiskCacheFileName); } catch (Exception ee) { // If for whatever reason something goes wrong here, invalidate disk cache DeleteXmlCache(); - + LogHelper.Error(string.Format( "Error saving content on thread '{0}' due to '{1}' (Threadpool? {2})", Thread.CurrentThread.Name, ee.Message, Thread.CurrentThread.IsThreadPoolThread), ee); @@ -1302,48 +1225,18 @@ order by umbracoNode.level, umbracoNode.sortOrder"; } /// - /// Marks a flag in the HttpContext so that, upon page execution completion, the Xml cache will - /// get persisted to disk. Ensure this method is only called from a thread executing a page request - /// since UmbracoModule is the only monitor of this flag and is responsible - /// for enacting the persistence at the PostRequestHandlerExecute stage of the page lifecycle. + /// Adds a task to the xml cache file persister /// private void QueueXmlForPersistence() { - //if this is called outside a web request we cannot queue it it will run in the current thread. - - if (UmbracoContext.Current != null && UmbracoContext.Current.HttpContext != null) - { - UmbracoContext.Current.HttpContext.Application.Lock(); - try - { - if (UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey] != null) - { - UmbracoContext.Current.HttpContext.Application.Add(PersistenceFlagContextKey, null); - } - UmbracoContext.Current.HttpContext.Application[PersistenceFlagContextKey] = DateTime.UtcNow; - } - finally - { - UmbracoContext.Current.HttpContext.Application.UnLock(); - } - } - else - { - // Save copy of content - if (UmbracoSettings.CloneXmlCacheOnPublish) - { - XmlDocument xmlContentCopy = CloneXmlDoc(_xmlContent); - PersistXmlToFile(xmlContentCopy); - } - else - { - PersistXmlToFile(); - } - } + FilePersister.Add(new XmlCacheFilePersister(_xmlContent, UmbracoXmlDiskCacheFileName, + new ProfilingLogger(LoggerResolver.Current.Logger, ProfilerResolver.Current.Profiler))); } internal DateTime GetCacheFileUpdateTime() { + //TODO: Should there be a try/catch here in case the file is being written to while this is trying to be executed? + if (File.Exists(UmbracoXmlDiskCacheFileName)) { return new FileInfo(UmbracoXmlDiskCacheFileName).LastWriteTimeUtc; diff --git a/src/Umbraco.Web/umbraco.presentation/requestModule.cs b/src/Umbraco.Web/umbraco.presentation/requestModule.cs index 6be1737545..9e2eeff4e6 100644 --- a/src/Umbraco.Web/umbraco.presentation/requestModule.cs +++ b/src/Umbraco.Web/umbraco.presentation/requestModule.cs @@ -317,10 +317,10 @@ namespace umbraco.presentation void context_PostRequestHandlerExecute(object sender, EventArgs e) { - if (content.Instance.IsXmlQueuedForPersistenceToFile) - { - content.Instance.PersistXmlToFile(); - } + //if (content.Instance.IsXmlQueuedForPersistenceToFile) + //{ + // content.Instance.PersistXmlToFile(); + //} }