From 4e10d30cf684c14559d4fb578dcd273e0fdf4b8f Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sat, 13 May 2017 23:11:05 +0200 Subject: [PATCH] Added Status refreshes to Download Monitoring Service and allow DownloadService to report success (but not failure). --- .../Checks/IndexerStatusCheckFixture.cs | 7 --- .../IndexerStatusServiceFixture.cs | 30 --------- .../Download/DownloadClientFactory.cs | 62 +++++++++++++++++-- .../Download/DownloadClientProvider.cs | 9 +-- src/NzbDrone.Core/Download/DownloadService.cs | 14 +++-- .../DownloadMonitoringService.cs | 28 +++++---- .../HealthCheck/Checks/DownloadClientCheck.cs | 2 +- .../Checks/DownloadClientStatusCheck.cs | 1 - .../Checks/ImportMechanismCheck.cs | 2 +- .../HealthCheck/Checks/IndexerStatusCheck.cs | 1 - 10 files changed, 85 insertions(+), 71 deletions(-) diff --git a/src/NzbDrone.Core.Test/HealthCheck/Checks/IndexerStatusCheckFixture.cs b/src/NzbDrone.Core.Test/HealthCheck/Checks/IndexerStatusCheckFixture.cs index d9b2f6095..50beaae7e 100644 --- a/src/NzbDrone.Core.Test/HealthCheck/Checks/IndexerStatusCheckFixture.cs +++ b/src/NzbDrone.Core.Test/HealthCheck/Checks/IndexerStatusCheckFixture.cs @@ -57,13 +57,6 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks { Subject.Check().ShouldBeOk(); } - [Test] - public void should_not_return_error_when_indexer_failed_less_than_an_hour() - { - GivenIndexer(1, 0.1, 0.5); - - Subject.Check().ShouldBeOk(); - } [Test] public void should_return_warning_if_indexer_unavailable() diff --git a/src/NzbDrone.Core.Test/IndexerTests/IndexerStatusServiceFixture.cs b/src/NzbDrone.Core.Test/IndexerTests/IndexerStatusServiceFixture.cs index 17a2eeaa6..646212116 100644 --- a/src/NzbDrone.Core.Test/IndexerTests/IndexerStatusServiceFixture.cs +++ b/src/NzbDrone.Core.Test/IndexerTests/IndexerStatusServiceFixture.cs @@ -41,21 +41,6 @@ namespace NzbDrone.Core.Test.IndexerTests .Verify(v => v.Upsert(It.IsAny()), Times.Never()); } - [Test] - public void should_start_backoff_on_first_failure() - { - WithStatus(new IndexerStatus()); - - Subject.RecordFailure(1); - - VerifyUpdate(); - - var status = Subject.GetBlockedProviders().FirstOrDefault(); - status.Should().NotBeNull(); - status.DisabledTill.Should().HaveValue(); - status.DisabledTill.Value.Should().BeCloseTo(_epoch + TimeSpan.FromMinutes(5), 500); - } - [Test] public void should_cancel_backoff_on_success() { @@ -78,20 +63,5 @@ namespace NzbDrone.Core.Test.IndexerTests VerifyNoUpdate(); } - - [Test] - public void should_preserve_escalation_on_intermittent_success() - { - WithStatus(new IndexerStatus { MostRecentFailure = _epoch - TimeSpan.FromSeconds(4), EscalationLevel = 3 }); - - Subject.RecordSuccess(1); - Subject.RecordSuccess(1); - Subject.RecordFailure(1); - - var status = Subject.GetBlockedProviders().FirstOrDefault(); - status.Should().NotBeNull(); - status.DisabledTill.Should().HaveValue(); - status.DisabledTill.Value.Should().BeCloseTo(_epoch + TimeSpan.FromMinutes(15), 500); - } } } diff --git a/src/NzbDrone.Core/Download/DownloadClientFactory.cs b/src/NzbDrone.Core/Download/DownloadClientFactory.cs index dc0f218b5..21a049f10 100644 --- a/src/NzbDrone.Core/Download/DownloadClientFactory.cs +++ b/src/NzbDrone.Core/Download/DownloadClientFactory.cs @@ -1,5 +1,7 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Linq; +using FluentValidation.Results; using NLog; using NzbDrone.Common.Composition; using NzbDrone.Core.Messaging.Events; @@ -9,17 +11,24 @@ namespace NzbDrone.Core.Download { public interface IDownloadClientFactory : IProviderFactory { - + List DownloadHandlingEnabled(bool filterBlockedClients = true); } public class DownloadClientFactory : ProviderFactory, IDownloadClientFactory { - private readonly IDownloadClientRepository _providerRepository; + private readonly IDownloadClientStatusService _downloadClientStatusService; + private readonly Logger _logger; - public DownloadClientFactory(IDownloadClientRepository providerRepository, IEnumerable providers, IContainer container, IEventAggregator eventAggregator, Logger logger) + public DownloadClientFactory(IDownloadClientStatusService downloadClientStatusService, + IDownloadClientRepository providerRepository, + IEnumerable providers, + IContainer container, + IEventAggregator eventAggregator, + Logger logger) : base(providerRepository, providers, container, eventAggregator, logger) { - _providerRepository = providerRepository; + _downloadClientStatusService = downloadClientStatusService; + _logger = logger; } protected override List Active() @@ -33,5 +42,46 @@ namespace NzbDrone.Core.Download definition.Protocol = provider.Protocol; } + + public List DownloadHandlingEnabled(bool filterBlockedClients = true) + { + var enabledClients = GetAvailableProviders(); + + if (filterBlockedClients) + { + return FilterBlockedClients(enabledClients).ToList(); + } + + return enabledClients.ToList(); + } + + private IEnumerable FilterBlockedClients(IEnumerable clients) + { + var blockedIndexers = _downloadClientStatusService.GetBlockedProviders().ToDictionary(v => v.ProviderId, v => v); + + foreach (var client in clients) + { + DownloadClientStatus downloadClientStatus; + if (blockedIndexers.TryGetValue(client.Definition.Id, out downloadClientStatus)) + { + _logger.Debug("Temporarily ignoring download client {0} till {1} due to recent failures.", client.Definition.Name, downloadClientStatus.DisabledTill.Value.ToLocalTime()); + continue; + } + + yield return client; + } + } + + public override ValidationResult Test(DownloadClientDefinition definition) + { + var result = base.Test(definition); + + if (result == null && definition.Id != 0) + { + _downloadClientStatusService.RecordSuccess(definition.Id); + } + + return result; + } } -} \ No newline at end of file +} diff --git a/src/NzbDrone.Core/Download/DownloadClientProvider.cs b/src/NzbDrone.Core/Download/DownloadClientProvider.cs index 5cb899806..0c8ab03ba 100644 --- a/src/NzbDrone.Core/Download/DownloadClientProvider.cs +++ b/src/NzbDrone.Core/Download/DownloadClientProvider.cs @@ -27,19 +27,12 @@ namespace NzbDrone.Core.Download public IEnumerable GetDownloadClients() { - return _downloadClientFactory.GetAvailableProviders();//.Select(MapDownloadClient); + return _downloadClientFactory.GetAvailableProviders(); } public IDownloadClient Get(int id) { return _downloadClientFactory.GetAvailableProviders().Single(d => d.Definition.Id == id); } - - public IDownloadClient MapDownloadClient(IDownloadClient downloadClient) - { - _downloadClientFactory.SetProviderCharacteristics(downloadClient, (DownloadClientDefinition)downloadClient.Definition); - - return downloadClient; - } } } diff --git a/src/NzbDrone.Core/Download/DownloadService.cs b/src/NzbDrone.Core/Download/DownloadService.cs index 4f76b1507..6580002e8 100644 --- a/src/NzbDrone.Core/Download/DownloadService.cs +++ b/src/NzbDrone.Core/Download/DownloadService.cs @@ -21,18 +21,21 @@ namespace NzbDrone.Core.Download public class DownloadService : IDownloadService { private readonly IProvideDownloadClient _downloadClientProvider; + private readonly IDownloadClientStatusService _downloadClientStatusService; private readonly IIndexerStatusService _indexerStatusService; private readonly IRateLimitService _rateLimitService; private readonly IEventAggregator _eventAggregator; private readonly Logger _logger; public DownloadService(IProvideDownloadClient downloadClientProvider, - IIndexerStatusService indexerStatusService, - IRateLimitService rateLimitService, - IEventAggregator eventAggregator, - Logger logger) + IDownloadClientStatusService downloadClientStatusService, + IIndexerStatusService indexerStatusService, + IRateLimitService rateLimitService, + IEventAggregator eventAggregator, + Logger logger) { _downloadClientProvider = downloadClientProvider; + _downloadClientStatusService = downloadClientStatusService; _indexerStatusService = indexerStatusService; _rateLimitService = rateLimitService; _eventAggregator = eventAggregator; @@ -64,6 +67,7 @@ namespace NzbDrone.Core.Download try { downloadClientId = downloadClient.Download(remoteEpisode); + _downloadClientStatusService.RecordSuccess(downloadClient.Definition.Id); _indexerStatusService.RecordSuccess(remoteEpisode.Release.IndexerId); } catch (ReleaseDownloadException ex) @@ -92,4 +96,4 @@ namespace NzbDrone.Core.Download _eventAggregator.PublishEvent(episodeGrabbedEvent); } } -} \ No newline at end of file +} diff --git a/src/NzbDrone.Core/Download/TrackedDownloads/DownloadMonitoringService.cs b/src/NzbDrone.Core/Download/TrackedDownloads/DownloadMonitoringService.cs index 4fb031dc5..bf375a797 100644 --- a/src/NzbDrone.Core/Download/TrackedDownloads/DownloadMonitoringService.cs +++ b/src/NzbDrone.Core/Download/TrackedDownloads/DownloadMonitoringService.cs @@ -15,7 +15,8 @@ namespace NzbDrone.Core.Download.TrackedDownloads IHandle, IHandle { - private readonly IProvideDownloadClient _downloadClientProvider; + private readonly IDownloadClientStatusService _downloadClientStatusService; + private readonly IDownloadClientFactory _downloadClientFactory; private readonly IEventAggregator _eventAggregator; private readonly IManageCommandQueue _manageCommandQueue; private readonly IConfigService _configService; @@ -25,16 +26,18 @@ namespace NzbDrone.Core.Download.TrackedDownloads private readonly Logger _logger; private readonly Debouncer _refreshDebounce; - public DownloadMonitoringService(IProvideDownloadClient downloadClientProvider, - IEventAggregator eventAggregator, - IManageCommandQueue manageCommandQueue, - IConfigService configService, - IFailedDownloadService failedDownloadService, - ICompletedDownloadService completedDownloadService, - ITrackedDownloadService trackedDownloadService, - Logger logger) + public DownloadMonitoringService(IDownloadClientStatusService downloadClientStatusService, + IDownloadClientFactory downloadClientFactory, + IEventAggregator eventAggregator, + IManageCommandQueue manageCommandQueue, + IConfigService configService, + IFailedDownloadService failedDownloadService, + ICompletedDownloadService completedDownloadService, + ITrackedDownloadService trackedDownloadService, + Logger logger) { - _downloadClientProvider = downloadClientProvider; + _downloadClientStatusService = downloadClientStatusService; + _downloadClientFactory = downloadClientFactory; _eventAggregator = eventAggregator; _manageCommandQueue = manageCommandQueue; _configService = configService; @@ -56,7 +59,7 @@ namespace NzbDrone.Core.Download.TrackedDownloads _refreshDebounce.Pause(); try { - var downloadClients = _downloadClientProvider.GetDownloadClients(); + var downloadClients = _downloadClientFactory.DownloadHandlingEnabled(); var trackedDownloads = new List(); @@ -84,9 +87,12 @@ namespace NzbDrone.Core.Download.TrackedDownloads try { downloadClientHistory = downloadClient.GetItems().ToList(); + + _downloadClientStatusService.RecordSuccess(downloadClient.Definition.Id); } catch (Exception ex) { + _downloadClientStatusService.RecordFailure(downloadClient.Definition.Id); _logger.Warn(ex, "Unable to retrieve queue and history items from " + downloadClient.Definition.Name); } diff --git a/src/NzbDrone.Core/HealthCheck/Checks/DownloadClientCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/DownloadClientCheck.cs index ea3b87586..4aceb0f2d 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/DownloadClientCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/DownloadClientCheck.cs @@ -36,7 +36,7 @@ namespace NzbDrone.Core.HealthCheck.Checks _logger.Debug(ex, "Unable to communicate with {0}", downloadClient.Definition.Name); var message = $"Unable to communicate with {downloadClient.Definition.Name}."; - return new HealthCheck(GetType(), HealthCheckResult.Error, $"{message} {ex.Message}"); + return new HealthCheck(GetType(), HealthCheckResult.Error, $"{message} {ex.Message}", "#unable-to-communicate-with-download-client"); } } diff --git a/src/NzbDrone.Core/HealthCheck/Checks/DownloadClientStatusCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/DownloadClientStatusCheck.cs index bf3f268b8..e003f7fa4 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/DownloadClientStatusCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/DownloadClientStatusCheck.cs @@ -23,7 +23,6 @@ namespace NzbDrone.Core.HealthCheck.Checks i => i.Definition.Id, s => s.ProviderId, (i, s) => new { Provider = i, Status = s }) - .Where(v => (v.Status.MostRecentFailure - v.Status.InitialFailure) > TimeSpan.FromHours(1)) .ToList(); if (backOffProviders.Empty()) diff --git a/src/NzbDrone.Core/HealthCheck/Checks/ImportMechanismCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/ImportMechanismCheck.cs index 2fb2d3e2d..e2ffc7ce3 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/ImportMechanismCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/ImportMechanismCheck.cs @@ -35,7 +35,7 @@ namespace NzbDrone.Core.HealthCheck.Checks Status = v.GetStatus() }).ToList(); } - catch (DownloadClientException) + catch (Exception) { // One or more download clients failed, assume the health is okay and verify later return new HealthCheck(GetType()); diff --git a/src/NzbDrone.Core/HealthCheck/Checks/IndexerStatusCheck.cs b/src/NzbDrone.Core/HealthCheck/Checks/IndexerStatusCheck.cs index 5226ce7b7..a9bd2802c 100644 --- a/src/NzbDrone.Core/HealthCheck/Checks/IndexerStatusCheck.cs +++ b/src/NzbDrone.Core/HealthCheck/Checks/IndexerStatusCheck.cs @@ -23,7 +23,6 @@ namespace NzbDrone.Core.HealthCheck.Checks i => i.Definition.Id, s => s.ProviderId, (i, s) => new { Provider = i, Status = s }) - .Where(v => (v.Status.MostRecentFailure - v.Status.InitialFailure) > TimeSpan.FromHours(1)) .ToList(); if (backOffProviders.Empty())