Fixed: Prevent Download Client from being queried every minute if it failed repeatedly similar to Indexer temporarily disabled logic.

This commit is contained in:
Taloth Saldono 2017-05-12 21:42:17 +02:00
parent f4bea5512c
commit f335cc1af8
19 changed files with 431 additions and 23 deletions

View File

@ -0,0 +1,106 @@
using System;
using System.Linq;
using FluentAssertions;
using Moq;
using NUnit.Framework;
using NzbDrone.Core.Download;
using NzbDrone.Core.Test.Framework;
namespace NzbDrone.Core.Test.Download
{
public class DownloadClientStatusServiceFixture : CoreTest<DownloadClientStatusService>
{
private DateTime _epoch;
[SetUp]
public void SetUp()
{
_epoch = DateTime.UtcNow;
}
private void WithStatus(DownloadClientStatus status)
{
Mocker.GetMock<IDownloadClientStatusRepository>()
.Setup(v => v.FindByProviderId(1))
.Returns(status);
Mocker.GetMock<IDownloadClientStatusRepository>()
.Setup(v => v.All())
.Returns(new[] { status });
}
private void VerifyUpdate()
{
Mocker.GetMock<IDownloadClientStatusRepository>()
.Verify(v => v.Upsert(It.IsAny<DownloadClientStatus>()), Times.Once());
}
private void VerifyNoUpdate()
{
Mocker.GetMock<IDownloadClientStatusRepository>()
.Verify(v => v.Upsert(It.IsAny<DownloadClientStatus>()), Times.Never());
}
[Test]
public void should_not_consider_blocked_within_5_minutes_since_initial_failure()
{
WithStatus(new DownloadClientStatus
{
InitialFailure = _epoch - TimeSpan.FromMinutes(4),
MostRecentFailure = _epoch - TimeSpan.FromSeconds(4),
EscalationLevel = 3
});
Subject.RecordFailure(1);
VerifyUpdate();
var status = Subject.GetBlockedProviders().FirstOrDefault();
status.Should().BeNull();
}
[Test]
public void should_consider_blocked_after_5_minutes_since_initial_failure()
{
WithStatus(new DownloadClientStatus
{
InitialFailure = _epoch - TimeSpan.FromMinutes(6),
MostRecentFailure = _epoch - TimeSpan.FromSeconds(120),
EscalationLevel = 3
});
Subject.RecordFailure(1);
VerifyUpdate();
var status = Subject.GetBlockedProviders().FirstOrDefault();
status.Should().NotBeNull();
}
[Test]
public void should_not_escalate_beyond_3_hours()
{
WithStatus(new DownloadClientStatus
{
InitialFailure = _epoch - TimeSpan.FromMinutes(6),
MostRecentFailure = _epoch - TimeSpan.FromSeconds(120),
EscalationLevel = 3
});
Subject.RecordFailure(1);
Subject.RecordFailure(1);
Subject.RecordFailure(1);
Subject.RecordFailure(1);
Subject.RecordFailure(1);
Subject.RecordFailure(1);
Subject.RecordFailure(1);
Subject.RecordFailure(1);
Subject.RecordFailure(1);
var status = Subject.GetBlockedProviders().FirstOrDefault();
status.Should().NotBeNull();
status.DisabledTill.Should().HaveValue();
status.DisabledTill.Should().NotBeAfter(_epoch + TimeSpan.FromHours(3.1));
}
}
}

View File

@ -3,7 +3,6 @@ using System.Collections.Generic;
using NUnit.Framework;
using NzbDrone.Core.Download;
using NzbDrone.Core.HealthCheck.Checks;
using NzbDrone.Core.Indexers;
using NzbDrone.Core.Test.Framework;
using NzbDrone.Test.Common;
@ -26,7 +25,7 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks
public void should_return_error_when_download_client_throws()
{
var downloadClient = Mocker.GetMock<IDownloadClient>();
downloadClient.Setup(s => s.Definition).Returns(new IndexerDefinition{Name = "Test"});
downloadClient.Setup(s => s.Definition).Returns(new DownloadClientDefinition{Name = "Test"});
downloadClient.Setup(s => s.GetItems())
.Throws<Exception>();
@ -36,8 +35,6 @@ namespace NzbDrone.Core.Test.HealthCheck.Checks
.Returns(new IDownloadClient[] { downloadClient.Object });
Subject.Check().ShouldBeError();
ExceptionVerification.ExpectedErrors(1);
}
[Test]

View File

