perf(refresh): stop refetching Book+Author+Editions+Files per book during author refresh (#163) - #166
Open
jordanfelle wants to merge 1 commit into
Open
jordanfelle wants to merge 1 commit into
jordanfelle wants to merge 1 commit into
Conversation
… author refresh (Chaptarr#163) RefreshBookService.PublishEntityUpdatedEvent called _bookService.GetBook(id) unconditionally for every book that changed during an author refresh, doing a full Book+Author+Editions+BookFiles reload even though the author-level refresh had already batch-loaded Editions/BookFiles for every book, and already has the author in memory (remoteData). On a large library this produced hundreds of thousands of avoidable per-book queries (measured via pg_stat_statements: 254k "Authors JOIN Books WHERE Books.Id = $1" calls and 233k "Editions WHERE BookId = $1" calls in a 204s window), which is why authors with very large catalogues (Charles Dickens, 10k+ books) can take hours to refresh and starve other commands of chaptarr's single disk-access slot in the meantime. Reuse the already-hydrated entity when its Editions/BookFiles were already loaded, falling back to the original full fetch otherwise (e.g. a merge target, or a direct single-book refresh that didn't pre-hydrate) so behavior for every other caller is unchanged. While testing this against an isolated copy of a production database, found and fixed a related footgun: Book.Author/.Editions/.BookFiles are backed by LazyLoaded<TParent,TChild> proxies that transparently issue a DB query the first time the materializing property is read. A naive `== null` check on those properties fires the exact query it's trying to detect; the fix checks the non-materializing Lazy*.IsLoaded flag instead. Testing: built and ran in an isolated container against a template copy of a production chaptarr-main database (10k+ book author), confirmed no regressions in a RefreshAuthor run. Query-volume improvement was smaller than expected in that isolated test, suggesting at least one more contributor to the per-book query volume remains; investigating separately.
jordanfelle
added a commit
to jordanfelle/chaptarr
that referenced
this pull request
Sep 22, 2026
…oring sync (Chaptarr#163) GetSyncUpdatesForMutations (called via SaveEntity on every book save) calls ApplyMutationSyncForWorkGroup once per author-owned work group when that author has SyncMonitoredAcrossFormats enabled. That path could call EnsureOneMonitoredOnFormat -> CanEnableMonitoringForMediaType -> HasCompatibleRootFolderForMediaType, which called _rootFolderService.All() fresh every time - an uncached full table read of a small, rarely-changing table, repeated once per media type per work group per changed book. Fetch it once per author (only for authors that actually have sync enabled) and thread it through instead. Existing call sites that aren't on this hot path (ApplyInsertSyncDefaults, BuildReconcileSyncUpdates, the direct CanEnableMonitoringForMediaType caller) are unchanged - the new parameter is optional and falls back to the original per-call fetch when not supplied. Measured on an isolated copy of a production database, on top of Chaptarr#166 and Chaptarr#167: the RootFolders query, which was running ~2,000/min during a RefreshAuthor pass, dropped out of the top statements entirely.
This was referenced Sep 22, 2026
Author
|
Tracked under #173 (author-refresh N+1/O(N^2) query cluster). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
RefreshBookService.PublishEntityUpdatedEventcalled_bookService.GetBook(id)unconditionally for every book that changed during an author refresh - a fullBook + Author + Editions + BookFilesreload, even though the author-level refresh already batch-loadedEditions/BookFilesfor every book up front (RefreshAuthorService.HydrateLocalChildrenForRefresh), andremoteData(the author) is already in memory and identical for every book in a single author-scoped refresh call.On a large library this produced a very large number of avoidable per-book queries. Measured via
pg_stat_statementson a live instance (204s window, mid-refresh): 254,056 calls toAuthors JOIN Books WHERE Books.Id = $1and 233,131 calls toEditions WHERE BookId = $1. This is a direct contributor to why authors with very large catalogues (e.g. Charles Dickens, 10k+ books) can take hours to refresh, and starve other commands of chaptarr's single disk-access slot for the duration.Reuse the already-hydrated entity when its
Editions/BookFilesare already loaded and itsAuthoris already set, instead of re-querying. Falls back to the original fullGetBook()fetch when something is genuinely missing (a merge target fetched viaGetEntityByForeignId, or a direct single-book refresh call that didn't pre-hydrate), so behavior for every other caller/subscriber is unchanged.Also fixes a related footgun found while testing this:
Book.Author/.Editions/.BookFilesare backed byLazyLoaded<TParent,TChild>proxies that transparently issue a DB query the first time the materializing property is read. A naive== nullcheck on those properties fires the exact query it's trying to detect. The fix checks the non-materializingLazy*.IsLoadedflag instead, and pre-assignsbook.Authorbefore any code path can read it for the first time.Fixes # (part of #163, tracked under #173)
Database Migration
NO
How was this tested?
chaptarr-maindatabase (real library, largest author has 10k+ books), so as not to touch any live data.RefreshAuthorrun completes without errors/regressions.BookUpdatedEventsubscriber: the published event still carries a fully-hydratedBookin every case, just sourced from memory instead of the database when possible.RefreshAuthorpass over the whole library (400+ authors, including several with 3,000-10,000+ books) completed with no errors.Screenshots (UI changes only)
N/A