From 80eaccd1e6822117b5926a5dc87476c96136cb93 Mon Sep 17 00:00:00 2001 From: Jonathan Finley Date: Fri, 13 Mar 2026 23:07:57 -0400 Subject: [PATCH] perf: tighten derived filter cache policy --- server/Database.js | 30 ++++ server/controllers/LibraryController.js | 17 +- server/scanner/LibraryItemScanner.js | 2 + server/scanner/LibraryScanner.js | 8 +- server/utils/queries/libraryFilters.js | 146 +++--------------- .../utils/queries/libraryItemsBookFilters.js | 4 +- ...braryController.largeLibraryBrowse.test.js | 63 ++++++++ 7 files changed, 131 insertions(+), 139 deletions(-) diff --git a/server/Database.js b/server/Database.js index 213c2c61b..c20b78785 100644 --- a/server/Database.js +++ b/server/Database.js @@ -21,6 +21,7 @@ class Database { // Cached library filter data this.libraryFilterData = {} + this.libraryFilterDataTtlMs = 1000 * 60 * 30 /** @type {import('./objects/settings/ServerSettings')} */ this.serverSettings = null @@ -583,6 +584,35 @@ class Database { this.libraryFilterData[libraryId].languages.push(language) } + getLibraryFilterCache(libraryId, { includeExpired = false } = {}) { + const cacheEntry = this.libraryFilterData[libraryId] + if (!cacheEntry) return undefined + if (includeExpired) return cacheEntry + + if (cacheEntry.expiresAt && cacheEntry.expiresAt > Date.now()) { + return cacheEntry + } + + delete this.libraryFilterData[libraryId] + return undefined + } + + setLibraryFilterCache(libraryId, data, ttlMs = this.libraryFilterDataTtlMs) { + const loadedAt = Date.now() + const expiresAt = loadedAt + ttlMs + const cacheEntry = { + ...data, + loadedAt, + expiresAt + } + this.libraryFilterData[libraryId] = cacheEntry + return cacheEntry + } + + invalidateLibraryFilterCache(libraryId) { + delete this.libraryFilterData[libraryId] + } + /** * Used when updating items to make sure author id exists * If library filter data is set then use that for check diff --git a/server/controllers/LibraryController.js b/server/controllers/LibraryController.js index 29864463c..646b1a6d6 100644 --- a/server/controllers/LibraryController.js +++ b/server/controllers/LibraryController.js @@ -510,7 +510,11 @@ class LibraryController { } SocketAuthority.emitter('library_updated', req.library.toOldJSON(), userFilter) - await Database.resetLibraryIssuesFilterData(req.library.id) + if (hasFolderUpdates) { + Database.invalidateLibraryFilterCache(req.library.id) + } else { + await Database.resetLibraryIssuesFilterData(req.library.id) + } } return res.json(req.library.toOldJSON()) } @@ -588,9 +592,7 @@ class LibraryController { SocketAuthority.emitter('library_removed', libraryJson) // Remove library filter data - if (Database.libraryFilterData[req.library.id]) { - delete Database.libraryFilterData[req.library.id] - } + Database.invalidateLibraryFilterCache(req.library.id) return res.json(libraryJson) } @@ -742,10 +744,7 @@ class LibraryController { await this.checkRemoveEmptySeries(seriesIds) } - // Set numIssues to 0 for library filter data - if (Database.libraryFilterData[req.library.id]) { - Database.libraryFilterData[req.library.id].numIssues = 0 - } + Database.invalidateLibraryFilterCache(req.library.id) res.sendStatus(200) } @@ -1287,7 +1286,7 @@ class LibraryController { const forceRescan = req.query.force === '1' await LibraryScanner.scan(req.library, forceRescan) - await Database.resetLibraryIssuesFilterData(req.library.id) + Database.invalidateLibraryFilterCache(req.library.id) Logger.info('[LibraryController] Scan complete') } diff --git a/server/scanner/LibraryItemScanner.js b/server/scanner/LibraryItemScanner.js index 501df4274..2322b0936 100644 --- a/server/scanner/LibraryItemScanner.js +++ b/server/scanner/LibraryItemScanner.js @@ -64,6 +64,7 @@ class LibraryItemScanner { const { libraryItem: expandedLibraryItem, wasUpdated } = await this.rescanLibraryItemMedia(libraryItem, libraryItemScanData, library.settings, scanLogger) if (libraryItemDataUpdated || wasUpdated) { + Database.invalidateLibraryFilterCache(library.id) SocketAuthority.libraryItemEmitter('item_updated', expandedLibraryItem) await this.checkAuthorsAndSeriesRemovedFromBooks(library.id, scanLogger) @@ -190,6 +191,7 @@ class LibraryItemScanner { newLibraryItem = await PodcastScanner.scanNewPodcastLibraryItem(libraryItemData, librarySettings, libraryScan) } if (newLibraryItem) { + Database.invalidateLibraryFilterCache(libraryItemData.libraryId) libraryScan.addLog(LogLevel.INFO, `Created new library item "${newLibraryItem.relPath}" with id "${newLibraryItem.id}"`) } return newLibraryItem diff --git a/server/scanner/LibraryScanner.js b/server/scanner/LibraryScanner.js index 640c82d76..f4d4756fd 100644 --- a/server/scanner/LibraryScanner.js +++ b/server/scanner/LibraryScanner.js @@ -89,6 +89,10 @@ class LibraryScanner { const canceled = await this.scanLibrary(libraryScan, forceRescan) libraryScan.setComplete() + if (!canceled) { + Database.invalidateLibraryFilterCache(library.id) + } + Logger.info(`[LibraryScanner] Library scan "${libraryScan.id}" ${canceled ? 'canceled after' : 'completed in'} ${libraryScan.elapsedTimestamp} | ${libraryScan.resultStats}`) if (!canceled) { @@ -431,9 +435,9 @@ class LibraryScanner { } }) - // If something was updated then reset numIssues filter data for library + // If something was updated then invalidate derived filter data for library if (resetFilterData) { - await Database.resetLibraryIssuesFilterData(libraryId) + Database.invalidateLibraryFilterCache(libraryId) } } diff --git a/server/utils/queries/libraryFilters.js b/server/utils/queries/libraryFilters.js index 3a9c38abd..5dbb3787b 100644 --- a/server/utils/queries/libraryFilters.js +++ b/server/utils/queries/libraryFilters.js @@ -450,14 +450,9 @@ module.exports = { * @returns {Promise} */ async getFilterData(mediaType, libraryId) { - const cachedFilterData = Database.libraryFilterData[libraryId] - if (cachedFilterData) { - const cacheElapsed = Date.now() - cachedFilterData.loadedAt - // Cache library filters for 30 mins - // TODO: Keep cached filter data up-to-date on updates - if (cacheElapsed < 1000 * 60 * 30) { - return cachedFilterData - } + const freshFilterData = Database.getLibraryFilterCache(libraryId) + if (freshFilterData) { + return freshFilterData } const start = Date.now() // Temp for checking load times @@ -477,9 +472,14 @@ module.exports = { numIssues: 0 } - const lastLoadedAt = cachedFilterData ? cachedFilterData.loadedAt : 0 - - if (mediaType === 'podcast') { + // Virtual ebook libraries get content from external API, so filter data is minimal + if (mediaType === 'ebook') { + // For ebook libraries, filter data will be populated from the external API + // Return empty filter data for now - can be enhanced to cache API-provided filters + Database.setLibraryFilterCache(libraryId, data) + Logger.debug(`Loaded empty filterdata for ebook library in ${((Date.now() - start) / 1000).toFixed(2)}s`) + return Database.libraryFilterData[libraryId] + } else if (mediaType === 'podcast') { // Check how many podcasts are in library to determine if we need to load all of the data // This is done to handle the edge case of podcasts having been deleted and not having // an updatedAt timestamp to trigger a reload of the filter data @@ -494,42 +494,6 @@ module.exports = { } }) - // To reduce the cold-start load time, first check if any podcasts - // have an "updatedAt" timestamp since the last time the filter - // data was loaded. If so, we can skip loading all of the data. - // Because many items could change, just check the count of items instead - // of actually loading the data twice - const changedPodcasts = await podcastModelCount({ - include: { - model: Database.libraryItemModel, - attributes: [], - where: { - libraryId: libraryId, - updatedAt: { - [Sequelize.Op.gt]: new Date(lastLoadedAt) - } - } - }, - where: { - updatedAt: { - [Sequelize.Op.gt]: new Date(lastLoadedAt) - } - }, - limit: 1 - }) - - if (changedPodcasts === 0) { - // If nothing has changed, check if the number of podcasts in - // library is still the same as prior check before updating cache creation time - - if (podcastCountFromDatabase === Database.libraryFilterData[libraryId]?.podcastCount) { - Logger.debug(`Filter data for ${libraryId} has not changed, returning cached data and updating cache time after ${((Date.now() - start) / 1000).toFixed(2)}s`) - Database.libraryFilterData[libraryId].loadedAt = Date.now() - return cachedFilterData - } - } - - // Something has changed in the podcasts table, so reload all of the filter data for library const findAll = process.env.QUERY_PROFILING ? profile(Database.podcastModel.findAll.bind(Database.podcastModel)) : Database.podcastModel.findAll.bind(Database.podcastModel) const podcasts = await findAll({ include: { @@ -556,82 +520,13 @@ module.exports = { // Set podcast count for later comparison data.podcastCount = podcastCountFromDatabase } else { - const bookCountFromDatabase = await Database.bookModel.count({ - include: { - model: Database.libraryItemModel, - attributes: [], - where: { - libraryId: libraryId - } - } - }) - - const seriesCountFromDatabase = await Database.seriesModel.count({ - where: { - libraryId: libraryId - } - }) - - const authorCountFromDatabase = await Database.authorModel.count({ - where: { - libraryId: libraryId - } - }) - - // To reduce the cold-start load time, first check if any library items, series, - // or authors have an "updatedAt" timestamp since the last time the filter - // data was loaded. If so, we can skip loading all of the data. - // Because many items could change, just check the count of items instead - // of actually loading the data twice - - const changedBooks = await Database.bookModel.count({ - include: { - model: Database.libraryItemModel, - attributes: [], - where: { - libraryId: libraryId, - updatedAt: { - [Sequelize.Op.gt]: new Date(lastLoadedAt) - } - } - }, - where: { - updatedAt: { - [Sequelize.Op.gt]: new Date(lastLoadedAt) - } - }, - limit: 1 - }) - - const changedSeries = await Database.seriesModel.count({ - where: { - libraryId: libraryId, - updatedAt: { - [Sequelize.Op.gt]: new Date(lastLoadedAt) - } - }, - limit: 1 - }) - - const changedAuthors = await Database.authorModel.count({ - where: { - libraryId: libraryId, - updatedAt: { - [Sequelize.Op.gt]: new Date(lastLoadedAt) - } - }, - limit: 1 - }) - - if (changedBooks + changedSeries + changedAuthors === 0) { - // If nothing has changed, check if the number of authors, series, and books - // matches the prior check before updating cache creation time - if (bookCountFromDatabase === Database.libraryFilterData[libraryId]?.bookCount && seriesCountFromDatabase === Database.libraryFilterData[libraryId]?.seriesCount && authorCountFromDatabase === Database.libraryFilterData[libraryId].authorCount) { - Logger.debug(`Filter data for ${libraryId} has not changed, returning cached data and updating cache time after ${((Date.now() - start) / 1000).toFixed(2)}s`) - Database.libraryFilterData[libraryId].loadedAt = Date.now() - return cachedFilterData - } - } + const [bookCountFromDatabase, seriesCountFromDatabase, authorCountFromDatabase] = await Promise.all([ + Database.bookModel.count({ + include: { model: Database.libraryItemModel, attributes: [], where: { libraryId } } + }), + Database.seriesModel.count({ where: { libraryId } }), + Database.authorModel.count({ where: { libraryId } }) + ]) // Store the counts for later comparison data.bookCount = bookCountFromDatabase @@ -694,10 +589,9 @@ module.exports = { data.publishers = naturalSort([...data.publishers]).asc() data.publishedDecades = naturalSort([...data.publishedDecades]).asc() data.languages = naturalSort([...data.languages]).asc() - data.loadedAt = Date.now() - Database.libraryFilterData[libraryId] = data + Database.setLibraryFilterCache(libraryId, data) Logger.debug(`Loaded filterdata in ${((Date.now() - start) / 1000).toFixed(2)}s`) - return data + return Database.libraryFilterData[libraryId] } } diff --git a/server/utils/queries/libraryItemsBookFilters.js b/server/utils/queries/libraryItemsBookFilters.js index c0ec1e277..219a7ea60 100644 --- a/server/utils/queries/libraryItemsBookFilters.js +++ b/server/utils/queries/libraryItemsBookFilters.js @@ -195,7 +195,7 @@ function getCollapsedSeriesSortContext(payload = {}) { function buildCollapsedSeriesBaseQuery(libraryId, seriesId, userPermissionBookWhere, sortContext) { const queryGenerator = Database.sequelize.dialect.queryGenerator - const sequenceSortExpr = dialectHelpers.getSafeSequenceCast(Database.getDialect(), '"bookSeries"."sequence"') + const sequenceSortExpr = 'CAST("bookSeries"."sequence" AS FLOAT)' const plainSortExpr = `LOWER("libraryItem"."${sortContext.titleColumn}")` const baseOptions = { attributes: [ @@ -1196,7 +1196,7 @@ module.exports = { const direction = payload.sortDesc ? 'DESC' : 'ASC' const sortOrder = collapsedSortBy === 'sequence' ? [ - [Sequelize.literal(`${dialectHelpers.getSafeSequenceCast(Database.getDialect(), '"bookSeries"."sequence"')} ${direction} NULLS LAST`)], + [Sequelize.literal(`CAST("bookSeries"."sequence" AS FLOAT) ${direction} NULLS LAST`)], ...this.getOrder('media.metadata.title', payload.sortDesc, false) ] : this.getOrder(collapsedSortBy, payload.sortDesc, false) diff --git a/test/server/controllers/LibraryController.largeLibraryBrowse.test.js b/test/server/controllers/LibraryController.largeLibraryBrowse.test.js index 968d651ff..af0a9b900 100644 --- a/test/server/controllers/LibraryController.largeLibraryBrowse.test.js +++ b/test/server/controllers/LibraryController.largeLibraryBrowse.test.js @@ -12,6 +12,20 @@ const libraryItemsPodcastFilters = require('../../../server/utils/queries/librar const libraryHelpers = require('../../../server/utils/libraryHelpers') describe('LibraryController large-library browse contract', () => { + const stubBookFilterDataRebuild = () => { + sinon.stub(Database.bookModel, 'count').resolves(0) + sinon.stub(Database.seriesModel, 'count').resolves(0) + sinon.stub(Database.authorModel, 'count').resolves(0) + sinon.stub(Database.bookModel, 'findAll').resolves([]) + sinon.stub(Database.seriesModel, 'findAll').resolves([]) + sinon.stub(Database.authorModel, 'findAll').resolves([]) + sinon.stub(Database.libraryItemModel, 'count').resolves(0) + + if (Database.genreModel) sinon.stub(Database.genreModel, 'findAll').resolves([]) + if (Database.tagModel) sinon.stub(Database.tagModel, 'findAll').resolves([]) + if (Database.narratorModel) sinon.stub(Database.narratorModel, 'findAll').resolves([]) + } + const buildBookPredicateSql = (where, replacements = {}) => { const query = Database.sequelize.dialect.queryGenerator.selectQuery( Database.bookModel.getTableName(), @@ -719,4 +733,53 @@ describe('LibraryController large-library browse contract', () => { expect(permissionSql).to.include('NOT EXISTS') expect(permissionSql).to.not.include('count(*)') }) + + it('returns a fresh filter cache hit without querying the database again', async () => { + Database.setLibraryFilterCache('lib_1', { authors: [] }, 60_000) + const countStub = sinon.stub(Database.bookModel, 'count') + + const data = await libraryFilters.getFilterData('book', 'lib_1') + + expect(data.authors).to.deep.equal([]) + expect(countStub.called).to.equal(false) + }) + + it('rebuilds filter data after the TTL expires', async () => { + Database.libraryFilterData.lib_1 = { authors: [], loadedAt: 1, expiresAt: 2 } + stubBookFilterDataRebuild() + + await libraryFilters.getFilterData('book', 'lib_1') + + expect(Database.bookModel.count.called).to.equal(true) + }) + + it('invalidates filter cache when a library write path changes library content', async () => { + Database.setLibraryFilterCache('lib_1', { authors: [] }, 60_000) + sinon.stub(Database.libraryItemModel, 'findAll').resolves([ + { + id: 'li_1', + mediaId: 'book_1', + media: { + bookAuthors: [], + bookSeries: [] + } + } + ]) + + const req = { + library: { id: 'lib_1', name: 'Lib', isPodcast: false }, + user: { isAdminOrUp: true }, + query: {} + } + const res = { json: sinon.spy(), sendStatus: sinon.spy() } + const controllerContext = { + handleDeleteLibraryItem: sinon.stub().resolves(), + checkRemoveAuthorsWithNoBooks: sinon.stub().resolves(), + checkRemoveEmptySeries: sinon.stub().resolves() + } + + await LibraryController.removeLibraryItemsWithIssues.call(controllerContext, req, res) + + expect(Database.libraryFilterData.lib_1).to.equal(undefined) + }) })