From 53e723a3010462dc803755a84f1d224406eec9df Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Sat, 19 Jul 2014 19:37:06 +0200 Subject: [PATCH] Fixed: Blacklist Retry logic will now properly handle Sabnzbd changing the unique id. --- src/NzbDrone.Api/History/HistoryModule.cs | 8 +- .../Specifications/NotInQueueSpecification.cs | 10 ++- .../Download/Clients/Nzbget/Nzbget.cs | 6 +- .../Download/Clients/Pneumatic/Pneumatic.cs | 6 +- .../Download/Clients/Sabnzbd/Sabnzbd.cs | 22 +++++- .../UsenetBlackhole/UsenetBlackhole.cs | 4 +- .../Download/CompletedDownloadService.cs | 2 +- .../Download/DownloadClientBase.cs | 4 +- .../Download/DownloadTrackingService.cs | 18 +++++ .../Download/FailedDownloadService.cs | 74 +++++++++++++------ src/NzbDrone.Core/Download/IDownloadClient.cs | 9 ++- 11 files changed, 116 insertions(+), 47 deletions(-) diff --git a/src/NzbDrone.Api/History/HistoryModule.cs b/src/NzbDrone.Api/History/HistoryModule.cs index 119ea1e68..232edfd79 100644 --- a/src/NzbDrone.Api/History/HistoryModule.cs +++ b/src/NzbDrone.Api/History/HistoryModule.cs @@ -10,12 +10,12 @@ namespace NzbDrone.Api.History public class HistoryModule : NzbDroneRestModule { private readonly IHistoryService _historyService; - private readonly IFailedDownloadService _failedDownloadService; + private readonly IDownloadTrackingService _downloadTrackingService; - public HistoryModule(IHistoryService historyService, IFailedDownloadService failedDownloadService) + public HistoryModule(IHistoryService historyService, IDownloadTrackingService downloadTrackingService) { _historyService = historyService; - _failedDownloadService = failedDownloadService; + _downloadTrackingService = downloadTrackingService; GetResourcePaged = GetHistory; Post["/failed"] = x => MarkAsFailed(); @@ -51,7 +51,7 @@ namespace NzbDrone.Api.History private Response MarkAsFailed() { var id = (int)Request.Form.Id; - _failedDownloadService.MarkAsFailed(id); + _downloadTrackingService.MarkAsFailed(id); return new Object().AsResponse(); } } diff --git a/src/NzbDrone.Core/DecisionEngine/Specifications/NotInQueueSpecification.cs b/src/NzbDrone.Core/DecisionEngine/Specifications/NotInQueueSpecification.cs index 92f73d562..64f19ad72 100644 --- a/src/NzbDrone.Core/DecisionEngine/Specifications/NotInQueueSpecification.cs +++ b/src/NzbDrone.Core/DecisionEngine/Specifications/NotInQueueSpecification.cs @@ -11,12 +11,12 @@ namespace NzbDrone.Core.DecisionEngine.Specifications { public class NotInQueueSpecification : IDecisionEngineSpecification { - private readonly IQueueService _queueService; + private readonly IDownloadTrackingService _downloadTrackingService; private readonly Logger _logger; - public NotInQueueSpecification(IQueueService queueService, Logger logger) + public NotInQueueSpecification(IDownloadTrackingService downloadTrackingService, Logger logger) { - _queueService = queueService; + _downloadTrackingService = downloadTrackingService; _logger = logger; } @@ -30,7 +30,9 @@ namespace NzbDrone.Core.DecisionEngine.Specifications public bool IsSatisfiedBy(RemoteEpisode subject, SearchCriteriaBase searchCriteria) { - var queue = _queueService.GetQueue().Select(q => q.RemoteEpisode); + var queue = _downloadTrackingService.GetQueuedDownloads() + .Where(v => v.State == TrackedDownloadState.Downloading) + .Select(q => q.DownloadItem.RemoteEpisode).ToList(); if (IsInQueue(subject, queue)) { diff --git a/src/NzbDrone.Core/Download/Clients/Nzbget/Nzbget.cs b/src/NzbDrone.Core/Download/Clients/Nzbget/Nzbget.cs index c670fffb1..76a87c911 100644 --- a/src/NzbDrone.Core/Download/Clients/Nzbget/Nzbget.cs +++ b/src/NzbDrone.Core/Download/Clients/Nzbget/Nzbget.cs @@ -223,14 +223,16 @@ namespace NzbDrone.Core.Download.Clients.Nzbget } } - public override void RemoveItem(string id) + public override void RemoveItem(String id) { _proxy.RemoveFromHistory(id, Settings); } - public override void RetryDownload(string id) + public override String RetryDownload(String id) { _proxy.RetryDownload(id, Settings); + + return id; } public override DownloadClientStatus GetStatus() diff --git a/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs b/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs index 8bf52fe95..c0f202af3 100644 --- a/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs +++ b/src/NzbDrone.Core/Download/Clients/Pneumatic/Pneumatic.cs @@ -74,13 +74,13 @@ namespace NzbDrone.Core.Download.Clients.Pneumatic { return new DownloadClientItem[0]; } - - public override void RemoveItem(string id) + + public override void RemoveItem(String id) { throw new NotSupportedException(); } - public override void RetryDownload(string id) + public override String RetryDownload(String id) { throw new NotSupportedException(); } diff --git a/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs b/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs index 5009bb828..a11e13c74 100644 --- a/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs +++ b/src/NzbDrone.Core/Download/Clients/Sabnzbd/Sabnzbd.cs @@ -215,7 +215,7 @@ namespace NzbDrone.Core.Download.Clients.Sabnzbd } } - public override void RemoveItem(string id) + public override void RemoveItem(String id) { if (GetQueue().Any(v => v.DownloadClientId == id)) { @@ -227,9 +227,27 @@ namespace NzbDrone.Core.Download.Clients.Sabnzbd } } - public override void RetryDownload(string id) + public override String RetryDownload(String id) { + // Sabnzbd changed the nzo_id for retried downloads without reporting it back to us. We need to try to determine the new ID. + + var history = GetHistory().Where(v => v.DownloadClientId == id).ToList(); + _proxy.RetryDownload(id, Settings); + + if (history.Count() != 1) + { + return id; + } + + var queue = GetQueue().Where(v => v.Category == history.First().Category && v.Title == history.First().Title).ToList(); + + if (queue.Count() != 1) + { + return id; + } + + return queue.First().DownloadClientId; } protected IEnumerable GetCategories(SabnzbdConfig config) diff --git a/src/NzbDrone.Core/Download/Clients/UsenetBlackhole/UsenetBlackhole.cs b/src/NzbDrone.Core/Download/Clients/UsenetBlackhole/UsenetBlackhole.cs index 421991974..bbc077dcd 100644 --- a/src/NzbDrone.Core/Download/Clients/UsenetBlackhole/UsenetBlackhole.cs +++ b/src/NzbDrone.Core/Download/Clients/UsenetBlackhole/UsenetBlackhole.cs @@ -124,12 +124,12 @@ namespace NzbDrone.Core.Download.Clients.UsenetBlackhole } } - public override void RemoveItem(string id) + public override void RemoveItem(String id) { throw new NotSupportedException(); } - public override void RetryDownload(string id) + public override String RetryDownload(String id) { throw new NotSupportedException(); } diff --git a/src/NzbDrone.Core/Download/CompletedDownloadService.cs b/src/NzbDrone.Core/Download/CompletedDownloadService.cs index 81e3a2b10..21c8c8835 100644 --- a/src/NzbDrone.Core/Download/CompletedDownloadService.cs +++ b/src/NzbDrone.Core/Download/CompletedDownloadService.cs @@ -205,7 +205,7 @@ namespace NzbDrone.Core.Download } } } - + private void UpdateStatusMessage(TrackedDownload trackedDownload, LogLevel logLevel, String message, params object[] args) { var statusMessage = String.Format(message, args); diff --git a/src/NzbDrone.Core/Download/DownloadClientBase.cs b/src/NzbDrone.Core/Download/DownloadClientBase.cs index 1f84b1ff0..e3b07e4dc 100644 --- a/src/NzbDrone.Core/Download/DownloadClientBase.cs +++ b/src/NzbDrone.Core/Download/DownloadClientBase.cs @@ -67,10 +67,10 @@ namespace NzbDrone.Core.Download get; } - public abstract string Download(RemoteEpisode remoteEpisode); + public abstract String Download(RemoteEpisode remoteEpisode); public abstract IEnumerable GetItems(); public abstract void RemoveItem(string id); - public abstract void RetryDownload(string id); + public abstract String RetryDownload(string id); public abstract DownloadClientStatus GetStatus(); protected RemoteEpisode GetRemoteEpisode(String title) diff --git a/src/NzbDrone.Core/Download/DownloadTrackingService.cs b/src/NzbDrone.Core/Download/DownloadTrackingService.cs index eef90e755..557c61cb5 100644 --- a/src/NzbDrone.Core/Download/DownloadTrackingService.cs +++ b/src/NzbDrone.Core/Download/DownloadTrackingService.cs @@ -18,6 +18,8 @@ namespace NzbDrone.Core.Download TrackedDownload[] GetTrackedDownloads(); TrackedDownload[] GetCompletedDownloads(); TrackedDownload[] GetQueuedDownloads(); + + void MarkAsFailed(Int32 historyId); } public class DownloadTrackingService : IDownloadTrackingService, IExecute, IHandleAsync, IHandle @@ -78,6 +80,22 @@ namespace NzbDrone.Core.Download }, TimeSpan.FromSeconds(5.0)); } + public void MarkAsFailed(Int32 historyId) + { + var item = _historyService.Get(historyId); + + var trackedDownload = GetTrackedDownloads() + .Where(h => h.DownloadItem.DownloadClientId.Equals(item.Data.GetValueOrDefault(DOWNLOAD_CLIENT_ID))) + .FirstOrDefault(); + + if (trackedDownload != null && trackedDownload.State == TrackedDownloadState.Unknown) + { + ProcessTrackedDownloads(); + } + + _failedDownloadService.MarkAsFailed(trackedDownload, item); + } + private TrackedDownload[] FilterQueuedDownloads(IEnumerable trackedDownloads) { var enabledFailedDownloadHandling = _configService.EnableFailedDownloadHandling; diff --git a/src/NzbDrone.Core/Download/FailedDownloadService.cs b/src/NzbDrone.Core/Download/FailedDownloadService.cs index fc56fbe06..782b4b417 100644 --- a/src/NzbDrone.Core/Download/FailedDownloadService.cs +++ b/src/NzbDrone.Core/Download/FailedDownloadService.cs @@ -13,7 +13,7 @@ namespace NzbDrone.Core.Download { public interface IFailedDownloadService { - void MarkAsFailed(int historyId); + void MarkAsFailed(TrackedDownload trackedDownload, History.History grabbedHistory); void CheckForFailedItem(IDownloadClient downloadClient, TrackedDownload trackedDownload, List grabbedHistory, List failedHistory); } @@ -35,10 +35,14 @@ namespace NzbDrone.Core.Download _logger = logger; } - public void MarkAsFailed(int historyId) + public void MarkAsFailed(TrackedDownload trackedDownload, History.History grabbedHistory) { - var item = _historyService.Get(historyId); - PublishDownloadFailedEvent(new List { item }, "Manually marked as failed"); + if (trackedDownload != null && trackedDownload.State == TrackedDownloadState.Downloading) + { + trackedDownload.State = TrackedDownloadState.DownloadFailed; + } + + PublishDownloadFailedEvent(new List { grabbedHistory }, "Manually marked as failed"); } public void CheckForFailedItem(IDownloadClient downloadClient, TrackedDownload trackedDownload, List grabbedHistory, List failedHistory) @@ -54,7 +58,7 @@ namespace NzbDrone.Core.Download if (!grabbedItems.Any()) { - UpdateStatusMessage(LogLevel.Debug, trackedDownload, "Download was not grabbed by drone, ignoring download"); + UpdateStatusMessage(trackedDownload, LogLevel.Debug, "Download was not grabbed by drone, ignoring download"); return; } @@ -64,7 +68,7 @@ namespace NzbDrone.Core.Download if (failedItems.Any()) { - UpdateStatusMessage(LogLevel.Debug, trackedDownload, "Already added to history as failed"); + UpdateStatusMessage(trackedDownload, LogLevel.Debug, "Already added to history as failed."); } else { @@ -78,7 +82,7 @@ namespace NzbDrone.Core.Download if (!grabbedItems.Any()) { - UpdateStatusMessage(LogLevel.Debug, trackedDownload, "Download wasn't grabbed by drone or not in a category, ignoring download."); + UpdateStatusMessage(trackedDownload, LogLevel.Debug, "Download wasn't grabbed by drone or not in a category, ignoring download."); return; } @@ -86,13 +90,13 @@ namespace NzbDrone.Core.Download if (trackedDownload.DownloadItem.Message.Equals("Unpacking failed, write error or disk is full?", StringComparison.InvariantCultureIgnoreCase)) { - UpdateStatusMessage(LogLevel.Error, trackedDownload, "Download failed due to lack of disk space, not blacklisting."); + UpdateStatusMessage(trackedDownload, LogLevel.Error, "Download failed due to lack of disk space, not blacklisting."); return; } if (FailedDownloadForRecentRelease(downloadClient, trackedDownload, grabbedItems)) { - UpdateStatusMessage(LogLevel.Debug, trackedDownload, "Recent release Failed, do not blacklist."); + _logger.Debug("[{0}] Recent release Failed, do not blacklist.", trackedDownload.DownloadItem.Title); return; } @@ -102,7 +106,7 @@ namespace NzbDrone.Core.Download if (failedItems.Any()) { - UpdateStatusMessage(LogLevel.Debug, trackedDownload, "Already added to history as failed."); + UpdateStatusMessage(trackedDownload, LogLevel.Debug, "Already added to history as failed."); } else { @@ -117,7 +121,7 @@ namespace NzbDrone.Core.Download if (grabbedItems.Any() && failedItems.Any()) { - UpdateStatusMessage(LogLevel.Debug, trackedDownload, "Already added to history as failed, updating tracked state."); + UpdateStatusMessage(trackedDownload, LogLevel.Debug, "Already added to history as failed, updating tracked state."); trackedDownload.State = TrackedDownloadState.DownloadFailed; } } @@ -126,14 +130,14 @@ namespace NzbDrone.Core.Download { try { - _logger.Debug("[{0}] Removing failed download from client", trackedDownload.DownloadItem.Title); + _logger.Debug("[{0}] Removing failed download from client.", trackedDownload.DownloadItem.Title); downloadClient.RemoveItem(trackedDownload.DownloadItem.DownloadClientId); trackedDownload.State = TrackedDownloadState.Removed; } catch (NotSupportedException) { - UpdateStatusMessage(LogLevel.Debug, trackedDownload, "Removing item not supported by your download client."); + UpdateStatusMessage(trackedDownload, LogLevel.Debug, "Removing item not supported by your download client."); } } } @@ -144,38 +148,62 @@ namespace NzbDrone.Core.Download if (!Double.TryParse(matchingHistoryItems.First().Data.GetValueOrDefault("ageHours"), out ageHours)) { - _logger.Info("[{0}] Unable to determine age of failed download.", trackedDownload.DownloadItem.Title); + UpdateStatusMessage(trackedDownload, LogLevel.Info, "Unable to determine age of failed download."); return false; } if (ageHours > _configService.BlacklistGracePeriod) { - _logger.Info("[{0}] Failed download is older than the grace period.", trackedDownload.DownloadItem.Title); + UpdateStatusMessage(trackedDownload, LogLevel.Info, "Download Failed, Failed download is older than the grace period."); return false; } if (trackedDownload.RetryCount >= _configService.BlacklistRetryLimit) { - _logger.Info("[{0}] Retry limit reached.", trackedDownload.DownloadItem.Title); + UpdateStatusMessage(trackedDownload, LogLevel.Info, "Download Failed, Retry limit reached."); return false; } - if (trackedDownload.RetryCount == 0 || trackedDownload.LastRetry.AddMinutes(_configService.BlacklistRetryInterval) < DateTime.UtcNow) + if (trackedDownload.LastRetry == new DateTime()) + { + trackedDownload.LastRetry = DateTime.UtcNow; + } + + if (trackedDownload.LastRetry.AddMinutes(_configService.BlacklistRetryInterval) < DateTime.UtcNow) { - _logger.Info("[{0}] Retrying failed release.", trackedDownload.DownloadItem.Title); trackedDownload.LastRetry = DateTime.UtcNow; trackedDownload.RetryCount++; + UpdateStatusMessage(trackedDownload, LogLevel.Info, "Download Failed, initiating retry attempt {0}/{1}.", trackedDownload.RetryCount, _configService.BlacklistRetryLimit); + try { - downloadClient.RetryDownload(trackedDownload.DownloadItem.DownloadClientId); + var newDownloadClientId = downloadClient.RetryDownload(trackedDownload.DownloadItem.DownloadClientId); + + if (newDownloadClientId != trackedDownload.DownloadItem.DownloadClientId) + { + var oldTrackingId = trackedDownload.TrackingId; + var newTrackingId = String.Format("{0}-{1}", downloadClient.Definition.Id, newDownloadClientId); + + trackedDownload.TrackingId = newTrackingId; + trackedDownload.DownloadItem.DownloadClientId = newDownloadClientId; + + _logger.Debug("[{0}] Changed id from {1} to {2}.", trackedDownload.DownloadItem.Title, oldTrackingId, newTrackingId); + var newHistoryData = new Dictionary(matchingHistoryItems.First().Data); + newHistoryData[DownloadTrackingService.DOWNLOAD_CLIENT_ID] = newDownloadClientId; + _historyService.UpdateHistoryData(matchingHistoryItems.First().Id, newHistoryData); + } } catch (NotSupportedException ex) { - _logger.Debug("Retrying failed downloads is not supported by your download client"); + UpdateStatusMessage(trackedDownload, LogLevel.Debug, "Retrying failed downloads is not supported by your download client."); return false; } } + else + { + UpdateStatusMessage(trackedDownload, LogLevel.Warn, "Download Failed, waiting for retry interval to expire."); + } return true; } @@ -206,16 +234,16 @@ namespace NzbDrone.Core.Download _eventAggregator.PublishEvent(downloadFailedEvent); } - private void UpdateStatusMessage(LogLevel logLevel, TrackedDownload trackedDownload, String message, params object[] args) + private void UpdateStatusMessage(TrackedDownload trackedDownload, LogLevel logLevel, String message, params object[] args) { var statusMessage = String.Format(message, args); - var logMessage = String.Format("[{0}] {1}", trackedDownload.DownloadItem.Title, message); + var logMessage = String.Format("[{0}] {1}", trackedDownload.DownloadItem.Title, statusMessage); if (trackedDownload.StatusMessage != statusMessage) { trackedDownload.HasError = logLevel >= LogLevel.Warn; trackedDownload.StatusMessage = statusMessage; - _logger.Log(logLevel, statusMessage); + _logger.Log(logLevel, logMessage); } else { diff --git a/src/NzbDrone.Core/Download/IDownloadClient.cs b/src/NzbDrone.Core/Download/IDownloadClient.cs index b0b3d5fb9..193c59254 100644 --- a/src/NzbDrone.Core/Download/IDownloadClient.cs +++ b/src/NzbDrone.Core/Download/IDownloadClient.cs @@ -1,4 +1,5 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using NzbDrone.Core.Indexers; using NzbDrone.Core.Parser.Model; using NzbDrone.Core.ThingiProvider; @@ -9,10 +10,10 @@ namespace NzbDrone.Core.Download { DownloadProtocol Protocol { get; } - string Download(RemoteEpisode remoteEpisode); + String Download(RemoteEpisode remoteEpisode); IEnumerable GetItems(); - void RemoveItem(string id); - void RetryDownload(string id); + void RemoveItem(String id); + String RetryDownload(String id); DownloadClientStatus GetStatus(); }