Fixed: Progressively degrading performance issue in Pending/Delayed releases system

pull/131/head
Qstick 7 years ago
parent fa051257e3
commit b3fbd81594

@ -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 ParsedAlbumInfo _parsedAlbumInfo;
private RemoteAlbum _remoteAlbum;
private List<PendingRelease> _heldReleases;
[SetUp]
public void Setup()
@ -63,14 +65,24 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests
_temporarilyRejected = new DownloadDecision(_remoteAlbum, new Rejection("Temp Rejected", RejectionType.Temporary));
_heldReleases = new List<PendingRelease>();
Mocker.GetMock<IPendingReleaseRepository>()
.Setup(s => s.All())
.Returns(new List<PendingRelease>());
.Returns(_heldReleases);
Mocker.GetMock<IPendingReleaseRepository>()
.Setup(s => s.AllByArtistId(It.IsAny<int>()))
.Returns<int>(i => _heldReleases.Where(v => v.ArtistId == i).ToList());
Mocker.GetMock<IArtistService>()
.Setup(s => s.GetArtist(It.IsAny<int>()))
.Returns(_artist);
Mocker.GetMock<IArtistService>()
.Setup(s => s.GetArtists(It.IsAny<IEnumerable<int>>()))
.Returns(new List<Artist> { _artist });
Mocker.GetMock<IParsingService>()
.Setup(s => s.GetAlbums(It.IsAny<ParsedAlbumInfo>(), _artist, null))
.Returns(new List<Album> {_album});
@ -80,7 +92,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests
.Returns((List<DownloadDecision> 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.ArtistId = _artist.Id)
.With(h => h.Title = title)
.With(h => h.Release = release)
.With(h => h.Reason = reason)
.Build();
Mocker.GetMock<IPendingReleaseRepository>()
.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<IPendingReleaseRepository>()
.Verify(v => v.Delete(It.IsAny<int>()), Times.Once());
}
[Test]
public void should_add_if_title_is_different()
{

@ -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 ParsedAlbumInfo _parsedAlbumInfo;
private RemoteAlbum _remoteAlbum;
private List<PendingRelease> _heldReleases;
[SetUp]
public void Setup()
@ -63,14 +65,24 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests
_temporarilyRejected = new DownloadDecision(_remoteAlbum, new Rejection("Temp Rejected", RejectionType.Temporary));
_heldReleases = new List<PendingRelease>();
Mocker.GetMock<IPendingReleaseRepository>()
.Setup(s => s.All())
.Returns(new List<PendingRelease>());
.Returns(_heldReleases);
Mocker.GetMock<IPendingReleaseRepository>()
.Setup(s => s.AllByArtistId(It.IsAny<int>()))
.Returns<int>(i => _heldReleases.Where(v => v.ArtistId == i).ToList());
Mocker.GetMock<IArtistService>()
.Setup(s => s.GetArtist(It.IsAny<int>()))
.Returns(_artist);
Mocker.GetMock<IArtistService>()
.Setup(s => s.GetArtists(It.IsAny<IEnumerable<int>>()))
.Returns(new List<Artist> { _artist });
Mocker.GetMock<IParsingService>()
.Setup(s => s.GetAlbums(It.IsAny<ParsedAlbumInfo>(), _artist, null))
.Returns(new List<Album> {_album});
@ -92,9 +104,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests
.With(h => h.ParsedAlbumInfo = parsedEpisodeInfo)
.Build();
Mocker.GetMock<IPendingReleaseRepository>()
.Setup(s => s.All())
.Returns(heldReleases);
_heldReleases.AddRange(heldReleases);
}
[Test]

@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Generic;
using System.Linq;
using FizzWare.NBuilder;
using Moq;
@ -38,6 +38,10 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests
.Setup(s => s.GetArtist(It.IsAny<int>()))
.Returns(new Artist());
Mocker.GetMock<IArtistService>()
.Setup(s => s.GetArtists(It.IsAny<IEnumerable<int>>()))
.Returns(new List<Artist> { new Artist() });
Mocker.GetMock<IParsingService>()
.Setup(s => s.GetAlbums(It.IsAny<ParsedAlbumInfo>(), It.IsAny<Artist>(), null))
.Returns(new List<Album>{ _album });

@ -62,7 +62,7 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests
_remoteAlbum.Artist = _artist;
_remoteAlbum.ParsedAlbumInfo = _parsedAlbumInfo;
_remoteAlbum.Release = _release;
_temporarilyRejected = new DownloadDecision(_remoteAlbum, new Rejection("Temp Rejected", RejectionType.Temporary));
Mocker.GetMock<IPendingReleaseRepository>()
@ -73,6 +73,10 @@ namespace NzbDrone.Core.Test.Download.Pending.PendingReleaseServiceTests
.Setup(s => s.GetArtist(It.IsAny<int>()))
.Returns(_artist);
Mocker.GetMock<IArtistService>()
.Setup(s => s.GetArtists(It.IsAny<IEnumerable<int>>()))
.Returns(new List<Artist> { _artist });
Mocker.GetMock<IParsingService>()
.Setup(s => s.GetAlbums(It.IsAny<ParsedAlbumInfo>(), _artist, null))
.Returns(new List<Album> {_album});

