From 393996fe88400ddf15344caa35e0957a18760e9b Mon Sep 17 00:00:00 2001 From: Taloth Saldono Date: Wed, 25 Oct 2017 22:58:52 +0200 Subject: [PATCH] Fixed: Progressively degrading performance issue in Pending/Delayed releases system. fixes #2205 --- .../PendingReleaseServiceTests/AddFixture.cs | 46 ++++++- .../RemoveGrabbedFixture.cs | 18 ++- .../RemovePendingFixture.cs | 8 +- .../RemoveRejectedFixture.cs | 6 +- .../RssSync/DelaySpecification.cs | 2 +- .../Pending/PendingReleaseRepository.cs | 8 +- .../Download/Pending/PendingReleaseService.cs | 120 +++++++++++------- .../Download/ProcessDownloadDecisions.cs | 13 +- 8 files changed, 160 insertions(+), 61 deletions(-) diff --git a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/AddFixture.cs b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/AddFixture.cs index 717ed3b57..5820b1f0b 100644 --- a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/AddFixture.cs +++ b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/AddFixture.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using FizzWare.NBuilder; using Marr.Data; using Moq; @@ -26,6 +27,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests private ReleaseInfo _release; private ParsedEpisodeInfo _parsedEpisodeInfo; private RemoteEpisode _remoteEpisode; + private List _heldReleases; [SetUp] public void Setup() @@ -60,17 +62,27 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests _remoteEpisode.Series = _series; _remoteEpisode.ParsedEpisodeInfo = _parsedEpisodeInfo; _remoteEpisode.Release = _release; - + _temporarilyRejected = new DownloadDecision(_remoteEpisode, new Rejection("Temp Rejected", RejectionType.Temporary)); + _heldReleases = new List(); + Mocker.GetMock() .Setup(s => s.All()) - .Returns(new List()); + .Returns(_heldReleases); + + Mocker.GetMock() + .Setup(s => s.AllBySeriesId(It.IsAny())) + .Returns(i => _heldReleases.Where(v => v.SeriesId == i).ToList()); Mocker.GetMock() .Setup(s => s.GetSeries(It.IsAny())) .Returns(_series); + Mocker.GetMock() + .Setup(s => s.GetSeries(It.IsAny>())) + .Returns(new List { _series }); + Mocker.GetMock() .Setup(s => s.GetEpisodes(It.IsAny(), _series, true, null)) .Returns(new List {_episode}); @@ -80,7 +92,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests .Returns((List d) => d); } - private void GivenHeldRelease(string title, string indexer, DateTime publishDate) + private void GivenHeldRelease(string title, string indexer, DateTime publishDate, PendingReleaseReason reason = PendingReleaseReason.Delay) { var release = _release.JsonClone(); release.Indexer = indexer; @@ -92,11 +104,10 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests .With(h => h.SeriesId = _series.Id) .With(h => h.Title = title) .With(h => h.Release = release) + .With(h => h.Reason = reason) .Build(); - Mocker.GetMock() - .Setup(s => s.All()) - .Returns(heldReleases); + _heldReleases.AddRange(heldReleases); } [Test] @@ -117,6 +128,29 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests VerifyNoInsert(); } + [Test] + public void should_not_add_if_it_is_the_same_release_from_the_same_indexer_twice() + { + GivenHeldRelease(_release.Title, _release.Indexer, _release.PublishDate, PendingReleaseReason.DownloadClientUnavailable); + GivenHeldRelease(_release.Title, _release.Indexer, _release.PublishDate, PendingReleaseReason.Fallback); + + Subject.Add(_temporarilyRejected, PendingReleaseReason.Delay); + + VerifyNoInsert(); + } + + [Test] + public void should_remove_duplicate_if_it_is_the_same_release_from_the_same_indexer_twice() + { + GivenHeldRelease(_release.Title, _release.Indexer, _release.PublishDate, PendingReleaseReason.DownloadClientUnavailable); + GivenHeldRelease(_release.Title, _release.Indexer, _release.PublishDate, PendingReleaseReason.Fallback); + + Subject.Add(_temporarilyRejected, PendingReleaseReason.Fallback); + + Mocker.GetMock() + .Verify(v => v.Delete(It.IsAny()), Times.Once()); + } + [Test] public void should_add_if_title_is_different() { diff --git a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveGrabbedFixture.cs b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveGrabbedFixture.cs index b988a30c9..84ef17ae7 100644 --- a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveGrabbedFixture.cs +++ b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveGrabbedFixture.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; using FizzWare.NBuilder; using Marr.Data; using Moq; @@ -26,6 +27,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests private ReleaseInfo _release; private ParsedEpisodeInfo _parsedEpisodeInfo; private RemoteEpisode _remoteEpisode; + private List _heldReleases; [SetUp] public void Setup() @@ -63,14 +65,24 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests _temporarilyRejected = new DownloadDecision(_remoteEpisode, new Rejection("Temp Rejected", RejectionType.Temporary)); + _heldReleases = new List(); + Mocker.GetMock() .Setup(s => s.All()) - .Returns(new List()); + .Returns(_heldReleases); + + Mocker.GetMock() + .Setup(s => s.AllBySeriesId(It.IsAny())) + .Returns(i => _heldReleases.Where(v => v.SeriesId == i).ToList()); Mocker.GetMock() .Setup(s => s.GetSeries(It.IsAny())) .Returns(_series); + Mocker.GetMock() + .Setup(s => s.GetSeries(It.IsAny>())) + .Returns(new List { _series }); + Mocker.GetMock() .Setup(s => s.GetEpisodes(It.IsAny(), _series, true, null)) .Returns(new List {_episode}); @@ -92,9 +104,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests .With(h => h.ParsedEpisodeInfo = parsedEpisodeInfo) .Build(); - Mocker.GetMock() - .Setup(s => s.All()) - .Returns(heldReleases); + _heldReleases.AddRange(heldReleases); } [Test] diff --git a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemovePendingFixture.cs b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemovePendingFixture.cs index 44c2a1029..d5c5058b1 100644 --- a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemovePendingFixture.cs +++ b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemovePendingFixture.cs @@ -38,6 +38,10 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests .Setup(s => s.GetSeries(It.IsAny())) .Returns(new Series()); + Mocker.GetMock() + .Setup(s => s.GetSeries(It.IsAny>())) + .Returns(new List { new Series() }); + Mocker.GetMock() .Setup(s => s.GetEpisodes(It.IsAny(), It.IsAny(), It.IsAny(), null)) .Returns(new List{ _episode }); @@ -63,7 +67,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests AssertRemoved(1); } - + [Test] public void should_remove_multiple_releases_release() { @@ -134,7 +138,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests AssertRemoved(2); } - + private void AssertRemoved(params int[] ids) { Mocker.GetMock().Verify(c => c.DeleteMany(It.Is>(s => s.SequenceEqual(ids)))); diff --git a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveRejectedFixture.cs b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveRejectedFixture.cs index d62fb0d2b..78e9c59cd 100644 --- a/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveRejectedFixture.cs +++ b/src/NzbDrone.Core.Test/Download/Pending/PendingReleaseServiceTests/RemoveRejectedFixture.cs @@ -62,7 +62,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests _remoteEpisode.Series = _series; _remoteEpisode.ParsedEpisodeInfo = _parsedEpisodeInfo; _remoteEpisode.Release = _release; - + _temporarilyRejected = new DownloadDecision(_remoteEpisode, new Rejection("Temp Rejected", RejectionType.Temporary)); Mocker.GetMock() @@ -73,6 +73,10 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests .Setup(s => s.GetSeries(It.IsAny())) .Returns(_series); + Mocker.GetMock() + .Setup(s => s.GetSeries(It.IsAny>())) + .Returns(new List { _series }); + Mocker.GetMock() .Setup(s => s.GetEpisodes(It.IsAny(), _series, true, null)) .Returns(new List {_episode}); diff --git a/src/NzbDrone.Core/DecisionEngine/Specifications/RssSync/DelaySpecification.cs b/src/NzbDrone.Core/DecisionEngine/Specifications/RssSync/DelaySpecification.cs index da6285942..254eceb91 100644 --- a/src/NzbDrone.Core/DecisionEngine/Specifications/RssSync/DelaySpecification.cs +++ b/src/NzbDrone.Core/DecisionEngine/Specifications/RssSync/DelaySpecification.cs @@ -81,7 +81,7 @@ namespace NzbDrone.Core.DecisionEngine.Specifications.RssSync var episodeIds = subject.Episodes.Select(e => e.Id); - var oldest = _pendingReleaseService.OldestPendingRelease(subject.Series.Id, episodeIds); + var oldest = _pendingReleaseService.OldestPendingRelease(subject.Series.Id, episodeIds.ToArray()); if (oldest != null && oldest.Release.AgeMinutes > delay) { diff --git a/src/NzbDrone.Core/Download/Pending/PendingReleaseRepository.cs b/src/NzbDrone.Core/Download/Pending/PendingReleaseRepository.cs index b98334978..8078bd508 100644 --- a/src/NzbDrone.Core/Download/Pending/PendingReleaseRepository.cs +++ b/src/NzbDrone.Core/Download/Pending/PendingReleaseRepository.cs @@ -8,6 +8,7 @@ namespace NzbDrone.Core.Download.Pending { void DeleteBySeriesId(int seriesId); List AllBySeriesId(int seriesId); + List WithoutFallback(); } public class PendingReleaseRepository : BasicRepository, IPendingReleaseRepository @@ -26,5 +27,10 @@ namespace NzbDrone.Core.Download.Pending { return Query.Where(p => p.SeriesId == seriesId); } + + public List WithoutFallback() + { + return Query.Where(p => p.Reason != PendingReleaseReason.Fallback); + } } -} \ No newline at end of file +} diff --git a/src/NzbDrone.Core/Download/Pending/PendingReleaseService.cs b/src/NzbDrone.Core/Download/Pending/PendingReleaseService.cs index c23890b01..3866818b0 100644 --- a/src/NzbDrone.Core/Download/Pending/PendingReleaseService.cs +++ b/src/NzbDrone.Core/Download/Pending/PendingReleaseService.cs @@ -21,12 +21,13 @@ namespace NzbDrone.Core.Download.Pending public interface IPendingReleaseService { void Add(DownloadDecision decision, PendingReleaseReason reason); + void AddMany(List> decisions); List GetPending(); List GetPendingRemoteEpisodes(int seriesId); List GetPendingQueue(); Queue.Queue FindPendingQueueItem(int queueId); void RemovePendingQueueItems(int queueId); - RemoteEpisode OldestPendingRelease(int seriesId, IEnumerable episodeIds); + RemoteEpisode OldestPendingRelease(int seriesId, int[] episodeIds); } public class PendingReleaseService : IPendingReleaseService, @@ -68,8 +69,27 @@ namespace NzbDrone.Core.Download.Pending public void Add(DownloadDecision decision, PendingReleaseReason reason) { - var alreadyPending = GetPendingReleases(); + var alreadyPending = _repository.AllBySeriesId(decision.RemoteEpisode.Series.Id); + alreadyPending = IncludeRemoteEpisodes(alreadyPending); + + Add(alreadyPending, decision, reason); + } + + public void AddMany(List> decisions) + { + var alreadyPending = decisions.Select(v => v.Item1.RemoteEpisode.Series.Id).Distinct().SelectMany(_repository.AllBySeriesId).ToList(); + + alreadyPending = IncludeRemoteEpisodes(alreadyPending); + + foreach (var pair in decisions) + { + Add(alreadyPending, pair.Item1, pair.Item2); + } + } + + private void Add(List alreadyPending, DownloadDecision decision, PendingReleaseReason reason) + { var episodeIds = decision.RemoteEpisode.Episodes.Select(e => e.Id); var existingReports = alreadyPending.Where(r => r.RemoteEpisode.Episodes.Select(e => e.Id) @@ -80,24 +100,31 @@ namespace NzbDrone.Core.Download.Pending if (matchingReports.Any()) { - var sameReason = true; + var matchingReport = matchingReports.First(); - foreach (var matchingReport in matchingReports) + if (matchingReport.Reason != reason) { - if (matchingReport.Reason != reason) + _logger.Debug("The release {0} is already pending with reason {1}, changing to {2}", decision.RemoteEpisode, matchingReport.Reason, reason); + matchingReport.Reason = reason; + _repository.Update(matchingReport); + } + else + { + _logger.Debug("The release {0} is already pending with reason {1}, not adding again", decision.RemoteEpisode, reason); + } + + if (matchingReports.Count() > 1) + { + _logger.Debug("The release {0} had {1} duplicate pending, removing duplicates.", decision.RemoteEpisode, matchingReports.Count() - 1); + + foreach (var duplicate in matchingReports.Skip(1)) { - _logger.Debug("The release {0} is already pending with reason {1}, changing to {2}", decision.RemoteEpisode, matchingReport.Reason, reason); - matchingReport.Reason = reason; - _repository.Update(matchingReport); - sameReason = false; + _repository.Delete(duplicate.Id); + alreadyPending.Remove(duplicate); } } - if (sameReason) - { - _logger.Debug("The release {0} is already pending with reason {1}, not adding again", decision.RemoteEpisode, reason); - return; - } + return; } _logger.Debug("Adding release {0} to pending releases with reason {1}", decision.RemoteEpisode, reason); @@ -125,7 +152,7 @@ namespace NzbDrone.Core.Download.Pending public List GetPendingRemoteEpisodes(int seriesId) { - return _repository.AllBySeriesId(seriesId).Select(GetRemoteEpisode).ToList(); + return IncludeRemoteEpisodes(_repository.AllBySeriesId(seriesId)).Select(v => v.RemoteEpisode).ToList(); } public List GetPendingQueue() @@ -134,7 +161,8 @@ namespace NzbDrone.Core.Download.Pending var nextRssSync = new Lazy(() => _taskManager.GetNextExecution(typeof(RssSyncCommand))); - foreach (var pendingRelease in GetPendingReleases().Where(p => p.Reason != PendingReleaseReason.Fallback)) + var pendingReleases = IncludeRemoteEpisodes(_repository.WithoutFallback()); + foreach (var pendingRelease in pendingReleases) { foreach (var episode in pendingRelease.RemoteEpisode.Episodes) { @@ -206,24 +234,48 @@ namespace NzbDrone.Core.Download.Pending _repository.DeleteMany(releasesToRemove.Select(c => c.Id)); } - public RemoteEpisode OldestPendingRelease(int seriesId, IEnumerable episodeIds) + public RemoteEpisode OldestPendingRelease(int seriesId, int[] episodeIds) { - return GetPendingRemoteEpisodes(seriesId).Where(r => r.Episodes.Select(e => e.Id).Intersect(episodeIds).Any()) - .OrderByDescending(p => p.Release.AgeHours) - .FirstOrDefault(); + var seriesReleases = GetPendingReleases(seriesId); + + return seriesReleases.Select(r => r.RemoteEpisode) + .Where(r => r.Episodes.Select(e => e.Id).Intersect(episodeIds).Any()) + .OrderByDescending(p => p.Release.AgeHours) + .FirstOrDefault(); } private List GetPendingReleases() + { + return IncludeRemoteEpisodes(_repository.All().ToList()); + } + + private List GetPendingReleases(int seriesId) + { + return IncludeRemoteEpisodes(_repository.AllBySeriesId(seriesId).ToList()); + } + + private List IncludeRemoteEpisodes(List releases) { var result = new List(); + var seriesMap = _seriesService.GetSeries(releases.Select(v => v.SeriesId).Distinct()) + .ToDictionary(v => v.Id); - foreach (var release in _repository.All()) + foreach (var release in releases) { - var remoteEpisode = GetRemoteEpisode(release); + var series = seriesMap.GetValueOrDefault(release.SeriesId); - if (remoteEpisode == null) continue; + // Just in case the series was removed, but wasn't cleaned up yet (housekeeper will clean it up) + if (series == null) return null; - release.RemoteEpisode = remoteEpisode; + var episodes = _parsingService.GetEpisodes(release.ParsedEpisodeInfo, series, true); + + release.RemoteEpisode = new RemoteEpisode + { + Series = series, + Episodes = episodes, + ParsedEpisodeInfo = release.ParsedEpisodeInfo, + Release = release.Release + }; result.Add(release); } @@ -231,24 +283,6 @@ namespace NzbDrone.Core.Download.Pending return result; } - private RemoteEpisode GetRemoteEpisode(PendingRelease release) - { - var series = _seriesService.GetSeries(release.SeriesId); - - //Just in case the series was removed, but wasn't cleaned up yet (housekeeper will clean it up) - if (series == null) return null; - - var episodes = _parsingService.GetEpisodes(release.ParsedEpisodeInfo, series, true); - - return new RemoteEpisode - { - Series = series, - Episodes = episodes, - ParsedEpisodeInfo = release.ParsedEpisodeInfo, - Release = release.Release - }; - } - private void Insert(DownloadDecision decision, PendingReleaseReason reason) { _repository.Insert(new PendingRelease @@ -288,7 +322,7 @@ namespace NzbDrone.Core.Download.Pending private void RemoveGrabbed(RemoteEpisode remoteEpisode) { - var pendingReleases = GetPendingReleases(); + var pendingReleases = GetPendingReleases(remoteEpisode.Series.Id); var episodeIds = remoteEpisode.Episodes.Select(e => e.Id); var existingReports = pendingReleases.Where(r => r.RemoteEpisode.Episodes.Select(e => e.Id) diff --git a/src/NzbDrone.Core/Download/ProcessDownloadDecisions.cs b/src/NzbDrone.Core/Download/ProcessDownloadDecisions.cs index 6fb248bf0..5614121b2 100644 --- a/src/NzbDrone.Core/Download/ProcessDownloadDecisions.cs +++ b/src/NzbDrone.Core/Download/ProcessDownloadDecisions.cs @@ -131,6 +131,8 @@ namespace NzbDrone.Core.Download var pending = new List(); var stored = new List(); + var addQueue = new List>(); + foreach (var report in failed) { // If a release was already grabbed with matching episodes we should store it as a fallback @@ -141,22 +143,27 @@ namespace NzbDrone.Core.Download if (IsEpisodeProcessed(grabbed, report)) { - _pendingReleaseService.Add(report, PendingReleaseReason.Fallback); + addQueue.Add(Tuple.Create(report, PendingReleaseReason.Fallback)); pending.Add(report); } else if (IsEpisodeProcessed(stored, report)) { - _pendingReleaseService.Add(report, PendingReleaseReason.Fallback); + addQueue.Add(Tuple.Create(report, PendingReleaseReason.Fallback)); pending.Add(report); } else { - _pendingReleaseService.Add(report, PendingReleaseReason.DownloadClientUnavailable); + addQueue.Add(Tuple.Create(report, PendingReleaseReason.DownloadClientUnavailable)); pending.Add(report); stored.Add(report); } } + if (addQueue.Any()) + { + _pendingReleaseService.AddMany(addQueue); + } + return pending; } }