Fixes #279 - #280
Open
marcelklehr wants to merge 2 commits into
Open
Fixes #279#280marcelklehr wants to merge 2 commits into
marcelklehr wants to merge 2 commits into
Conversation
`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 <mklehr@gmx.net>
`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/<id>`, 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 <mklehr@gmx.net>
marcelklehr
force-pushed
the
fix/indexing-correctness-perf
branch
from
September 16, 2026 15:11
523d823 to
e9068bd
Compare
Member
Author
|
CI failure is preexisting, it seems |
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
2 participants
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.
QueueController::setInitialIndexCompletion()runs on everyDELETE /queues/documentsrequest from the backend. On instances where itfalls through to the file counting path, every one of those requests walks
all mounts of the instance via
StorageService::countFiles(). With severalhundred group folders that is minutes of
SELECT COUNT(*) FROM oc_filecacheper request, several of them concurrently, and it never stops.
Four issues conspire here:
last_enqueued_db_idis only written byStorageCrawlJoband backfilled byVersion005004000Date20260302135634while the referenced file is stillqueued. 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 thatare 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.DivisionByZeroErroris anError, so neither the inner\OCP\DB\Exceptioncatch nor the caller's
\Exceptioncatch stopped it from turning into a 500on 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 ingetFilesInMount()does, rather than from the storage root. Counting from theroot included
uploads/,cache/andfiles_encryption/, inflatingeligible_files_countagainst files that are never queued for indexing.🤖 AI (if applicable)