diff --git a/src/Chaptarr.Core.Test/Books/BookProviderIdCanonicalizationFixture.cs b/src/Chaptarr.Core.Test/Books/BookProviderIdCanonicalizationFixture.cs index 89b4f4a0..f20d2e24 100644 --- a/src/Chaptarr.Core.Test/Books/BookProviderIdCanonicalizationFixture.cs +++ b/src/Chaptarr.Core.Test/Books/BookProviderIdCanonicalizationFixture.cs @@ -83,6 +83,14 @@ protected override object Invoke(MethodInfo targetMethod, object[] args) return Editions.Where(e => e.BookId == bookId).ToList(); } + if (targetMethod?.Name == nameof(IEditionService.GetEditionsByBook) && + args?.Length == 1 && + args[0] is IEnumerable bookIds) + { + var wanted = bookIds.ToHashSet(); + return Editions.Where(e => wanted.Contains(e.BookId)).ToList(); + } + throw new NotImplementedException($"Test proxy does not implement IEditionService.{targetMethod?.Name}"); } } diff --git a/src/Chaptarr.Core.Test/Books/BookServiceFormatMonitoringSyncFixture.cs b/src/Chaptarr.Core.Test/Books/BookServiceFormatMonitoringSyncFixture.cs index 6eaff1b3..a47658c0 100644 --- a/src/Chaptarr.Core.Test/Books/BookServiceFormatMonitoringSyncFixture.cs +++ b/src/Chaptarr.Core.Test/Books/BookServiceFormatMonitoringSyncFixture.cs @@ -169,7 +169,13 @@ public List GetEditionsByBook(IEnumerable bookIds) public void UpdateMany(List editions) => throw new NotImplementedException(); public void DeleteMany(List editions) => throw new NotImplementedException(); public List GetEditionsForRefresh(int bookId) => throw new NotImplementedException(); - public List GetEditionsByAuthor(int authorId) => throw new NotImplementedException(); + public int AuthorLookupCount { get; private set; } + + public List GetEditionsByAuthor(int authorId) + { + AuthorLookupCount++; + return _editions.ToList(); + } public Edition FindByTitle(int authorId, string title) => throw new NotImplementedException(); public Edition FindByTitleInexact(int authorId, string title) => throw new NotImplementedException(); public List GetCandidates(int authorId, string title) => throw new NotImplementedException(); @@ -333,6 +339,83 @@ private static BookService BuildService(StubBookRepository repository, StubAutho logger: LogManager.GetCurrentClassLogger()); } + private sealed class CountingLazyEditions : NzbDrone.Core.Datastore.LazyLoaded> + { + private readonly Counter _counter; + + public CountingLazyEditions(Counter counter) + { + _counter = counter; + } + + public override void LazyLoad() + { + if (IsLoaded) + { + return; + } + + _counter.Count++; + _value = new List(); + IsLoaded = true; + } + } + + private sealed class Counter + { + public int Count { get; set; } + } + + [Test] + public void update_many_should_load_author_editions_in_one_query_instead_of_one_per_book() + { + var author = BuildAuthor(1); + var counter = new Counter(); + var books = Enumerable.Range(0, 25) + .Select(i => BuildBook(10 + i, author.Id, i % 2 == 0 ? BookMediaType.Audiobook : BookMediaType.Ebook, $"hc:work-{i / 2}", monitored: false)) + .ToList(); + foreach (var book in books) + { + book.LazyEditions = new CountingLazyEditions(counter); + } + + var editions = books.Select(book => new Edition { Id = book.Id * 10, BookId = book.Id, Asin = $"ASIN{book.Id}" }).ToList(); + var editionService = new StubEditionService(editions); + var repository = new StubBookRepository(books); + var service = BuildService(repository, new StubAuthorService(new[] { author }), editionService: editionService); + + var changed = books[0]; + changed.SetMonitored(true); + service.UpdateMany(new List { changed }); + + Assert.That(counter.Count, Is.EqualTo(0), "no per-book lazy Editions load should be needed"); + Assert.That(editionService.AuthorLookupCount, Is.EqualTo(1), "exactly one author-wide query; the changed books use the by-book query"); + } + + [Test] + public void update_many_should_give_each_book_only_its_own_editions_and_not_touch_loaded_books() + { + var author = BuildAuthor(1); + var first = BuildBook(10, author.Id, BookMediaType.Audiobook, "hc:work-1", monitored: false); + var second = BuildBook(11, author.Id, BookMediaType.Ebook, "hc:work-1", monitored: false); + first.LazyEditions = new CountingLazyEditions(new Counter()); + var preloaded = new List { new Edition { Id = 999, BookId = 11, Asin = "KEEP" } }; + second.Editions = preloaded; + + var editionService = new StubEditionService(new[] + { + new Edition { Id = 100, BookId = 10, Asin = "A" }, + new Edition { Id = 110, BookId = 11, Asin = "B" } + }); + var service = BuildService(new StubBookRepository(new[] { first, second }), new StubAuthorService(new[] { author }), editionService: editionService); + + first.SetMonitored(true); + service.UpdateMany(new List { first }); + + Assert.That(first.Editions.Select(e => e.Id), Is.EqualTo(new[] { 100 })); + Assert.That(second.Editions, Is.SameAs(preloaded), "an already-loaded book must keep its in-memory editions"); + } + [Test] public void set_book_monitored_should_enable_one_sibling_format() { diff --git a/src/NzbDrone.Core/Books/Services/BookService.cs b/src/NzbDrone.Core/Books/Services/BookService.cs index 39f75f8e..d4cf7d01 100644 --- a/src/NzbDrone.Core/Books/Services/BookService.cs +++ b/src/NzbDrone.Core/Books/Services/BookService.cs @@ -1205,6 +1205,64 @@ private static bool HasMonitoringChanged(Book book, MonitoredStateSnapshot snaps book.EbookMonitored != snapshot.EbookMonitored; } + // Book.Editions is lazy-loaded, and both BuildWorkGroups (BookIdentity.GetProviderIdentityTokens -> + // BookEditionIdentity.GetOrderedEditions) and CloneStoredBook (GetAsin) read it. Handing a whole author's + // books to them therefore cost one Editions query per book, on every save that reaches the format-sync + // pass (for an author with thousands of books, most of a refresh's wall-clock time). Fill the books that + // have not loaded their editions from a single per-author query instead; already-loaded books are left + // alone, and the rows are the same ones the lazy loader would have returned (Editions by BookId). + private void PreloadEditions(int authorId, IEnumerable books) + { + if (_editionService == null || books == null) + { + return; + } + + var unloaded = books + .Where(book => book != null && book.Id > 0 && (book.LazyEditions == null || !book.LazyEditions.IsLoaded)) + .ToList(); + + if (unloaded.Count == 0) + { + return; + } + + var editionsByBookId = (_editionService.GetEditionsByAuthor(authorId) ?? new List()) + .ToLookup(edition => edition.BookId); + + foreach (var book in unloaded) + { + book.Editions = editionsByBookId[book.Id].ToList(); + } + } + + // Same idea as PreloadEditions, for a caller that holds a specific set of books rather than a whole + // author's: one Editions-by-BookId query for the ones that have not loaded them. + private void PreloadEditionsByBook(IEnumerable books) + { + if (_editionService == null || books == null) + { + return; + } + + var unloaded = books + .Where(book => book != null && book.Id > 0 && (book.LazyEditions == null || !book.LazyEditions.IsLoaded)) + .ToList(); + + if (unloaded.Count == 0) + { + return; + } + + var editionsByBookId = (_editionService.GetEditionsByBook(unloaded.Select(book => book.Id).Distinct().ToList()) ?? new List()) + .ToLookup(edition => edition.BookId); + + foreach (var book in unloaded) + { + book.Editions = editionsByBookId[book.Id].ToList(); + } + } + private static Book CloneStoredBook(Book book) { if (book == null) @@ -1483,6 +1541,7 @@ private List GetSyncUpdatesForMutations(List cha } var repositoryBooks = _bookRepository.GetBooksByAuthorId(authorBooks.Key) ?? new List(); + PreloadEditions(authorBooks.Key, repositoryBooks); var authorStoredById = repositoryBooks.ToDictionary(book => book.Id, CloneStoredBook); var authorBooksById = repositoryBooks.ToDictionary(book => book.Id); @@ -1496,6 +1555,10 @@ private List GetSyncUpdatesForMutations(List cha authorBooksById[changedBook.Id] = changedBook; } + // The caller's changed books replaced their repository copies above; fill only those (a small + // IN query) rather than re-running the author-wide query. + PreloadEditionsByBook(authorBooks); + var changedBookIds = authorBooks.Select(book => book.Id).ToHashSet(); var baseStates = authorBooksById.ToDictionary(pair => pair.Key, pair => SnapshotMonitoredState(pair.Value)); @@ -1541,6 +1604,7 @@ private void ApplyInsertSyncDefaults(List books) var combinedBooks = (_bookRepository.GetBooksByAuthorId(authorBooks.Key) ?? new List()) .Concat(insertedBooks) .ToList(); + PreloadEditions(authorBooks.Key, combinedBooks); foreach (var workGroup in BuildWorkGroups(combinedBooks)) { @@ -1607,6 +1671,8 @@ private List BuildReconcileSyncUpdates(IEnumerable book.Id, CloneStoredBook); var baseStates = authorBooks.ToDictionary(book => book.Id, SnapshotMonitoredState); @@ -1725,7 +1791,9 @@ public void UpdateMany(List books) // Ensure unique TitleSlugs for duplicate books when updating EnsureUniqueTitleSlugs(books); books.ForEach(EnsureBookDbFields); - var storedById = _bookRepository.Get(books.Select(book => book.Id)).ToDictionary(book => book.Id, CloneStoredBook); + var storedBooks = _bookRepository.Get(books.Select(book => book.Id)).ToList(); + PreloadEditionsByBook(storedBooks); + var storedById = storedBooks.ToDictionary(book => book.Id, CloneStoredBook); var syncUpdates = GetSyncUpdatesForMutations(books, storedById); var booksToUpdate = books .Concat(syncUpdates.Select(update => update.Book)) @@ -1756,9 +1824,11 @@ private List PersistWithLifecycle(List books) } var bookIds = changedBooks.Select(book => book.Id).ToList(); - var storedById = _bookRepository.FindExisting(bookIds) + var storedBooks = _bookRepository.FindExisting(bookIds) .Where(book => book != null) - .ToDictionary(book => book.Id, CloneStoredBook); + .ToList(); + PreloadEditionsByBook(storedBooks); + var storedById = storedBooks.ToDictionary(book => book.Id, CloneStoredBook); changedBooks = changedBooks.Where(book => storedById.ContainsKey(book.Id)).ToList(); if (!changedBooks.Any()) @@ -2111,6 +2181,7 @@ public void SetBookMonitored(int bookId, bool monitored) public void SetMonitored(IEnumerable ids, bool monitored) { var books = _bookRepository.Get(ids).ToList(); + PreloadEditionsByBook(books); var storedById = books.ToDictionary(book => book.Id, CloneStoredBook); foreach (var book in books) @@ -2155,6 +2226,7 @@ public void SetMonitoredForMediaType(IEnumerable ids, string mediaType, boo } var books = _bookRepository.Get(ids).ToList(); + PreloadEditionsByBook(books); var storedById = books.ToDictionary(book => book.Id, CloneStoredBook); var booksToMutate = new List();