From 6833c195c1faf28851fed949687cce513b54e565 Mon Sep 17 00:00:00 2001 From: Stephan Date: Mon, 6 Jul 2015 16:56:01 +0200 Subject: [PATCH] BackgroundTaskRunner - refactor recurring tasks, improve shutdown --- .../Scheduling/BackgroundTaskRunnerTests.cs | 67 +++++------ .../XmlCacheFilePersister.cs | 26 +---- .../Scheduling/BackgroundTaskRunner.cs | 9 +- .../Scheduling/DelayedRecurringTaskBase.cs | 74 ------------- src/Umbraco.Web/Scheduling/KeepAlive.cs | 79 +++++++++---- .../Scheduling/LatchedBackgroundTaskBase.cs | 77 +++++++++++++ src/Umbraco.Web/Scheduling/LogScrubber.cs | 37 +++---- .../Scheduling/RecurringTaskBase.cs | 104 ++++++++---------- .../Scheduling/ScheduledPublishing.cs | 71 ++++-------- src/Umbraco.Web/Scheduling/ScheduledTasks.cs | 35 ++---- src/Umbraco.Web/Scheduling/Scheduler.cs | 24 ++-- src/Umbraco.Web/Umbraco.Web.csproj | 2 +- 12 files changed, 278 insertions(+), 327 deletions(-) delete mode 100644 src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs create mode 100644 src/Umbraco.Web/Scheduling/LatchedBackgroundTaskBase.cs diff --git a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs index b541182de0..e6fc492350 100644 --- a/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs +++ b/src/Umbraco.Tests/Scheduling/BackgroundTaskRunnerTests.cs @@ -525,15 +525,11 @@ namespace Umbraco.Tests.Scheduling var waitHandle = new ManualResetEvent(false); using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) { - runner.TaskCompleted += (sender, args) => runCount++; - runner.TaskStarting += async (sender, args) => + runner.TaskCompleted += (sender, args) => { - //wait for each task to finish once it's started - await sender.CurrentThreadingTask; + runCount++; if (runCount > 3) - { waitHandle.Set(); - } }; var task = new MyRecurringTask(runner, 200, 500); @@ -548,7 +544,10 @@ namespace Umbraco.Tests.Scheduling Assert.GreaterOrEqual(runCount, 4); // stops recurring - runner.Shutdown(false, false); + runner.Shutdown(false, true); + + // check that task has been disposed (timer has been killed, etc) + Assert.IsTrue(task.Disposed); } } @@ -595,15 +594,12 @@ namespace Umbraco.Tests.Scheduling var waitHandle = new ManualResetEvent(false); using (var runner = new BackgroundTaskRunner(new BackgroundTaskRunnerOptions(), _logger)) { - runner.TaskCompleted += (sender, args) => runCount++; - runner.TaskStarting += async (sender, args) => + runner.TaskCompleted += (sender, args) => { - //wait for each task to finish once it's started - await sender.CurrentThreadingTask; + runCount++; if (runCount > 3) - { waitHandle.Set(); - } + }; var task = new MyDelayedRecurringTask(runner, 2000, 1000); @@ -727,37 +723,31 @@ namespace Umbraco.Tests.Scheduling } } - private class MyDelayedRecurringTask : DelayedRecurringTaskBase + private class MyDelayedRecurringTask : RecurringTaskBase { public bool HasRun { get; private set; } - public MyDelayedRecurringTask(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds) + public MyDelayedRecurringTask(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds) : base(runner, delayMilliseconds, periodMilliseconds) { } - private MyDelayedRecurringTask(MyDelayedRecurringTask source) - : base(source) - { } - public override bool IsAsync { get { return false; } } - public override void PerformRun() + public override bool PerformRun() { HasRun = true; + return true; // repeat } - public override Task PerformRunAsync(CancellationToken token) + public override Task PerformRunAsync(CancellationToken token) { throw new NotImplementedException(); } - protected override MyDelayedRecurringTask GetRecurring() - { - return new MyDelayedRecurringTask(this); - } + public override bool RunsOnShutdown { get { return true; } } } private class MyDelayedTask : ILatchedBackgroundTask @@ -811,29 +801,23 @@ namespace Umbraco.Tests.Scheduling { } } - private class MyRecurringTask : RecurringTaskBase + private class MyRecurringTask : RecurringTaskBase { private readonly int _runMilliseconds; - - public MyRecurringTask(IBackgroundTaskRunner runner, int runMilliseconds, int periodMilliseconds) - : base(runner, periodMilliseconds) + public MyRecurringTask(IBackgroundTaskRunner runner, int runMilliseconds, int periodMilliseconds) + : base(runner, 0, periodMilliseconds) { _runMilliseconds = runMilliseconds; } - private MyRecurringTask(MyRecurringTask source, int runMilliseconds) - : base(source) - { - _runMilliseconds = runMilliseconds; - } - - public override void PerformRun() + public override bool PerformRun() { Thread.Sleep(_runMilliseconds); + return true; // repeat } - public override Task PerformRunAsync(CancellationToken token) + public override Task PerformRunAsync(CancellationToken token) { throw new NotImplementedException(); } @@ -843,10 +827,15 @@ namespace Umbraco.Tests.Scheduling get { return false; } } - protected override MyRecurringTask GetRecurring() + public override bool RunsOnShutdown { get { return false; } } + + protected override void Dispose(bool disposing) { - return new MyRecurringTask(this, _runMilliseconds); + Disposed = true; + base.Dispose(disposing); } + + public bool Disposed { get; private set; } } private class MyTask : BaseTask diff --git a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs index 137ed11482..ef84f31473 100644 --- a/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs +++ b/src/Umbraco.Web/PublishedCache/XmlPublishedCache/XmlCacheFilePersister.cs @@ -17,12 +17,11 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache /// 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 : ILatchedBackgroundTask + internal class XmlCacheFilePersister : LatchedBackgroundTaskBase { private readonly IBackgroundTaskRunner _runner; private readonly content _content; private readonly ProfilingLogger _logger; - private readonly ManualResetEventSlim _latch = new ManualResetEventSlim(false); private readonly object _locko = new object(); private bool _released; private Timer _timer; @@ -39,7 +38,7 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache private const int MaxWaitMilliseconds = 30000; // save the cache after some time (ie no more than 30s of changes) // save the cache when the app goes down - public bool RunsOnShutdown { get { return true; } } + public override bool RunsOnShutdown { get { return true; } } // initialize the first instance, which is inactive (not touched yet) public XmlCacheFilePersister(IBackgroundTaskRunner runner, content content, ProfilingLogger logger) @@ -150,21 +149,11 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache // if running (because of shutdown) this will have no effect // else it tells the runner it is time to run the task - _latch.Set(); + Release(); } } - public WaitHandle Latch - { - get { return _latch.WaitHandle; } - } - - public bool IsLatched - { - get { return true; } - } - - public async Task RunAsync(CancellationToken token) + public override async Task RunAsync(CancellationToken token) { lock (_locko) { @@ -189,15 +178,12 @@ namespace Umbraco.Web.PublishedCache.XmlPublishedCache } } - public bool IsAsync + public override bool IsAsync { get { return true; } } - public void Dispose() - { } - - public void Run() + public override void Run() { lock (_locko) { diff --git a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs index 3cac89c8de..388012930b 100644 --- a/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs +++ b/src/Umbraco.Web/Scheduling/BackgroundTaskRunner.cs @@ -391,6 +391,7 @@ namespace Umbraco.Web.Scheduling // still latched & not running on shutdown = stop here if (dbgTask.IsLatched && dbgTask.RunsOnShutdown == false) { + dbgTask.Dispose(); // will not run TaskSourceCompleted(taskSource, token); return; } @@ -448,7 +449,7 @@ namespace Umbraco.Web.Scheduling try { - using (bgTask) // ensure it's disposed + try { if (bgTask.IsAsync) //configure await = false since we don't care about the context, we're on a background thread. @@ -456,6 +457,12 @@ namespace Umbraco.Web.Scheduling else bgTask.Run(); } + finally // ensure we disposed - unless latched (again) + { + var lbgTask = bgTask as ILatchedBackgroundTask; + if (lbgTask == null || lbgTask.IsLatched == false) + bgTask.Dispose(); + } } catch (Exception e) { diff --git a/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs deleted file mode 100644 index af3dedbe70..0000000000 --- a/src/Umbraco.Web/Scheduling/DelayedRecurringTaskBase.cs +++ /dev/null @@ -1,74 +0,0 @@ -using System; -using System.Threading; -using System.Threading.Tasks; - -namespace Umbraco.Web.Scheduling -{ - /// - /// Provides a base class for recurring background tasks. - /// - /// The type of the managed tasks. - internal abstract class DelayedRecurringTaskBase : RecurringTaskBase, ILatchedBackgroundTask - where T : class, IBackgroundTask - { - private readonly ManualResetEventSlim _latch; - private Timer _timer; - - protected DelayedRecurringTaskBase(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds) - : base(runner, periodMilliseconds) - { - if (delayMilliseconds > 0) - { - _latch = new ManualResetEventSlim(false); - _timer = new Timer(_ => - { - _timer.Dispose(); - _timer = null; - _latch.Set(); - }); - _timer.Change(delayMilliseconds, 0); - } - } - - protected DelayedRecurringTaskBase(DelayedRecurringTaskBase source) - : base(source) - { - // no latch on recurring instances - _latch = null; - } - - public override void Run() - { - if (_latch != null) - _latch.Dispose(); - base.Run(); - } - - public override async Task RunAsync(CancellationToken token) - { - if (_latch != null) - _latch.Dispose(); - await base.RunAsync(token); - } - - public WaitHandle Latch - { - get - { - if (_latch == null) - throw new InvalidOperationException("The task is not latched."); - return _latch.WaitHandle; - } - } - - public bool IsLatched - { - get { return _latch != null && _latch.IsSet == false; } - } - - public virtual bool RunsOnShutdown - { - get { return true; } - } - } -} diff --git a/src/Umbraco.Web/Scheduling/KeepAlive.cs b/src/Umbraco.Web/Scheduling/KeepAlive.cs index a47f8aa72c..d5ffbc811d 100644 --- a/src/Umbraco.Web/Scheduling/KeepAlive.cs +++ b/src/Umbraco.Web/Scheduling/KeepAlive.cs @@ -1,41 +1,76 @@ using System; using System.Net; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; using Umbraco.Core; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Logging; namespace Umbraco.Web.Scheduling { - internal class KeepAlive + internal class KeepAlive : RecurringTaskBase { - public static void Start(ApplicationContext appContext, IUmbracoSettingsSection settings) + private readonly ApplicationContext _appContext; + + public KeepAlive(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, + ApplicationContext appContext) + : base(runner, delayMilliseconds, periodMilliseconds) { - using (DisposableTimer.DebugDuration(() => "Keep alive executing", () => "Keep alive complete")) + _appContext = appContext; + } + + public override bool PerformRun() + { + throw new NotImplementedException(); + } + + public override async Task PerformRunAsync(CancellationToken token) + { + if (_appContext == null) return true; // repeat... + + string umbracoAppUrl = null; + + try { - var umbracoAppUrl = appContext.UmbracoApplicationUrl; - if (umbracoAppUrl.IsNullOrWhiteSpace()) + using (DisposableTimer.DebugDuration(() => "Keep alive executing", () => "Keep alive complete")) { - LogHelper.Warn("No url for service (yet), skip."); - return; - } - - var url = umbracoAppUrl + "/ping.aspx"; - - try - { - using (var wc = new WebClient()) + umbracoAppUrl = _appContext.UmbracoApplicationUrl; + if (umbracoAppUrl.IsNullOrWhiteSpace()) { - wc.DownloadString(url); + LogHelper.Warn("No url for service (yet), skip."); + return true; // repeat + } + + var url = umbracoAppUrl + "/ping.aspx"; + using (var wc = new HttpClient()) + { + var request = new HttpRequestMessage() + { + RequestUri = new Uri(url), + Method = HttpMethod.Get + }; + + var result = await wc.SendAsync(request, token); } } - catch (Exception ee) - { - LogHelper.Error( - string.Format("Error in ping. The base url used in the request was: {0}, see http://our.umbraco.org/documentation/Using-Umbraco/Config-files/umbracoSettings/#ScheduledTasks documentation for details on setting a baseUrl if this is in error", umbracoAppUrl) - , ee); - } } - + catch (Exception e) + { + LogHelper.Error(string.Format("Failed (at \"{0}\").", umbracoAppUrl), e); + } + + return true; // repeat + } + + public override bool IsAsync + { + get { return true; } + } + + public override bool RunsOnShutdown + { + get { return false; } } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/LatchedBackgroundTaskBase.cs b/src/Umbraco.Web/Scheduling/LatchedBackgroundTaskBase.cs new file mode 100644 index 0000000000..c024382ee8 --- /dev/null +++ b/src/Umbraco.Web/Scheduling/LatchedBackgroundTaskBase.cs @@ -0,0 +1,77 @@ +using System; +using System.Threading; +using System.Threading.Tasks; + +namespace Umbraco.Web.Scheduling +{ + internal abstract class LatchedBackgroundTaskBase : ILatchedBackgroundTask + { + private readonly ManualResetEventSlim _latch; + private bool _disposed; + + protected LatchedBackgroundTaskBase() + { + _latch = new ManualResetEventSlim(false); + } + + /// + /// Implements IBackgroundTask.Run(). + /// + public abstract void Run(); + + /// + /// Implements IBackgroundTask.RunAsync(). + /// + public abstract Task RunAsync(CancellationToken token); + + /// + /// Indicates whether the background task can run asynchronously. + /// + public abstract bool IsAsync { get; } + + public WaitHandle Latch + { + get { return _latch.WaitHandle; } + } + + public bool IsLatched + { + get { return _latch.IsSet == false; } + } + + protected void Release() + { + _latch.Set(); + } + + protected void Reset() + { + _latch.Reset(); + } + + public abstract bool RunsOnShutdown { get; } + + public void Dispose() + { + Dispose(true); + GC.SuppressFinalize(this); + } + + // the task is going to be disposed again after execution, + // unless it is latched again, thus indicating it wants to + // remain active + + protected virtual void Dispose(bool disposing) + { + // lock on _latch instead of creating a new object as _timer is + // private, non-null, readonly - so safe here + lock (_latch) + { + if (_disposed) return; + _disposed = true; + + _latch.Dispose(); + } + } + } +} diff --git a/src/Umbraco.Web/Scheduling/LogScrubber.cs b/src/Umbraco.Web/Scheduling/LogScrubber.cs index 14f534653d..f920b5bb9d 100644 --- a/src/Umbraco.Web/Scheduling/LogScrubber.cs +++ b/src/Umbraco.Web/Scheduling/LogScrubber.cs @@ -7,15 +7,16 @@ using umbraco.BusinessLogic; using Umbraco.Core; using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Logging; +using Umbraco.Core.Sync; namespace Umbraco.Web.Scheduling { - internal class LogScrubber : DelayedRecurringTaskBase + internal class LogScrubber : RecurringTaskBase { private readonly ApplicationContext _appContext; private readonly IUmbracoSettingsSection _settings; - public LogScrubber(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, + public LogScrubber(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, ApplicationContext appContext, IUmbracoSettingsSection settings) : base(runner, delayMilliseconds, periodMilliseconds) { @@ -23,20 +24,8 @@ namespace Umbraco.Web.Scheduling _settings = settings; } - public LogScrubber(LogScrubber source) - : base(source) - { - _appContext = source._appContext; - _settings = source._settings; - } - - protected override LogScrubber GetRecurring() - { - return new LogScrubber(this); - } - // maximum age, in minutes - private int GetLogScrubbingMaximumAge(IUmbracoSettingsSection settings) + private static int GetLogScrubbingMaximumAge(IUmbracoSettingsSection settings) { var maximumAge = 24 * 60; // 24 hours, in minutes try @@ -46,7 +35,7 @@ namespace Umbraco.Web.Scheduling } catch (Exception e) { - LogHelper.Error("Unable to locate a log scrubbing maximum age. Defaulting to 24 hours.", e); + LogHelper.Error("Unable to locate a log scrubbing maximum age. Defaulting to 24 hours.", e); } return maximumAge; @@ -67,15 +56,25 @@ namespace Umbraco.Web.Scheduling return interval; } - public override void PerformRun() + public override bool PerformRun() { + if (_appContext == null) return true; // repeat... + + if (ServerEnvironmentHelper.GetStatus(_settings) == CurrentServerEnvironmentStatus.Slave) + { + LogHelper.Debug("Does not run on slave servers."); + return false; // do NOT repeat, server status comes from config and will NOT change + } + using (DisposableTimer.DebugDuration("Log scrubbing executing", "Log scrubbing complete")) { Log.CleanLogs(GetLogScrubbingMaximumAge(_settings)); - } + } + + return true; // repeat } - public override Task PerformRunAsync(CancellationToken token) + public override Task PerformRunAsync(CancellationToken token) { throw new NotImplementedException(); } diff --git a/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs index dc82795852..3fea70a2b8 100644 --- a/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs +++ b/src/Umbraco.Web/Scheduling/RecurringTaskBase.cs @@ -6,57 +6,51 @@ namespace Umbraco.Web.Scheduling /// /// Provides a base class for recurring background tasks. /// - /// The type of the managed tasks. - internal abstract class RecurringTaskBase : IBackgroundTask - where T : class, IBackgroundTask + internal abstract class RecurringTaskBase : LatchedBackgroundTaskBase { - private readonly IBackgroundTaskRunner _runner; + private readonly IBackgroundTaskRunner _runner; private readonly int _periodMilliseconds; - private Timer _timer; - private T _recurrent; + private readonly Timer _timer; + private bool _disposed; /// - /// Initializes a new instance of the class with a tasks runner and a period. + /// Initializes a new instance of the class. /// /// The task runner. + /// The delay. /// The period. /// The task will repeat itself periodically. Use this constructor to create a new task. - protected RecurringTaskBase(IBackgroundTaskRunner runner, int periodMilliseconds) + protected RecurringTaskBase(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds) { _runner = runner; _periodMilliseconds = periodMilliseconds; - } - /// - /// Initializes a new instance of the class with a source task. - /// - /// The source task. - /// Use this constructor to create a new task from a source task in GetRecurring. - protected RecurringTaskBase(RecurringTaskBase source) - { - _runner = source._runner; - _timer = source._timer; - _periodMilliseconds = source._periodMilliseconds; + // note + // must use the single-parameter constructor on Timer to avoid it from being GC'd + // read http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer + + _timer = new Timer(_ => Release()); + _timer.Change(delayMilliseconds, 0); } /// /// Implements IBackgroundTask.Run(). /// /// Classes inheriting from RecurringTaskBase must implement PerformRun. - public virtual void Run() + public override void Run() { - PerformRun(); - Repeat(); + var shouldRepeat = PerformRun(); + if (shouldRepeat) Repeat(); } /// /// Implements IBackgroundTask.RunAsync(). /// /// Classes inheriting from RecurringTaskBase must implement PerformRun. - public virtual async Task RunAsync(CancellationToken token) + public override async Task RunAsync(CancellationToken token) { - await PerformRunAsync(token); - Repeat(); + var shouldRepeat = await PerformRunAsync(token); + if (shouldRepeat) Repeat(); } private void Repeat() @@ -64,53 +58,45 @@ namespace Umbraco.Web.Scheduling // again? if (_runner.IsCompleted) return; // fail fast - if (_periodMilliseconds == 0) return; + if (_periodMilliseconds == 0) return; // safe - _recurrent = GetRecurring(); - if (_recurrent == null) - { - _timer.Dispose(); - _timer = null; - return; // done - } + Reset(); // re-latch - // note - // must use the single-parameter constructor on Timer to avoid it from being GC'd - // read http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer - - _timer = _timer ?? new Timer(_ => _runner.TryAdd(_recurrent)); - _timer.Change(_periodMilliseconds, 0); + // try to add again (may fail if runner has completed) + // if added, re-start the timer, else kill it + if (_runner.TryAdd(this)) + _timer.Change(_periodMilliseconds, 0); + else + Dispose(true); } - /// - /// Indicates whether the background task can run asynchronously. - /// - public abstract bool IsAsync { get; } - /// /// Runs the background task. /// - public abstract void PerformRun(); + /// A value indicating whether to repeat the task. + public abstract bool PerformRun(); /// /// Runs the task asynchronously. /// /// A cancellation token. - /// A instance representing the execution of the background task. - public abstract Task PerformRunAsync(CancellationToken token); + /// A instance representing the execution of the background task, + /// and returning a value indicating whether to repeat the task. + public abstract Task PerformRunAsync(CancellationToken token); - /// - /// Gets a new occurence of the recurring task. - /// - /// A new task instance to be queued, or null to terminate the recurring task. - /// The new task instance must be created via the RecurringTaskBase(RecurringTaskBase{T} source) constructor, - /// where source is the current task, eg: return new MyTask(this); - protected abstract T GetRecurring(); + protected override void Dispose(bool disposing) + { + // lock on _timer instead of creating a new object as _timer is + // private, non-null, readonly - so safe here + lock (_timer) + { + if (_disposed) return; + _disposed = true; - /// - /// Dispose the task. - /// - public virtual void Dispose() - { } + // stop the timer + _timer.Change(Timeout.Infinite, Timeout.Infinite); + _timer.Dispose(); + } + } } } \ No newline at end of file diff --git a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs index d1dc8d1935..78a91f6341 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledPublishing.cs @@ -10,14 +10,12 @@ using Umbraco.Web.Mvc; namespace Umbraco.Web.Scheduling { - internal class ScheduledPublishing : DelayedRecurringTaskBase + internal class ScheduledPublishing : RecurringTaskBase { private readonly ApplicationContext _appContext; private readonly IUmbracoSettingsSection _settings; - private static bool _isPublishingRunning; - - public ScheduledPublishing(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, + public ScheduledPublishing(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, ApplicationContext appContext, IUmbracoSettingsSection settings) : base(runner, delayMilliseconds, periodMilliseconds) { @@ -25,48 +23,34 @@ namespace Umbraco.Web.Scheduling _settings = settings; } - private ScheduledPublishing(ScheduledPublishing source) - : base(source) - { - _appContext = source._appContext; - _settings = source._settings; - } - - protected override ScheduledPublishing GetRecurring() - { - return new ScheduledPublishing(this); - } - - public override void PerformRun() + public override bool PerformRun() { throw new NotImplementedException(); } - public override async Task PerformRunAsync(CancellationToken token) - { - - if (_appContext == null) return; + public override async Task PerformRunAsync(CancellationToken token) + { + if (_appContext == null) return true; // repeat... + if (ServerEnvironmentHelper.GetStatus(_settings) == CurrentServerEnvironmentStatus.Slave) { LogHelper.Debug("Does not run on slave servers."); - return; + return false; // do NOT repeat, server status comes from config and will NOT change } using (DisposableTimer.DebugDuration(() => "Scheduled publishing executing", () => "Scheduled publishing complete")) { - if (_isPublishingRunning) return; - - _isPublishingRunning = true; - - var umbracoAppUrl = _appContext.UmbracoApplicationUrl; - if (umbracoAppUrl.IsNullOrWhiteSpace()) - { - LogHelper.Warn("No url for service (yet), skip."); - return; - } + string umbracoAppUrl = null; try { + umbracoAppUrl = _appContext.UmbracoApplicationUrl; + if (umbracoAppUrl.IsNullOrWhiteSpace()) + { + LogHelper.Warn("No url for service (yet), skip."); + return true; // repeat + } + var url = umbracoAppUrl + "/RestServices/ScheduledPublish/Index"; using (var wc = new HttpClient()) { @@ -79,27 +63,16 @@ namespace Umbraco.Web.Scheduling //pass custom the authorization header request.Headers.Authorization = AdminTokenAuthorizeAttribute.GetAuthenticationHeaderValue(_appContext); - try - { - var result = await wc.SendAsync(request, token); - } - catch (Exception ex) - { - LogHelper.Error("An error occurred calling scheduled publish url", ex); - } + var result = await wc.SendAsync(request, token); } } - catch (Exception ee) + catch (Exception e) { - LogHelper.Error( - string.Format("An error occurred with the scheduled publishing. The base url used in the request was: {0}, see http://our.umbraco.org/documentation/Using-Umbraco/Config-files/umbracoSettings/#ScheduledTasks documentation for details on setting a baseUrl if this is in error", umbracoAppUrl) - , ee); + LogHelper.Error(string.Format("Failed (at \"{0}\").", umbracoAppUrl), e); } - finally - { - _isPublishingRunning = false; - } - } + } + + return true; // repeat } public override bool IsAsync diff --git a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs index 1015b2d4f6..92214e5199 100644 --- a/src/Umbraco.Web/Scheduling/ScheduledTasks.cs +++ b/src/Umbraco.Web/Scheduling/ScheduledTasks.cs @@ -15,14 +15,13 @@ namespace Umbraco.Web.Scheduling // would need to be a publicly available task (URL) which isn't really very good :( // We should really be using the AdminTokenAuthorizeAttribute for this stuff - internal class ScheduledTasks : DelayedRecurringTaskBase + internal class ScheduledTasks : RecurringTaskBase { private readonly ApplicationContext _appContext; private readonly IUmbracoSettingsSection _settings; private static readonly Hashtable ScheduledTaskTimes = new Hashtable(); - private static bool _isPublishingRunning = false; - public ScheduledTasks(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, + public ScheduledTasks(IBackgroundTaskRunner runner, int delayMilliseconds, int periodMilliseconds, ApplicationContext appContext, IUmbracoSettingsSection settings) : base(runner, delayMilliseconds, periodMilliseconds) { @@ -30,18 +29,6 @@ namespace Umbraco.Web.Scheduling _settings = settings; } - public ScheduledTasks(ScheduledTasks source) - : base(source) - { - _appContext = source._appContext; - _settings = source._settings; - } - - protected override ScheduledTasks GetRecurring() - { - return new ScheduledTasks(this); - } - private async Task ProcessTasksAsync(CancellationToken token) { var scheduledTasks = _settings.ScheduledTasks.Tasks; @@ -99,25 +86,23 @@ namespace Umbraco.Web.Scheduling } } - public override void PerformRun() + public override bool PerformRun() { throw new NotImplementedException(); } - public override async Task PerformRunAsync(CancellationToken token) + public override async Task PerformRunAsync(CancellationToken token) { + if (_appContext == null) return true; // repeat... + if (ServerEnvironmentHelper.GetStatus(_settings) == CurrentServerEnvironmentStatus.Slave) { LogHelper.Debug("Does not run on slave servers."); - return; + return false; // do NOT repeat, server status comes from config and will NOT change } using (DisposableTimer.DebugDuration(() => "Scheduled tasks executing", () => "Scheduled tasks complete")) { - if (_isPublishingRunning) return; - - _isPublishingRunning = true; - try { await ProcessTasksAsync(token); @@ -126,11 +111,9 @@ namespace Umbraco.Web.Scheduling { LogHelper.Error("Error executing scheduled task", ee); } - finally - { - _isPublishingRunning = false; - } } + + return true; // repeat } public override bool IsAsync diff --git a/src/Umbraco.Web/Scheduling/Scheduler.cs b/src/Umbraco.Web/Scheduling/Scheduler.cs index fff6261401..f1f48f141a 100644 --- a/src/Umbraco.Web/Scheduling/Scheduler.cs +++ b/src/Umbraco.Web/Scheduling/Scheduler.cs @@ -1,11 +1,7 @@ -using System; -using System.Threading; -using System.Web; +using System.Web; using Umbraco.Core; using Umbraco.Core.Configuration; -using Umbraco.Core.Configuration.UmbracoSettings; using Umbraco.Core.Logging; -using Umbraco.Core.Sync; namespace Umbraco.Web.Scheduling { @@ -18,7 +14,7 @@ namespace Umbraco.Web.Scheduling /// internal sealed class Scheduler : ApplicationEventHandler { - private static Timer _pingTimer; + private static BackgroundTaskRunner _keepAliveRunner; private static BackgroundTaskRunner _publishingRunner; private static BackgroundTaskRunner _tasksRunner; private static BackgroundTaskRunner _scrubberRunner; @@ -48,30 +44,24 @@ namespace Umbraco.Web.Scheduling LogHelper.Debug(() => "Initializing the scheduler"); // backgrounds runners are web aware, if the app domain dies, these tasks will wind down correctly + _keepAliveRunner = new BackgroundTaskRunner("KeepAlive", applicationContext.ProfilingLogger.Logger); _publishingRunner = new BackgroundTaskRunner("ScheduledPublishing", applicationContext.ProfilingLogger.Logger); _tasksRunner = new BackgroundTaskRunner("ScheduledTasks", applicationContext.ProfilingLogger.Logger); _scrubberRunner = new BackgroundTaskRunner("LogScrubber", applicationContext.ProfilingLogger.Logger); var settings = UmbracoConfig.For.UmbracoSettings(); - // note - // must use the single-parameter constructor on Timer to avoid it from being GC'd - // also make the timer static to ensure further GC safety - // read http://stackoverflow.com/questions/4962172/why-does-a-system-timers-timer-survive-gc-but-not-system-threading-timer - - // ping/keepalive - no need for a background runner - does not need to be web aware, ok if the app domain dies - _pingTimer = new Timer(state => KeepAlive.Start(applicationContext, UmbracoConfig.For.UmbracoSettings())); - _pingTimer.Change(60000, 300000); + // ping/keepalive + // on all servers + _keepAliveRunner.Add(new KeepAlive(_keepAliveRunner, 60000, 300000, applicationContext)); // scheduled publishing/unpublishing // install on all, will only run on non-slaves servers - // both are delayed recurring tasks _publishingRunner.Add(new ScheduledPublishing(_publishingRunner, 60000, 60000, applicationContext, settings)); _tasksRunner.Add(new ScheduledTasks(_tasksRunner, 60000, 60000, applicationContext, settings)); // log scrubbing - // install & run on all servers - // LogScrubber is a delayed recurring task + // install on all, will only run on non-slaves servers _scrubberRunner.Add(new LogScrubber(_scrubberRunner, 60000, LogScrubber.GetLogScrubbingInterval(settings), applicationContext, settings)); } } diff --git a/src/Umbraco.Web/Umbraco.Web.csproj b/src/Umbraco.Web/Umbraco.Web.csproj index f4ff5df589..c4adb2ab23 100644 --- a/src/Umbraco.Web/Umbraco.Web.csproj +++ b/src/Umbraco.Web/Umbraco.Web.csproj @@ -304,6 +304,7 @@ + @@ -558,7 +559,6 @@ -