Fixes #279

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;
 	}
 }

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

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;
 	}
@@ -66,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)));
@@ -125,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 {
@@ -269,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)));