@ -94,7 +94,7 @@ namespace NzbDrone.Core.DecisionEngine.Specifications.RssSync
var albumIds = subject.Albums.Select(e => e.Id);
var oldest = _pendingReleaseService.OldestPendingRelease(subject.Artist.Id, albumIds);
var oldest = _pendingReleaseService.OldestPendingRelease(subject.Artist.Id, albumIds.ToArray());
if (oldest != null && oldest.Release.AgeMinutes > delay)
{

@ -1,4 +1,4 @@
using System.Collections.Generic;
using System.Collections.Generic;
using NzbDrone.Core.Datastore;
using NzbDrone.Core.Messaging.Events;
@ -8,6 +8,7 @@ namespace NzbDrone.Core.Download.Pending
{
void DeleteByArtistId(int artistId);
List<PendingRelease> AllByArtistId(int artistId);
List<PendingRelease> WithoutFallback();
}
public class PendingReleaseRepository : BasicRepository<PendingRelease>, IPendingReleaseRepository
@ -26,5 +27,10 @@ namespace NzbDrone.Core.Download.Pending
{
return Query.Where(p => p.ArtistId == artistId);
}
public List<PendingRelease> WithoutFallback()
{
return Query.Where(p => p.Reason != PendingReleaseReason.Fallback);
}
}
}
}