@ -29,10 +29,16 @@ namespace NzbDrone.Core.Test.IndexerTests
.Returns(new[] { status });
}
private void VerifyUpdate(bool updated = true)
private void VerifyUpdate()
{
Mocker.GetMock<IIndexerStatusRepository>()
.Verify(v => v.Upsert(It.IsAny<IndexerStatus>()), Times.Exactly(updated ? 1 : 0));
.Verify(v => v.Upsert(It.IsAny<IndexerStatus>()), Times.Once());
}
private void VerifyNoUpdate()
{
Mocker.GetMock<IIndexerStatusRepository>()
.Verify(v => v.Upsert(It.IsAny<IndexerStatus>()), Times.Never());
}
[Test]
@ -70,7 +76,7 @@ namespace NzbDrone.Core.Test.IndexerTests
Subject.RecordSuccess(1);
VerifyUpdate(false);
VerifyNoUpdate();
}
[Test]

View File

@ -178,6 +178,7 @@
<Compile Include="Download\DownloadClientTests\DownloadStationTests\SharedFolderResolverFixture.cs" />
<Compile Include="Download\DownloadClientTests\DownloadStationTests\UsenetDownloadStationFixture.cs" />
<Compile Include="Download\DownloadClientTests\HadoukenTests\HadoukenFixture.cs" />
<Compile Include="Download\DownloadClientStatusServiceFixture.cs" />
<Compile Include="Download\DownloadClientTests\NzbgetTests\NzbgetFixture.cs" />
<Compile Include="Download\DownloadClientTests\NzbVortexTests\NzbVortexFixture.cs" />
<Compile Include="Download\DownloadClientTests\PneumaticProviderFixture.cs" />
@ -369,7 +370,8 @@
<Compile Include="Qualities\QualityModelComparerFixture.cs" />
<Compile Include="RootFolderTests\RootFolderServiceFixture.cs" />
<Compile Include="SeriesStatsTests\SeriesStatisticsFixture.cs" />
<Compile Include="ThingiProvider\ProviderBaseFixture.cs" />
<Compile Include="ThingiProviderTests\ProviderStatusServiceFixture.cs" />
<Compile Include="ThingiProviderTests\ProviderBaseFixture.cs" />
<Compile Include="ThingiProviderTests\NullConfigFixture.cs" />
<Compile Include="TvTests\EpisodeServiceTests\FindEpisodeByTitleFixture.cs" />
<Compile Include="TvTests\EpisodeServiceTests\HandleEpisodeFileDeletedFixture.cs" />

View File

@ -14,4 +14,4 @@ namespace NzbDrone.Core.Test.ThingiProviderTests
Subject.Validate().IsValid.Should().BeTrue();
}
}
}
}

View File

@ -5,9 +5,8 @@ using NzbDrone.Core.Indexers;
using NzbDrone.Core.Indexers.Newznab;
using NzbDrone.Core.Test.Framework;
namespace NzbDrone.Core.Test.ThingiProvider
namespace NzbDrone.Core.Test.ThingiProviderTests
{
public class ProviderRepositoryFixture : DbTest<IndexerRepository, IndexerDefinition>
{
[Test]
@ -27,4 +26,4 @@ namespace NzbDrone.Core.Test.ThingiProvider
storedSetting.ShouldBeEquivalentTo(newznabSettings, o=>o.IncludingAllRuntimeProperties());
}
}
}
}

View File

