fix(core): handle parameter limits in files metadata deletion

[gitedmond]

• Resolves: [Bug]: files:scan creates oversized files_metadata DELETE query and causes severe resource usage #62325

Summary

dropMetadataForFiles() was already intended to split file IDs into chunks using IQueryBuilder::MAX_IN_PARAMETERS, but the DELETE query mistakenly bound the original $fileIds array instead of the current $chunk. As a result, every query still contained the full ID list and could exceed database parameter limits.

This PR passes $chunk to each query and adds regression coverage that verifies the exact chunks sent to the query builder.

Testing

Added a regression test using more than two parameter-limit batches. It verifies that every ID is bound exactly once, in order, and no query receives more than MAX_IN_PARAMETERS IDs.

AI (if applicable)

• The content of this PR was partly or fully generated using AI

[github-actions]

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

[come-nc]

Hello,

Please switch back to using the const MAX_IN_PARAMETERS, we do not want to have a magic number 500 in there.

Nice catch on the wrong var being used.

Regarding the transaction I lack knowledge to judge in it makes sense here. Is a partial metadata deletion an issue?

[gitedmond]

Hello,

Please switch back to using the const MAX_IN_PARAMETERS, we do not want to have a magic number 500 in there.

Nice catch on the wrong var being used.

Regarding the transaction I lack knowledge to judge in it makes sense here. Is a partial metadata deletion an issue?

You're right: the transaction is not needed for this fix, and it would not make metadata and index deletion atomic because those happen in separate calls. I've removed it. The PR now only binds each existing MAX_IN_PARAMETERS chunk instead of the full $fileIds array, with a regression test that verifies the exact chunks passed to the query builder.

[dispather]

Confirming this from a production instance (Nextcloud 34.0.0, MariaDB 10.6) — 2368 occurrences across 7 days.

Two notes that may help review:

1. The identical bug in the sibling call site, IndexRequestService::dropIndexForFiles(), was fixed separately by 482c306 (fix(metadata): bind chunked file ids #61857) and backported in [stable34] fix(metadata): bind chunked file ids #62771 for 34.0.3. This PR is the remaining half — on my instance the two sites logged exactly 1184 lines each, so fixing only one leaves half the load.

2. The trigger is not limited to files:scan. It also fires from cron via Cache::removeChildren → CacheEntriesRemovedEvent → MetadataDelete, i.e. any bulk deletion of a directory with more than 1000 children (trashbin expiry, in my case).

Detailed measurements and the runtime evidence that these QueryExceptions are logged but never thrown are in #62325 (comment).

[come-nc]

👍

[welcome]

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

[dispather]

Thanks for merging this.

Would it be possible to also backport it to stable34? The sibling call site IndexRequestService::dropIndexForFiles() was fixed in #61857 and backported in #62771, so stable34 currently carries exactly half of the fix:

branch	file	createNamedParameter argument
stable34	IndexRequestService.php	$chunk ✅ (via #62771, milestone 34.0.3)
stable34	MetadataRequestService.php	still $fileIds ❌
master	MetadataRequestService.php	$chunk ✅ (this PR, milestone 35)

On the production instance I reported in #62325, the two call sites logged exactly 1184 lines each out of 2368 over 7 days. So 34.0.3 will remove the index half while the metadata half keeps repeating the same full-list DELETE once per chunk — the part #62325 describes as severe resource usage — for the whole 34 series.

I don't have write access here, so a /backport to stable34 from me would just be ignored. If there is no reason to keep this 35-only, could someone with the permission trigger the bot or add the backport label? @artonge

Happy to test a stable34 backport on 34.0.2 with the reproducer from #62325 (bulk-deleting a directory with more than 1000 children) if that is useful.