From d640fa65e892fde6c912a03c72fd1b599a4bfabc Mon Sep 17 00:00:00 2001 From: "kay.one" Date: Sun, 10 Jul 2011 21:53:34 -0700 Subject: [PATCH] Fixed orphaned job issue in JobController System/Jobs now shows items currently in queue. --- NzbDrone.Core.Test/Framework/TestBase.cs | 2 + NzbDrone.Core.Test/JobProviderTest.cs | 128 ++++++++++++++----- NzbDrone.Core/Providers/Jobs/JobProvider.cs | 66 ++++------ NzbDrone.Web/Controllers/SystemController.cs | 2 + NzbDrone.Web/NzbDrone.Web.csproj | 1 + NzbDrone.Web/Views/System/Jobs.cshtml | 12 +- 6 files changed, 137 insertions(+), 74 deletions(-) diff --git a/NzbDrone.Core.Test/Framework/TestBase.cs b/NzbDrone.Core.Test/Framework/TestBase.cs index 900d071a5..8e403b880 100644 --- a/NzbDrone.Core.Test/Framework/TestBase.cs +++ b/NzbDrone.Core.Test/Framework/TestBase.cs @@ -1,4 +1,5 @@ using NUnit.Framework; +using NzbDrone.Core.Providers.Jobs; namespace NzbDrone.Core.Test.Framework { @@ -17,6 +18,7 @@ namespace NzbDrone.Core.Test.Framework [TearDown] public void TearDown() { + JobProvider.Queue.Clear(); ExceptionVerification.AssertNoUnexcpectedLogs(); } diff --git a/NzbDrone.Core.Test/JobProviderTest.cs b/NzbDrone.Core.Test/JobProviderTest.cs index 8bf8cf543..7cf9128c7 100644 --- a/NzbDrone.Core.Test/JobProviderTest.cs +++ b/NzbDrone.Core.Test/JobProviderTest.cs @@ -1,9 +1,11 @@ // ReSharper disable RedundantUsingDirective using System; +using System.Collections; using System.Collections.Generic; using System.Threading; using AutoMoq; using FluentAssertions; +using Moq; using NUnit.Framework; using NzbDrone.Core.Model.Notification; using NzbDrone.Core.Providers.Jobs; @@ -31,9 +33,8 @@ namespace NzbDrone.Core.Test //Assert var settings = timerProvider.All(); - Assert.IsNotEmpty(settings); + Assert.AreNotEqual(DateTime.MinValue, settings[0].LastExecution); - settings[0].Success.Should().BeTrue(); } [Test] @@ -51,32 +52,33 @@ namespace NzbDrone.Core.Test timerProvider.Initialize(); timerProvider.RunScheduled(); + Thread.Sleep(1000); + //Assert var settings = timerProvider.All(); - Assert.IsNotEmpty(settings); Assert.AreNotEqual(DateTime.MinValue, settings[0].LastExecution); - Assert.IsFalse(settings[0].Success); + settings[0].Success.Should().BeFalse(); ExceptionVerification.ExcpectedErrors(1); } [Test] - //This test will confirm that the concurrency checks are rest - //after execution so the job can successfully run. - public void can_run_job_again() + public void scheduler_skips_jobs_that_arent_mature_yet() { - IList fakeJobs = new List { new FakeJob() }; + var fakeJob = new FakeJob(); var mocker = new AutoMoqer(); + IList fakeJobs = new List { fakeJob }; mocker.SetConstant(MockLib.GetEmptyDatabase()); mocker.SetConstant(fakeJobs); var timerProvider = mocker.Resolve(); timerProvider.Initialize(); - var firstRun = timerProvider.RunScheduled(); - var secondRun = timerProvider.RunScheduled(); + timerProvider.RunScheduled(); + Thread.Sleep(500); + timerProvider.RunScheduled(); + Thread.Sleep(500); - firstRun.Should().BeTrue(); - secondRun.Should().BeTrue(); + fakeJob.ExexutionCount.Should().Be(1); } [Test] @@ -84,21 +86,21 @@ namespace NzbDrone.Core.Test //after execution so the job can successfully run. public void can_run_async_job_again() { - IList fakeJobs = new List { new FakeJob() }; + var fakeJob = new FakeJob(); var mocker = new AutoMoqer(); - + + IList fakeJobs = new List {fakeJob}; mocker.SetConstant(MockLib.GetEmptyDatabase()); mocker.SetConstant(fakeJobs); var timerProvider = mocker.Resolve(); timerProvider.Initialize(); - var firstRun = timerProvider.QueueJob(typeof(FakeJob)); - Thread.Sleep(2000); - var secondRun = timerProvider.QueueJob(typeof(FakeJob)); - - firstRun.Should().BeTrue(); - secondRun.Should().BeTrue(); + timerProvider.QueueJob(typeof(FakeJob)); + Thread.Sleep(1000); + timerProvider.QueueJob(typeof(FakeJob)); + Thread.Sleep(1000); JobProvider.Queue.Should().BeEmpty(); + fakeJob.ExexutionCount.Should().Be(2); } [Test] @@ -130,7 +132,8 @@ namespace NzbDrone.Core.Test //after execution so the job can successfully run. public void can_run_broken_async_job_again() { - IList fakeJobs = new List { new BrokenJob() }; + var brokenJob = new BrokenJob(); + IList fakeJobs = new List { brokenJob }; var mocker = new AutoMoqer(); mocker.SetConstant(MockLib.GetEmptyDatabase()); @@ -138,14 +141,14 @@ namespace NzbDrone.Core.Test var timerProvider = mocker.Resolve(); timerProvider.Initialize(); - var firstRun = timerProvider.QueueJob(typeof(BrokenJob)); + timerProvider.QueueJob(typeof(BrokenJob)); Thread.Sleep(2000); - var secondRun = timerProvider.QueueJob(typeof(BrokenJob)); + timerProvider.QueueJob(typeof(BrokenJob)); + - firstRun.Should().BeTrue(); - secondRun.Should().BeTrue(); Thread.Sleep(2000); JobProvider.Queue.Should().BeEmpty(); + brokenJob.ExexutionCount.Should().Be(2); ExceptionVerification.ExcpectedErrors(2); } @@ -154,7 +157,8 @@ namespace NzbDrone.Core.Test //after execution so the job can successfully run. public void can_run_two_jobs_at_the_same_time() { - IList fakeJobs = new List { new SlowJob() }; + var slowJob = new SlowJob(); + IList fakeJobs = new List { slowJob }; var mocker = new AutoMoqer(); mocker.SetConstant(MockLib.GetEmptyDatabase()); @@ -163,20 +167,18 @@ namespace NzbDrone.Core.Test var timerProvider = mocker.Resolve(); timerProvider.Initialize(); - bool firstRun = false; - bool secondRun = false; - var thread1 = new Thread(() => firstRun = timerProvider.RunScheduled()); + var thread1 = new Thread(() => timerProvider.RunScheduled()); thread1.Start(); Thread.Sleep(1000); - var thread2 = new Thread(() => secondRun = timerProvider.RunScheduled()); + var thread2 = new Thread(() => timerProvider.RunScheduled()); thread2.Start(); thread1.Join(); thread2.Join(); - firstRun.Should().BeTrue(); - Assert.IsFalse(secondRun); + + slowJob.ExexutionCount = 2; } @@ -389,6 +391,61 @@ namespace NzbDrone.Core.Test Assert.IsNotEmpty(settings); Assert.IsFalse(settings[0].Success); } + + + [Test] + public void existing_queue_should_start_queue_if_not_running() + { + var mocker = new AutoMoqer(); + + var fakeJob = new FakeJob(); + IList fakeJobs = new List { fakeJob }; + + + mocker.SetConstant(MockLib.GetEmptyDatabase()); + mocker.SetConstant(fakeJobs); + + var fakeQueueItem = new Tuple(fakeJob.GetType(), 12); + //Act + var jobProvider = mocker.Resolve(); + jobProvider.Initialize(); + JobProvider.Queue.Add(fakeQueueItem); + jobProvider.QueueJob(fakeJob.GetType(), 12); + Thread.Sleep(1000); + + //Assert + fakeJob.ExexutionCount.Should().Be(1); + } + + + [Test] + public void Item_added_to_queue_while_scheduler_runs_is_executed() + { + var mocker = new AutoMoqer(); + + var slowJob = new SlowJob(); + var disabledJob = new DisabledJob(); + IList fakeJobs = new List { slowJob, disabledJob }; + + mocker.SetConstant(MockLib.GetEmptyDatabase()); + mocker.SetConstant(fakeJobs); + + mocker.Resolve().Initialize(); + + var _jobThread = new Thread(() => mocker.Resolve().RunScheduled()); + _jobThread.Start(); + + Thread.Sleep(200); + + mocker.Resolve().QueueJob(typeof(DisabledJob), 12); + + Thread.Sleep(3000); + + //Assert + JobProvider.Queue.Should().BeEmpty(); + slowJob.ExexutionCount.Should().Be(1); + disabledJob.ExexutionCount.Should().Be(1); + } } public class FakeJob : IJob @@ -403,9 +460,11 @@ namespace NzbDrone.Core.Test get { return 15; } } + public int ExexutionCount { get; set; } + public void Start(ProgressNotification notification, int targetId) { - + ExexutionCount++; } } @@ -441,8 +500,11 @@ namespace NzbDrone.Core.Test get { return 15; } } + public int ExexutionCount { get; set; } + public void Start(ProgressNotification notification, int targetId) { + ExexutionCount++; throw new ApplicationException("Broken job is broken"); } } diff --git a/NzbDrone.Core/Providers/Jobs/JobProvider.cs b/NzbDrone.Core/Providers/Jobs/JobProvider.cs index 068b21f96..4c15457b1 100644 --- a/NzbDrone.Core/Providers/Jobs/JobProvider.cs +++ b/NzbDrone.Core/Providers/Jobs/JobProvider.cs @@ -67,40 +67,31 @@ namespace NzbDrone.Core.Providers.Jobs /// /// Iterates through all registered jobs and executed any that are due for an execution. /// - /// True if ran, false if skipped - public virtual bool RunScheduled() + public virtual void RunScheduled() { lock (ExecutionLock) { if (_isRunning) { Logger.Trace("Queue is already running. Ignoring scheduler's request."); - return false; + return; } - _isRunning = true; } - try - { - var pendingJobs = All().Where( - t => t.Enable && - (DateTime.Now - t.LastExecution) > TimeSpan.FromMinutes(t.Interval) - ); + var counter = 0; - foreach (var pendingTimer in pendingJobs) - { - var timer = pendingTimer; - var timerClass = _jobs.Where(t => t.GetType().ToString() == timer.TypeName).FirstOrDefault(); - Execute(timerClass.GetType()); - } - } - finally + var pendingJobs = All().Where( + t => t.Enable && + (DateTime.Now - t.LastExecution) > TimeSpan.FromMinutes(t.Interval) + ).Select(c => _jobs.Where(t => t.GetType().ToString() == c.TypeName).Single()); + + foreach (var job in pendingJobs) { - _isRunning = false; + QueueJob(job.GetType()); + counter++; } - Logger.Trace("Finished executing scheduled tasks."); - return true; + Logger.Trace("{0} Scheduled tasks have been added to the queue", counter); } /// @@ -109,32 +100,33 @@ namespace NzbDrone.Core.Providers.Jobs /// Type of the job that should be executed. /// The targetId could be any Id parameter eg. SeriesId. it will be passed to the job implementation /// to allow it to filter it's target of execution. - /// True if queued, false if duplicate and was skipped /// Job is only added to the queue if same job with the same targetId doesn't already exist in the queue. - public virtual bool QueueJob(Type jobType, int targetId = 0) + public virtual void QueueJob(Type jobType, int targetId = 0) { Logger.Debug("Adding [{0}:{1}] to the queue", jobType.Name, targetId); - lock (Queue) - { - var queueTuple = new Tuple(jobType, targetId); - if (Queue.Contains(queueTuple)) + lock (ExecutionLock) + { + lock (Queue) { - Logger.Info("[{0}:{1}] already exists in job queue. Skipping.", jobType.Name, targetId); - return false; - } + var queueTuple = new Tuple(jobType, targetId); - Queue.Add(queueTuple); - Logger.Trace("Job [{0}:{1}] added to the queue", jobType.Name, targetId); + if (!Queue.Contains(queueTuple)) + { + Queue.Add(queueTuple); + Logger.Trace("Job [{0}:{1}] added to the queue", jobType.Name, targetId); - } + } + else + { + Logger.Info("[{0}:{1}] already exists in job queue. Skipping.", jobType.Name, targetId); + } + } - lock (ExecutionLock) - { if (_isRunning) { Logger.Trace("Queue is already running. No need to start it up."); - return true; + return; } _isRunning = true; } @@ -166,10 +158,8 @@ namespace NzbDrone.Core.Providers.Jobs else { Logger.Error("Execution lock has fucked up. Thread still active. Ignoring request."); - return true; } - return true; } /// diff --git a/NzbDrone.Web/Controllers/SystemController.cs b/NzbDrone.Web/Controllers/SystemController.cs index 3a9966ca7..ec1a15d2e 100644 --- a/NzbDrone.Web/Controllers/SystemController.cs +++ b/NzbDrone.Web/Controllers/SystemController.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Collections.Generic; using System.IO; using System.Web.Mvc; @@ -28,6 +29,7 @@ namespace NzbDrone.Web.Controllers public ActionResult Jobs() { + ViewData["Queue"] = JobProvider.Queue.Select(c => new Tuple(c.Item1.Name, c.Item2)); return View(_jobProvider.All()); } diff --git a/NzbDrone.Web/NzbDrone.Web.csproj b/NzbDrone.Web/NzbDrone.Web.csproj index 5e14a3267..0d6a9a2b6 100644 --- a/NzbDrone.Web/NzbDrone.Web.csproj +++ b/NzbDrone.Web/NzbDrone.Web.csproj @@ -45,6 +45,7 @@ x86 + False ..\Libraries\MVC3\Microsoft.Web.Infrastructure.dll diff --git a/NzbDrone.Web/Views/System/Jobs.cshtml b/NzbDrone.Web/Views/System/Jobs.cshtml index 4c9201715..25d266f33 100644 --- a/NzbDrone.Web/Views/System/Jobs.cshtml +++ b/NzbDrone.Web/Views/System/Jobs.cshtml @@ -1,9 +1,15 @@ -@model IEnumerable +@using System.Collections +@model IEnumerable @section TitleContent{ Jobs } @section MainContent{ @{Html.Telerik().Grid(Model).Name("Grid") - .TableHtmlAttributes(new { @class = "Grid" }) - .Render();} + .Render();} + + Items currently in queue + + @{Html.Telerik().Grid((IEnumerable>)ViewData["Queue"]).Name("QueueGrid") + .Columns(c => c.Bound(g => g.Item1).Title("Type").Width(100)).Columns(c => c.Bound(g => g.Item2).Title("Target")) + .Render();} }