@ -0,0 +1,126 @@
using System;
using System.Linq;
using FluentAssertions;
using Moq;
using NLog;
using NUnit.Framework;
using NzbDrone.Core.Messaging.Events;
using NzbDrone.Core.Test.Framework;
using NzbDrone.Core.ThingiProvider;
using NzbDrone.Core.ThingiProvider.Status;
namespace NzbDrone.Core.Test.ThingiProviderTests
{
public class MockProviderStatus : ProviderStatusBase
{
}
public interface IMockProvider : IProvider
{
}
public interface IMockProviderStatusRepository : IProviderStatusRepository<MockProviderStatus>
{
}
public class MockProviderStatusService : ProviderStatusServiceBase<IMockProvider, MockProviderStatus>
{
public MockProviderStatusService(IMockProviderStatusRepository providerStatusRepository, IEventAggregator eventAggregator, Logger logger)
: base(providerStatusRepository, eventAggregator, logger)
{
}
}
public class ProviderStatusServiceFixture : CoreTest<MockProviderStatusService>
{
private DateTime _epoch;
[SetUp]
public void SetUp()
{
_epoch = DateTime.UtcNow;
}
private void WithStatus(MockProviderStatus status)
{
Mocker.GetMock<IMockProviderStatusRepository>()
.Setup(v => v.FindByProviderId(1))
.Returns(status);
Mocker.GetMock<IMockProviderStatusRepository>()
.Setup(v => v.All())
.Returns(new[] { status });
}
private void VerifyUpdate()
{
Mocker.GetMock<IMockProviderStatusRepository>()
.Verify(v => v.Upsert(It.IsAny<MockProviderStatus>()), Times.Once());
}
private void VerifyNoUpdate()
{
Mocker.GetMock<IMockProviderStatusRepository>()
.Verify(v => v.Upsert(It.IsAny<MockProviderStatus>()), Times.Never());
}
[Test]
public void should_start_backoff_on_first_failure()
{
WithStatus(new MockProviderStatus());
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()
{
WithStatus(new MockProviderStatus { EscalationLevel = 2 });
Subject.RecordSuccess(1);
VerifyUpdate();
var status = Subject.GetBlockedProviders().FirstOrDefault();
status.Should().BeNull();
}
[Test]
public void should_not_store_update_if_already_okay()
{
WithStatus(new MockProviderStatus { EscalationLevel = 0 });
Subject.RecordSuccess(1);
VerifyNoUpdate();
}
[Test]
public void should_preserve_escalation_on_intermittent_success()
{
WithStatus(new MockProviderStatus
{
InitialFailure = _epoch - TimeSpan.FromSeconds(20),
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);
}
}
}

View File

@ -0,0 +1,19 @@
using FluentMigrator;
using NzbDrone.Core.Datastore.Migration.Framework;
namespace NzbDrone.Core.Datastore.Migration
{
[Migration(115)]
public class add_downloadclient_status : NzbDroneMigrationBase
{
protected override void MainDbUpgrade()
{
Create.TableForModel("DownloadClientStatus")
.WithColumn("ProviderId").AsInt32().NotNullable().Unique()
.WithColumn("InitialFailure").AsDateTime().Nullable()
.WithColumn("MostRecentFailure").AsDateTime().Nullable()
.WithColumn("EscalationLevel").AsInt32().NotNullable()
.WithColumn("DisabledTill").AsDateTime().Nullable();
}
}
}

View File

@ -60,7 +60,7 @@ namespace NzbDrone.Core.Datastore
.Ignore(i => i.SupportsOnDownload)
.Ignore(i => i.SupportsOnUpgrade)
.Ignore(i => i.SupportsOnRename);
Mapper.Entity<MetadataDefinition>().RegisterDefinition("Metadata");
Mapper.Entity<DownloadClientDefinition>().RegisterDefinition("DownloadClients")
@ -80,7 +80,7 @@ namespace NzbDrone.Core.Datastore
.Ignore(f => f.Path)
.Relationships.AutoMapICollectionOrComplexProperties()
.For("Episodes")
.LazyLoad(condition: parent => parent.Id > 0,
.LazyLoad(condition: parent => parent.Id > 0,
query: (db, parent) => db.Query<Episode>().Where(c => c.EpisodeFileId == parent.Id).ToList())
.HasOne(file => file.Series, file => file.SeriesId);
@ -116,6 +116,7 @@ namespace NzbDrone.Core.Datastore
.Ignore(c => c.Message);
Mapper.Entity<IndexerStatus>().RegisterModel("IndexerStatus");
Mapper.Entity<DownloadClientStatus>().RegisterModel("DownloadClientStatus");
}
private static void RegisterMappers()
@ -171,4 +172,4 @@ namespace NzbDrone.Core.Datastore
}
}
}
}
}

View File

@ -0,0 +1,9 @@
using NzbDrone.Core.ThingiProvider.Status;
namespace NzbDrone.Core.Download
{
public class DownloadClientStatus : ProviderStatusBase
{
}
}

View File

@ -0,0 +1,19 @@
using NzbDrone.Core.Datastore;
using NzbDrone.Core.Messaging.Events;
using NzbDrone.Core.ThingiProvider.Status;
namespace NzbDrone.Core.Download
{
public interface IDownloadClientStatusRepository : IProviderStatusRepository<DownloadClientStatus>
{
}
public class DownloadClientStatusRepository : ProviderStatusRepository<DownloadClientStatus>, IDownloadClientStatusRepository
{
public DownloadClientStatusRepository(IMainDatabase database, IEventAggregator eventAggregator)
: base(database, eventAggregator)
{
}
}
}

View File

