From 7b2fca930b50c154e7ddd1a158717842ae9773a3 Mon Sep 17 00:00:00 2001 From: jordan Date: Tue, 29 Sep 2026 17:15:22 +0000 Subject: [PATCH 1/2] perf(books): load editions in bulk before format-sync work grouping instead of one query per book Book.Editions is lazy-loaded. BuildWorkGroups (BookIdentity.GetProviderIdentityTokens -> BookEditionIdentity.GetOrderedEditions) and CloneStoredBook (GetAsin) both read it, and the format-sync pass hands them every one of an author's books on each save (UpdateMany, insert defaults, bulk reconcile). That issued one Editions query per book, per save - a stack sample of a bulk refresh spent most samples in that lazy load or in the DB round trips around it, on a single thread. Add PreloadEditions (one GetEditionsByAuthor per author) and PreloadEditionsByBook (one IN query for a given set) and call them right after the books are fetched and before they are cloned or grouped. Only books whose editions have not loaded are filled, with the same rows the lazy loader returns (Editions by BookId), so already-loaded books keep their in-memory editions. Tests: an UpdateMany over 25 books performs no per-book lazy loads and one bulk author query; each book gets only its own editions and a loaded book is left untouched. Both fail without the change. The strict edition-service proxy in BookProviderIdCanonicalizationFixture learns the bulk GetEditionsByBook overload. --- .../BookProviderIdCanonicalizationFixture.cs | 8 ++ .../BookServiceFormatMonitoringSyncFixture.cs | 85 ++++++++++++++++++- .../Books/Services/BookService.cs | 76 ++++++++++++++++- 3 files changed, 165 insertions(+), 4 deletions(-) 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..8dbb4b34 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.LessThanOrEqualTo(2), "one bulk query per author (plus at most one for the changed books)"); + } + + [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..6817a607 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,8 @@ private List GetSyncUpdatesForMutations(List cha authorBooksById[changedBook.Id] = changedBook; } + PreloadEditions(authorBooks.Key, authorBooks); + var changedBookIds = authorBooks.Select(book => book.Id).ToHashSet(); var baseStates = authorBooksById.ToDictionary(pair => pair.Key, pair => SnapshotMonitoredState(pair.Value)); @@ -1541,6 +1602,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 +1669,8 @@ private List BuildReconcileSyncUpdates(IEnumerable book.Id, CloneStoredBook); var baseStates = authorBooks.ToDictionary(book => book.Id, SnapshotMonitoredState); @@ -1725,7 +1789,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 +1822,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 +2179,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 +2224,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(); From df2be9bd36a4ba1c9d46f27f2d8ce0d3f9b70c78 Mon Sep 17 00:00:00 2001 From: jordan Date: Tue, 29 Sep 2026 17:17:17 +0000 Subject: [PATCH 2/2] Fill only the changed books by id in the format-sync pass instead of re-running the author-wide editions query Adversarial review of #265: the second PreloadEditions call in GetSyncUpdatesForMutations re-loaded every edition for the author whenever the caller's changed books were unloaded (the normal case). Use the by-book IN query for just those books, and pin exactly one author-wide query in the test. --- .../Books/BookServiceFormatMonitoringSyncFixture.cs | 2 +- src/NzbDrone.Core/Books/Services/BookService.cs | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Chaptarr.Core.Test/Books/BookServiceFormatMonitoringSyncFixture.cs b/src/Chaptarr.Core.Test/Books/BookServiceFormatMonitoringSyncFixture.cs index 8dbb4b34..a47658c0 100644 --- a/src/Chaptarr.Core.Test/Books/BookServiceFormatMonitoringSyncFixture.cs +++ b/src/Chaptarr.Core.Test/Books/BookServiceFormatMonitoringSyncFixture.cs @@ -389,7 +389,7 @@ public void update_many_should_load_author_editions_in_one_query_instead_of_one_ 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.LessThanOrEqualTo(2), "one bulk query per author (plus at most one for the changed books)"); + Assert.That(editionService.AuthorLookupCount, Is.EqualTo(1), "exactly one author-wide query; the changed books use the by-book query"); } [Test] diff --git a/src/NzbDrone.Core/Books/Services/BookService.cs b/src/NzbDrone.Core/Books/Services/BookService.cs index 6817a607..d4cf7d01 100644 --- a/src/NzbDrone.Core/Books/Services/BookService.cs +++ b/src/NzbDrone.Core/Books/Services/BookService.cs @@ -1555,7 +1555,9 @@ private List GetSyncUpdatesForMutations(List cha authorBooksById[changedBook.Id] = changedBook; } - PreloadEditions(authorBooks.Key, authorBooks); + // 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));