@ -21,12 +21,13 @@ namespace NzbDrone.Core.Download.Pending
public interface IPendingReleaseService
{
void Add(DownloadDecision decision, PendingReleaseReason reason);
void AddMany(List<Tuple<DownloadDecision, PendingReleaseReason>> decisions);
List<ReleaseInfo> GetPending();
List<RemoteAlbum> GetPendingRemoteAlbums(int artistId);
List<Queue.Queue> GetPendingQueue();
Queue.Queue FindPendingQueueItem(int queueId);
void RemovePendingQueueItems(int queueId);
RemoteAlbum OldestPendingRelease(int artistId, IEnumerable<int> albumIds);
RemoteAlbum OldestPendingRelease(int artistId, int[] albumIds);
}
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.AllByArtistId(decision.RemoteAlbum.Artist.Id);
alreadyPending = IncludeRemoteAlbums(alreadyPending);
Add(alreadyPending, decision, reason);
}
public void AddMany(List<Tuple<DownloadDecision, PendingReleaseReason>> decisions)
{
var alreadyPending = decisions.Select(v => v.Item1.RemoteAlbum.Artist.Id).Distinct().SelectMany(_repository.AllByArtistId).ToList();
alreadyPending = IncludeRemoteAlbums(alreadyPending);
foreach (var pair in decisions)
{
Add(alreadyPending, pair.Item1, pair.Item2);
}
}
private void Add(List<PendingRelease> alreadyPending, DownloadDecision decision, PendingReleaseReason reason)
{
var albumIds = decision.RemoteAlbum.Albums.Select(e => e.Id);
var existingReports = alreadyPending.Where(r => r.RemoteAlbum.Albums.Select(e => e.Id)
@ -80,22 +100,32 @@ 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.RemoteAlbum, matchingReport.Reason, reason); matchingReport.Reason = reason;
_repository.Update(matchingReport);
sameReason = false;
}
_logger.Debug("The release {0} is already pending with reason {1}, changing to {2}", decision.RemoteAlbum, matchingReport.Reason, reason);
matchingReport.Reason = reason;
_repository.Update(matchingReport);
}
if (sameReason)
else
{
_logger.Debug("The release {0} is already pending with reason {1}, not adding again", decision.RemoteAlbum, reason); return;
}
if (matchingReports.Count() > 1)
{
_logger.Debug("The release {0} had {1} duplicate pending, removing duplicates.", decision.RemoteAlbum, matchingReports.Count() - 1);
foreach (var duplicate in matchingReports.Skip(1))
{
_repository.Delete(duplicate.Id);
alreadyPending.Remove(duplicate);
}
}
return;
}
_logger.Debug("Adding release {0} to pending releases with reason {1}", decision.RemoteAlbum, reason); Insert(decision, reason);
@ -122,7 +152,7 @@ namespace NzbDrone.Core.Download.Pending
public List<RemoteAlbum> GetPendingRemoteAlbums(int artistId)
{
return _repository.AllByArtistId(artistId).Select(GetRemoteAlbum).ToList();
return IncludeRemoteAlbums(_repository.AllByArtistId(artistId)).Select(v => v.RemoteAlbum).ToList();
}
public List<Queue.Queue> GetPendingQueue()
@ -131,7 +161,8 @@ namespace NzbDrone.Core.Download.Pending
var nextRssSync = new Lazy<DateTime>(() => _taskManager.GetNextExecution(typeof(RssSyncCommand)));
foreach (var pendingRelease in GetPendingReleases().Where(p => p.Reason != PendingReleaseReason.Fallback))
var pendingReleases = IncludeRemoteAlbums(_repository.WithoutFallback());
foreach (var pendingRelease in pendingReleases)
{
foreach (var album in pendingRelease.RemoteAlbum.Albums)
{
@ -203,27 +234,48 @@ namespace NzbDrone.Core.Download.Pending
_repository.DeleteMany(releasesToRemove.Select(c => c.Id));
}
public RemoteAlbum OldestPendingRelease(int artistId, IEnumerable<int> albumIds)
public RemoteAlbum OldestPendingRelease(int artistId, int[] albumIds)
{
return GetPendingRemoteAlbums(artistId).Where(r => r.Albums.Select(e => e.Id).Intersect(albumIds).Any())
.OrderByDescending(p => p.Release.AgeHours)
.FirstOrDefault();
var artistReleases = GetPendingReleases(artistId);
return artistReleases.Select(r => r.RemoteAlbum)
.Where(r => r.Albums.Select(e => e.Id).Intersect(albumIds).Any())
.OrderByDescending(p => p.Release.AgeHours)
.FirstOrDefault();
}
private List<PendingRelease> GetPendingReleases()
{
return IncludeRemoteAlbums(_repository.All().ToList());
}
private List<PendingRelease> GetPendingReleases(int artistId)
{
return IncludeRemoteAlbums(_repository.AllByArtistId(artistId).ToList());
}
private List<PendingRelease> IncludeRemoteAlbums(List<PendingRelease> releases)
{
var result = new List<PendingRelease>();
var artistMap = _artistService.GetArtists(releases.Select(v => v.ArtistId).Distinct())
.ToDictionary(v => v.Id);
foreach (var release in _repository.All())
foreach (var release in releases)
{
var remoteAlbum = GetRemoteAlbum(release);
var artist = artistMap.GetValueOrDefault(release.ArtistId);
if (remoteAlbum == null)
{
continue;
}
// Just in case the artist was removed, but wasn't cleaned up yet (housekeeper will clean it up)
if (artist == null) return null;
var albums = _parsingService.GetAlbums(release.ParsedAlbumInfo, artist);
release.RemoteAlbum = remoteAlbum;
release.RemoteAlbum = new RemoteAlbum
{
Artist = artist,
Albums = albums,
ParsedAlbumInfo = release.ParsedAlbumInfo,
Release = release.Release
};
result.Add(release);
}
@ -231,27 +283,6 @@ namespace NzbDrone.Core.Download.Pending
return result;
}
private RemoteAlbum GetRemoteAlbum(PendingRelease release)
{
var artist = _artistService.GetArtist(release.ArtistId);
//Just in case the series was removed, but wasn't cleaned up yet (housekeeper will clean it up)
if (artist == null)
{
return null;
}
var album = _parsingService.GetAlbums(release.ParsedAlbumInfo, artist);
return new RemoteAlbum
{
Artist = artist,
Albums = album,
ParsedAlbumInfo = release.ParsedAlbumInfo,
Release = release.Release
};
}
private void Insert(DownloadDecision decision, PendingReleaseReason reason)
{
_repository.Insert(new PendingRelease
@ -291,7 +322,7 @@ namespace NzbDrone.Core.Download.Pending
private void RemoveGrabbed(RemoteAlbum remoteAlbum)
{
var pendingReleases = GetPendingReleases();
var pendingReleases = GetPendingReleases(remoteAlbum.Artist.Id);
var albumIds = remoteAlbum.Albums.Select(e => e.Id);
var existingReports = pendingReleases.Where(r => r.RemoteAlbum.Albums.Select(e => e.Id)

@ -131,6 +131,8 @@ namespace NzbDrone.Core.Download
var pending = new List<DownloadDecision>();
var stored = new List<DownloadDecision>();
var addQueue = new List<Tuple<DownloadDecision, PendingReleaseReason>>();
foreach (var report in failed)
{
// If a release was already grabbed with matching albums we should store it as a fallback
@ -141,22 +143,27 @@ namespace NzbDrone.Core.Download
if (IsAlbumProcessed(grabbed, report))
{
_pendingReleaseService.Add(report, PendingReleaseReason.Fallback);
addQueue.Add(Tuple.Create(report, PendingReleaseReason.Fallback));
pending.Add(report);
}
else if (IsAlbumProcessed(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;
}
}

Loading…
Cancel
Save