@ -0,0 +1,22 @@
using System;
using NLog;
using NzbDrone.Core.Messaging.Events;
using NzbDrone.Core.ThingiProvider.Status;
namespace NzbDrone.Core.Download
{
public interface IDownloadClientStatusService : IProviderStatusServiceBase<DownloadClientStatus>
{
}
public class DownloadClientStatusService : ProviderStatusServiceBase<IDownloadClient, DownloadClientStatus>, IDownloadClientStatusService
{
public DownloadClientStatusService(IDownloadClientStatusRepository providerStatusRepository, IEventAggregator eventAggregator, Logger logger)
: base(providerStatusRepository, eventAggregator, logger)
{
MinimumTimeSinceInitialFailure = TimeSpan.FromMinutes(5);
MaximumEscalationLevel = 5;
}
}
}

View File

@ -33,8 +33,7 @@ namespace NzbDrone.Core.HealthCheck.Checks
}
catch (Exception ex)
{
_logger.Error(ex, "Unable to communicate with {0}", downloadClient.Definition.Name);
_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}");

View File

@ -0,0 +1,42 @@
using System;
using System.Linq;
using NzbDrone.Common.Extensions;
using NzbDrone.Core.Download;
namespace NzbDrone.Core.HealthCheck.Checks
{
public class DownloadClientStatusCheck : HealthCheckBase
{
private readonly IDownloadClientFactory _providerFactory;
private readonly IDownloadClientStatusService _providerStatusService;
public DownloadClientStatusCheck(IDownloadClientFactory providerFactory, IDownloadClientStatusService providerStatusService)
{
_providerFactory = providerFactory;
_providerStatusService = providerStatusService;
}
public override HealthCheck Check()
{
var enabledProviders = _providerFactory.GetAvailableProviders();
var backOffProviders = enabledProviders.Join(_providerStatusService.GetBlockedProviders(),
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())
{
return new HealthCheck(GetType());
}
if (backOffProviders.Count == enabledProviders.Count)
{
return new HealthCheck(GetType(), HealthCheckResult.Error, "All download clients are unavailable due to failures", "#download-clients-are-unavailable-due-to-failures");
}
return new HealthCheck(GetType(), HealthCheckResult.Warning, string.Format("Download clients unavailable due to failures: {0}", string.Join(", ", backOffProviders.Select(v => v.Provider.Definition.Name))), "#download-clients-are-unavailable-due-to-failures");
}
}
}

View File

