Skip to content

Reducing queries in ScanFiles.php#40019

Closed
FedericoHeichou wants to merge 3 commits into
nextcloud:masterfrom
FedericoHeichou:master
Closed

Reducing queries in ScanFiles.php#40019
FedericoHeichou wants to merge 3 commits into
nextcloud:masterfrom
FedericoHeichou:master

Conversation

@FedericoHeichou

Copy link
Copy Markdown
Contributor

Summary

By changing the approach to the problem, it will guarantee that all users are scanned and it will only perform 2 (or 3) fast queries without performing joins.

TODO

  • ...

Checklist

@szaimen szaimen added enhancement 3. to review Waiting for reviews technical debt 🧱 🤔🚀 labels Aug 23, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Aug 23, 2023
@szaimen szaimen requested review from a team, ArtificialOwl, CarlSchwan, icewind1991 and nfebe and removed request for a team August 23, 2023 14:31
Comment thread apps/files/lib/BackgroundJob/ScanFiles.php Fixed
Comment thread apps/files/lib/BackgroundJob/ScanFiles.php Fixed

if ($lastUser === $user) {
$this->logger->warning("User $user still has unscanned files after running background scan, background scan might be stopped prematurely");
if ($this->config->getSystemValue('loglevel') > ILogger::WARN) {

Check notice

Code scanning / Psalm

DeprecatedClass

Class OCP\ILogger is deprecated

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking the code there are not good alternatives.
There is a Psr\Log\LogLevel but it is a string, not a int.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine, the exception just need to be added in ./build/psalm-baseline.xml

@FedericoHeichou

Copy link
Copy Markdown
Contributor Author

Do you think the Samba's failure is my fault? I think it just not sleeped enough or something like that. https://drone.nextcloud.com/nextcloud/server/39214/7/3

A question, I see it usually fails the "Cypress" test in every PR. I'm doing something wrong?

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
Signed-off-by: FedericoHeichou <federicoheichou@gmail.com>
Signed-off-by: FedericoHeichou <federicoheichou@gmail.com>
Signed-off-by: FedericoHeichou <federicoheichou@gmail.com>
This was referenced Sep 25, 2025
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
This was referenced Jan 7, 2026
This was referenced Jan 14, 2026
This was referenced Jan 29, 2026
This was referenced Feb 11, 2026
@blizzz blizzz modified the milestones: Nextcloud 33, Nextcloud 34 Feb 16, 2026
@DorraJaouad DorraJaouad requested a review from a team as a code owner April 23, 2026 09:41
This was referenced Jun 5, 2026
@susnux susnux added the community pull requests from community label Jun 9, 2026
@icewind1991

Copy link
Copy Markdown
Member

I'm unsure about the scalability of this approach. pre-fetching the data into memory will become unfeasible on instances with very large numbers of users or with many mounts per user (e.g. with heavy sharing).

The current approach is in large parts the way it is for being fully incremental.

@susnux

susnux commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Hello 👋
currently here have been no activity in the last years.

Moreover this might resolve the query problem but will run into memory issues on large instances, so at least some kind of chunking would be required.

If you think this is still actual feel free to reopen the pull request and resolve the conflicts and adjust the approach to the review comments so we can incorporate it into Nextcloud.
Thank you very much for your contribution!

@susnux susnux closed this Jun 9, 2026
@FedericoHeichou

Copy link
Copy Markdown
Contributor Author

I'm using this patch from years and it fixed all the problems in my instances with low number of users but hug number of files. But I understand your point of view

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2. developing Work in progress community pull requests from community enhancement feature: filesystem performance 🚀 stale Ticket or PR with no recent activity technical debt 🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ScanFiles never performed and slow with big tables