From 0bdb28aa14d7dc1ebd284fde25559b6a4617417f Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 16 Sep 2026 14:59:35 +0200 Subject: [PATCH 1/2] fix: stop counting the whole file cache on every queue delete request `QueueController::setInitialIndexCompletion()` runs on every `DELETE /queues/documents` request from the backend. On instances where it falls through to the file counting path, every one of those requests walks all mounts of the instance via `StorageService::countFiles()`. With several hundred group folders that is minutes of `SELECT COUNT(*) FROM oc_filecache` per request, several of them concurrently, and it never stops. Four issues conspire here: * `last_enqueued_db_id` is only written by `StorageCrawlJob` and backfilled by `Version005004000Date20260302135634` while the referenced file is still queued. Instances that finished their crawl before the value existed never get it, so the cheap completion check is skipped and the counting fallback runs forever. Seed the value from the current queue head instead, and treat an empty queue with no pending crawl jobs as a completed initial index. * Even after counting, `withinThreshold()` compared the fraction of files that are *already indexed* against the threshold instead of the fraction still queued, so completion was never detected and the fallback could not terminate. 557 queued of 100000 eligible evaluated to `0.994 < 0.02`. * `withinThreshold()` divided by zero when nothing is eligible for indexing. `DivisionByZeroError` is an `Error`, so neither the inner `\OCP\DB\Exception` catch nor the caller's `\Exception` catch stopped it from turning into a 500 on the endpoint the backend needs to drain the queue. Guard the division and widen the caller's catch to `\Throwable`. * Keep the counting fallback as a last resort but throttle it to once an hour, so it can never again be driven by request volume. Also count home mounts from their `files/` folder, like the crawl in `getFilesInMount()` does, rather than from the storage root. Counting from the root included `uploads/`, `cache/` and `files_encryption/`, inflating `eligible_files_count` against files that are never queued for indexing. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr --- lib/Controller/QueueController.php | 50 ++++++++++++++++++++++++------ lib/Db/QueueMapper.php | 18 +++++++++++ lib/Service/StorageService.php | 6 +++- 3 files changed, 64 insertions(+), 10 deletions(-) diff --git a/lib/Controller/QueueController.php b/lib/Controller/QueueController.php index 59ae1521..b8e492f9 100644 --- a/lib/Controller/QueueController.php +++ b/lib/Controller/QueueController.php @@ -40,6 +40,7 @@ class QueueController extends OCSController { private const INDEX_COMPLETION_THRESHOLD = 0.02; // 2% + private const INDEX_COMPLETION_CHECK_INTERVAL = 60 * 60; // 1 hour public function __construct( string $appName, @@ -182,7 +183,7 @@ public function deleteDocumentsQueueItems(IDBConnection $db, QueueMapper $queueM try { $this->setInitialIndexCompletion(); - } catch (\Exception $e) { + } catch (\Throwable $e) { $this->logger->warning('Could not check for initial index completion', ['exception' => $e]); } @@ -356,21 +357,48 @@ private function setInitialIndexCompletion(): void { try { $lastEnqueuedDbId = $this->appConfig->getAppValueInt('last_enqueued_db_id', -1, lazy: true); - if ($lastEnqueuedDbId !== -1) { - $initiallyQueuedFilesExist = $this->queueMapper->existsQueueItemsUpToDbId($lastEnqueuedDbId); - if ($initiallyQueuedFilesExist) { - $this->logger->debug('Initially queued files still in the queue, intial indexing has not completed.'); + if ($lastEnqueuedDbId === -1) { + // Instances that completed their crawl before this value was introduced never got + // it set, and the migration only backfills it while the referenced file is still + // queued. Seed it from the queue instead of falling through to counting every file + // in the file cache on every single request. + $maxQueuedDbId = $this->queueMapper->getMaxId(); + if ($maxQueuedDbId === null) { + // no crawl jobs left and nothing queued, so everything has been indexed once + $this->logger->info('Initial index completion detected, setting last indexed time'); + $this->appConfig->setAppValueInt('last_indexed_time', $this->timeFactory->getTime(), lazy: true); return; } - $this->logger->info('Initial index completion detected, setting last indexed time'); - $this->appConfig->setAppValueInt('last_indexed_time', $this->timeFactory->getTime(), lazy: true); + $this->appConfig->setAppValueInt('last_enqueued_db_id', $maxQueuedDbId, lazy: true); + $lastEnqueuedDbId = $maxQueuedDbId; + } + + $initiallyQueuedFilesExist = $this->queueMapper->existsQueueItemsUpToDbId($lastEnqueuedDbId); + if ($initiallyQueuedFilesExist) { + $this->logger->debug('Initially queued files still in the queue, intial indexing has not completed.'); return; } + $this->logger->info('Initial index completion detected, setting last indexed time'); + $this->appConfig->setAppValueInt('last_indexed_time', $this->timeFactory->getTime(), lazy: true); + return; } catch (\Exception $e) { $this->logger->warning('Could not get last enqueued file\'s DB id', ['exception' => $e]); } - // last enqueued file's ID could not be retrieved, falling back to file counting method + // last enqueued file's ID could not be retrieved, falling back to file counting method. + // countFiles() walks every mount of the instance, so it must never run per request. + try { + $now = $this->timeFactory->getTime(); + $lastCheck = $this->appConfig->getAppValueInt('last_index_completion_check', 0, lazy: true); + if ($now - $lastCheck < self::INDEX_COMPLETION_CHECK_INTERVAL) { + return; + } + $this->appConfig->setAppValueInt('last_index_completion_check', $now, lazy: true); + } catch (\Exception $e) { + $this->logger->warning('Could not throttle the initial index completion check', ['exception' => $e]); + return; + } + try { $queuedNewFilesCount = $this->queueService->countNewFiles(); $eligibleFilesCount = $this->storageService->countFiles(); @@ -396,6 +424,10 @@ private function setInitialIndexCompletion(): void { } private static function withinThreshold(int $current, int $total, float $threshold = self::INDEX_COMPLETION_THRESHOLD): bool { - return ((float)($total - $current) / (float)$total) < $threshold; + if ($total <= 0) { + // nothing is eligible for indexing, so there is nothing left to wait for + return true; + } + return ((float)$current / (float)$total) < $threshold; } } diff --git a/lib/Db/QueueMapper.php b/lib/Db/QueueMapper.php index 2af16ce5..4d18314e 100644 --- a/lib/Db/QueueMapper.php +++ b/lib/Db/QueueMapper.php @@ -72,6 +72,24 @@ public function removeFromQueue(array $ids): void { } } + /** + * @return int|null The highest queue item id, or null if the queue is empty + * @throws \OCP\DB\Exception + */ + public function getMaxId(): ?int { + $qb = $this->db->getQueryBuilder(); + $qb->select('id') + ->from($this->getTableName()) + ->orderBy('id', 'DESC') + ->setMaxResults(1); + + $result = $qb->executeQuery(); + $id = $result->fetchOne(); + $result->closeCursor(); + + return ($id === false || $id === null) ? null : (int)$id; + } + /** * @param int $dbId * @return bool diff --git a/lib/Service/StorageService.php b/lib/Service/StorageService.php index b9372591..8fb86bf7 100644 --- a/lib/Service/StorageService.php +++ b/lib/Service/StorageService.php @@ -55,7 +55,11 @@ public function __construct( public function countFiles(): int { $totalCount = 0; foreach ($this->getMounts() as $mount) { - $totalCount += $this->countFilesInMount($mount['storage_id'], $mount['root_id']); + // use the overridden root so home mounts are counted from their `files/` folder, the + // same root the crawl in getFilesInMount() uses; counting from the storage root would + // also include `uploads/`, `cache/`, `files_encryption/` and friends, which are never + // queued for indexing + $totalCount += $this->countFilesInMount($mount['storage_id'], $mount['overridden_root'] ?? $mount['root_id']); } return $totalCount; } From e9068bd36c93e786376385d55ab12b41205981eb Mon Sep 17 00:00:00 2001 From: Marcel Klehr Date: Wed, 16 Sep 2026 15:01:06 +0200 Subject: [PATCH 2/2] perf(StorageService): make the per-mount file count use the path index `countFilesInMount()` took 1-7 seconds per mount on a 1.9M row file cache, so a full count over a few hundred group folders ran for tens of minutes. The dominant cost is the LIKE pattern. `_` and `%` are LIKE wildcards, and a group folder root path is `__groupfolders/`, so the unescaped pattern `__groupfolders/28/%` has no literal prefix for the planner to work with. fs_storage_path_prefix cannot be used for a range scan and every count falls back to filtering the whole storage. Escape the prefix with `escapeLikeParameter()`, which both restores the index range scan and stops the pattern from matching unrelated paths. The legacy crawl path in `getFilesInMountOld()` had the same unescaped pattern. The end-to-end-encryption check was a correlated scalar subquery on the parent row, re-executed for every candidate row. `fileid` is the primary key, so an inner join on `filecache.parent` selects the same rows (a missing parent excluded the row before, and excludes it now) while letting the planner resolve it as a primary key lookup. Also: * Fetch the mount root path with a plain `SELECT path`, instead of `selectFileCache()` pulling every file cache column plus the metadata join to read a single field. * Drop the duplicated `filecache.storage` predicate. * Drop the `files_versions/` and `files_trashbin/` exclusions. Mounts are now counted from their overridden root, so neither can match a home mount's `files/` prefix, and the crawl in `getFilesInMount()` applies no such filter either, so leaving them made the count disagree with what is indexed. * Cast the count before returning it. `fetchOne()` yields a string, which under `strict_types=1` would be a TypeError against the `int` return type. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr --- lib/Service/StorageService.php | 78 ++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 37 deletions(-) diff --git a/lib/Service/StorageService.php b/lib/Service/StorageService.php index 8fb86bf7..c781f57a 100644 --- a/lib/Service/StorageService.php +++ b/lib/Service/StorageService.php @@ -70,50 +70,33 @@ public function countFiles(): int { * @return int */ public function countFilesInMount(int $storageId, int $rootId): int { - $qb = $this->getCacheQueryBuilder(); - try { - $qb->selectFileCache(); - $qb->andWhere($qb->expr()->eq('filecache.fileid', $qb->createNamedParameter($rootId, IQueryBuilder::PARAM_INT))); - $result = $qb->executeQuery(); - /** @var array{path:string}|false $root */ - $root = $result->fetch(); - $result->closeCursor(); - } catch (DBException $e) { - $this->logger->error('Could not fetch storage root', ['exception' => $e]); - return 0; - } - - if ($root === false) { + $rootPath = $this->getPathOfFileId($rootId); + if ($rootPath === null) { $this->logger->error('Could not fetch storage root'); return 0; } $mimeTypes = array_map(fn ($mimeType) => $this->mimeTypes->getId($mimeType), Application::MIMETYPES); + $path = $rootPath === '' ? '' : $rootPath . '/'; + // `_` and `%` are LIKE wildcards. Group folder roots start with two underscores + // (`__groupfolders/28/`), so an unescaped pattern has no literal prefix at all and + // fs_storage_path_prefix degenerates into a scan of the entire storage. + $pathPattern = $this->db->escapeLikeParameter($path) . '%'; - $qb = $this->getCacheQueryBuilder(); + $qb = $this->db->getQueryBuilder(); try { - $path = $root['path'] === '' ? '' : $root['path'] . '/'; - $qb->select($qb->func()->count('*')) - ->from('filecache', 'filecache'); - - // End to end encrypted files are descendants of a folder with encrypted=1 - // Use a subquery to check the `encrypted` status of the parent folder - $subQuery = $this->getCacheQueryBuilder()->select('p.encrypted') - ->from('filecache', 'p') - ->andWhere($qb->expr()->eq('p.fileid', 'filecache.parent')) - ->getSQL(); - - $qb->andWhere( - $qb->expr()->eq($qb->createFunction(sprintf('(%s)', $subQuery)), $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)) - ); - $qb->andWhere($qb->expr()->eq('filecache.storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))); - $qb - ->andWhere($qb->expr()->like('filecache.path', $qb->createNamedParameter($path . '%'))) - ->andWhere($qb->expr()->notLike('filecache.path', $qb->createNamedParameter('files_versions/%'))) - ->andWhere($qb->expr()->notLike('filecache.path', $qb->createNamedParameter('files_trashbin/%'))) - ->andWhere($qb->expr()->eq('filecache.storage', $qb->createNamedParameter($storageId))) + ->from('filecache', 'filecache') + // End to end encrypted files are descendants of a folder with encrypted=1. + // `fileid` is the primary key, so joining the parent row matches at most one row + // and is equivalent to the correlated subquery this replaces, except that the + // planner can satisfy it with a primary key lookup instead of re-running a + // subquery for every candidate row. + ->innerJoin('filecache', 'filecache', 'p', $qb->expr()->eq('p.fileid', 'filecache.parent')) + ->where($qb->expr()->eq('p.encrypted', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->eq('filecache.storage', $qb->createNamedParameter($storageId, IQueryBuilder::PARAM_INT))) + ->andWhere($qb->expr()->like('filecache.path', $qb->createNamedParameter($pathPattern))) ->andWhere($qb->expr()->in('filecache.mimetype', $qb->createNamedParameter($mimeTypes, IQueryBuilder::PARAM_INT_ARRAY))) ->andWhere($qb->expr()->lte('filecache.size', $qb->createNamedParameter(Application::CC_MAX_SIZE, IQueryBuilder::PARAM_INT))) ->andWhere($qb->expr()->gt('filecache.size', $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT))); @@ -129,7 +112,28 @@ public function countFilesInMount(int $storageId, int $rootId): int { $this->logger->warning('Could not count files in mount: storage=' . $storageId . ' root=' . $rootId); return 0; } - return $countInMount; + return (int)$countInMount; + } + + /** + * @param int $fileId + * @return string|null The file cache path, or null if the row does not exist or is unreadable + */ + private function getPathOfFileId(int $fileId): ?string { + $qb = $this->db->getQueryBuilder(); + try { + $qb->select('path') + ->from('filecache') + ->where($qb->expr()->eq('fileid', $qb->createNamedParameter($fileId, IQueryBuilder::PARAM_INT))); + $result = $qb->executeQuery(); + $path = $result->fetchOne(); + $result->closeCursor(); + } catch (DBException $e) { + $this->logger->error('Could not fetch path of file ' . $fileId, ['exception' => $e]); + return null; + } + + return $path === false ? null : (string)$path; } private function isFileAccessAvailable(): bool { @@ -273,7 +277,7 @@ private function getFilesInMountOld(int $storageId, int $rootId, int $lastFileId $qb->expr()->eq($qb->createFunction(sprintf('(%s)', $subQuery)), $qb->createNamedParameter(0, IQueryBuilder::PARAM_INT)) ); $qb - ->andWhere($qb->expr()->like('filecache.path', $qb->createNamedParameter($path . '%'))) + ->andWhere($qb->expr()->like('filecache.path', $qb->createNamedParameter($this->db->escapeLikeParameter($path) . '%'))) ->andWhere($qb->expr()->eq('filecache.storage', $qb->createNamedParameter($storageId))) ->andWhere($qb->expr()->gt('filecache.fileid', $qb->createNamedParameter($lastFileId))) ->andWhere($qb->expr()->in('filecache.mimetype', $qb->createNamedParameter($mimeTypes, IQueryBuilder::PARAM_INT_ARRAY)));