@ -0,0 +1,26 @@
using NzbDrone.Core.Datastore;
namespace NzbDrone.Core.Housekeeping.Housekeepers
{
public class CleanupOrphanedDownloadClientStatus : IHousekeepingTask
{
private readonly IMainDatabase _database;
public CleanupOrphanedDownloadClientStatus(IMainDatabase database)
{
_database = database;
}
public void Clean()
{
var mapper = _database.GetDataMapper();
mapper.ExecuteNonQuery(@"DELETE FROM DownloadClientStatus
WHERE Id IN (
SELECT DownloadClientStatus.Id FROM DownloadClientStatus
LEFT OUTER JOIN DownloadClients
ON DownloadClientStatus.ProviderId = DownloadClients.Id
WHERE DownloadClients.Id IS NULL)");
}
}
}

View File

@ -18,8 +18,8 @@ namespace NzbDrone.Core.Indexers
public class IndexerStatusService : ProviderStatusServiceBase<IIndexer, IndexerStatus>, IIndexerStatusService
{
public IndexerStatusService(IIndexerStatusRepository providerStatusRepository, Logger logger)
: base(providerStatusRepository, logger)
public IndexerStatusService(IIndexerStatusRepository providerStatusRepository, IEventAggregator eventAggregator, Logger logger)
: base(providerStatusRepository, eventAggregator, logger)
{
}

View File

@ -293,6 +293,7 @@
</Compile>
<Compile Include="Datastore\Migration\105_rename_torrent_downloadstation.cs" />
<Compile Include="Datastore\Migration\111_create_language_profiles.cs" />
<Compile Include="Datastore\Migration\115_add_downloadclient_status.cs" />
<Compile Include="Datastore\Migration\Framework\MigrationContext.cs" />
<Compile Include="Datastore\Migration\Framework\MigrationController.cs" />
<Compile Include="Datastore\Migration\Framework\MigrationDbFactory.cs" />
@ -494,7 +495,10 @@
<Compile Include="Download\Clients\uTorrent\UTorrentTorrentStatus.cs" />
<Compile Include="Download\Clients\Vuze\Vuze.cs" />
<Compile Include="Download\CompletedDownloadService.cs" />
<Compile Include="Download\DownloadClientStatus.cs" />
<Compile Include="Download\DownloadEventHub.cs" />
<Compile Include="Download\DownloadClientStatusRepository.cs" />
<Compile Include="Download\DownloadClientStatusService.cs" />
<Compile Include="Download\TrackedDownloads\DownloadMonitoringService.cs" />
<Compile Include="Download\TrackedDownloads\TrackedDownload.cs" />
<Compile Include="Download\TrackedDownloads\TrackedDownloadService.cs" />
@ -557,6 +561,7 @@
<Compile Include="HealthCheck\CheckHealthCommand.cs" />
<Compile Include="HealthCheck\Checks\AppDataLocationCheck.cs" />
<Compile Include="HealthCheck\Checks\DownloadClientCheck.cs" />
<Compile Include="HealthCheck\Checks\DownloadClientStatusCheck.cs" />
<Compile Include="HealthCheck\Checks\MountCheck.cs" />
<Compile Include="HealthCheck\Checks\DroneFactoryCheck.cs" />
<Compile Include="HealthCheck\Checks\ImportMechanismCheck.cs" />
@ -582,6 +587,7 @@
<Compile Include="Housekeeping\Housekeepers\CleanupAbsolutePathMetadataFiles.cs" />
<Compile Include="Housekeeping\Housekeepers\CleanupDuplicateMetadataFiles.cs" />
<Compile Include="Housekeeping\Housekeepers\CleanupOrphanedBlacklist.cs" />
<Compile Include="Housekeeping\Housekeepers\CleanupOrphanedDownloadClientStatus.cs" />
<Compile Include="Housekeeping\Housekeepers\CleanupOrphanedEpisodeFiles.cs" />
<Compile Include="Housekeeping\Housekeepers\CleanupOrphanedEpisodes.cs" />
<Compile Include="Housekeeping\Housekeepers\CleanupOrphanedIndexerStatus.cs" />
@ -1073,6 +1079,7 @@
<Compile Include="Tags\TagService.cs" />
<Compile Include="Tags\TagsUpdatedEvent.cs" />
<Compile Include="ThingiProvider\ConfigContractNotFoundException.cs" />
<Compile Include="ThingiProvider\Events\ProviderStatusChangedEvent.cs" />
<Compile Include="ThingiProvider\Events\ProviderDeletedEvent.cs" />
<Compile Include="ThingiProvider\Events\ProviderUpdatedEvent.cs" />
<Compile Include="ThingiProvider\IProvider.cs" />

View File

@ -0,0 +1,18 @@
using NzbDrone.Common.Messaging;
using NzbDrone.Core.ThingiProvider.Status;
namespace NzbDrone.Core.ThingiProvider.Events
{
public class ProviderStatusChangedEvent<TProvider> : IEvent
{
public int ProviderId { get; private set; }
public ProviderStatusBase Status { get; private set; }
public ProviderStatusChangedEvent(int id, ProviderStatusBase status)
{
ProviderId = id;
Status = status;
}
}
}

View File

@ -33,16 +33,19 @@ namespace NzbDrone.Core.ThingiProvider.Status
24 * 60 * 60
};
private static readonly int MaximumEscalationLevel = EscalationBackOffPeriods.Length - 1;
protected readonly object _syncRoot = new object();
protected readonly IProviderStatusRepository<TModel> _providerStatusRepository;
protected readonly IEventAggregator _eventAggregator;
protected readonly Logger _logger;
public ProviderStatusServiceBase(IProviderStatusRepository<TModel> providerStatusRepository, Logger logger)
protected int MaximumEscalationLevel { get; set; } = EscalationBackOffPeriods.Length - 1;
protected TimeSpan MinimumTimeSinceInitialFailure { get; set; } = TimeSpan.Zero;
public ProviderStatusServiceBase(IProviderStatusRepository<TModel> providerStatusRepository, IEventAggregator eventAggregator, Logger logger)
{
_providerStatusRepository = providerStatusRepository;
_eventAggregator = eventAggregator;
_logger = logger;
}
@ -78,6 +81,8 @@ namespace NzbDrone.Core.ThingiProvider.Status
status.DisabledTill = null;
_providerStatusRepository.Upsert(status);
_eventAggregator.PublishEvent(new ProviderStatusChangedEvent<TProvider>(providerId, status));
}
}
@ -108,9 +113,14 @@ namespace NzbDrone.Core.ThingiProvider.Status
}
}
status.DisabledTill = now + CalculateBackOffPeriod(status);
if (status.InitialFailure.Value + MinimumTimeSinceInitialFailure <= now || minimumBackOff != TimeSpan.Zero)
{
status.DisabledTill = now + CalculateBackOffPeriod(status);
}
_providerStatusRepository.Upsert(status);
_eventAggregator.PublishEvent(new ProviderStatusChangedEvent<TProvider>(providerId, status));
}
}