Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[Bug]: File watcher thread slows down whole application #7873

Open
5 of 8 tasks
BBArikL opened this issue Feb 17, 2025 · 4 comments
Open
5 of 8 tasks

[Bug]: File watcher thread slows down whole application #7873

BBArikL opened this issue Feb 17, 2025 · 4 comments

Comments

@BBArikL
Copy link

BBArikL commented Feb 17, 2025

⚠️ Before submitting, please verify the following: ⚠️

Bug description

The file watcher thread gets over-pressured by spurious notifications on Linux.

Running on Arch Linux, present in the most recent Nextcloud desktop release (tried even git version), with an account that has 220k+ files (Server ZFS and client ext4).

Nextcloud version 3.16.50daily
Git revision 120505be92d1aa9ffe31a4fd5e4d187b10bf4602
Using Qt 6.8.2, built against Qt 6.8.2
Using Qt platform plugin 'wayland'
Using 'OpenSSL 3.4.1 11 Feb 2025'
Running on Arch Linux, x86_64

I do not know if it could also be a inotify problem (from the code it seems it might be, there are no mentions of Windows but mentions of MacOS not having that issue), and since it even is acknowledged by the person who wrote the code, there is counter-measures to discard a file that has not changed. Which is great! But since it does check, it also writes thousands upon thousands of log lines, running the operation to a slog. To see if it would help, I changed the qcInfo call to a qcDebug, recompiled in Release mode and the application went from being unresponsive for hours to... being unresponsive for a few minutes. Which is a major improvement!

But this does raise an important question: why isn't that file watching thread being run in a separate thread as the main one? If it is so computationally intensive task, running it in the background and maybe writing to the log files the number of spurious notification met would keep the main application responsive and the experience much more smooth. I do understand the value of seeing per-file spurious notification discard, but this tie into major issues where users have hundreds of MB of logs written on every sync. Being able to set the debug level to write to logs in the application could also be a solution to this situation.

The offending code : slotWatchedPathChanged and more specifically this line that prints heavily on the log file

I hope to see that line changed from qcInfo to qcDebug in the next release so I can finally use VFS on my laptop. In the long-term, I hope that this process can be multi-threaded to avoid the main applet being unresponsive during this operation.

Anyway, thanks for making such a great self-hosting cloud application!

Steps to reproduce

  1. Start Sync
  2. Tail the created logs
  3. See thousands of lines of "Ignoring spurious notification for file XYZ"
  4. The nextcloud client is unresponsive for how long it takes it to both discard the notifications and write the logs

Expected behavior

  1. Start sync
  2. See "Verifying ## possible changed files..."
  3. The nextcloud client should stay responsive
  4. See in the info logs "Ignored ## spurious notifications" when the sync is finished

OR

  1. Being able to raise log level to warning and above from the application

Which files are affected by this bug

src/gui/folder.cpp

Operating system

Linux

Which version of the operating system you are running.

Linux 6.13.2-arch1-1

Package

Compiled it myself

Nextcloud Server version

30.0.4

Nextcloud Desktop Client version

3.16-rc1

Is this bug present after an update or on a fresh install?

Fresh desktop client install

Are you using the Nextcloud Server Encryption module?

Encryption is Disabled

Are you using an external user-backend?

  • Default internal user-backend
  • LDAP/ Active Directory
  • SSO - SAML
  • Other

Nextcloud Server logs

Additional info

No response

@BBArikL
Copy link
Author

BBArikL commented Feb 17, 2025

Related bugs : #7729 and #7861

camilasan added a commit that referenced this issue Feb 18, 2025
Reported at #7873.

Signed-off-by: Camila Ayres <hello@camilasan.com>
@camilasan
Copy link
Member

The offending code : slotWatchedPathChanged and more specifically this line that prints heavily on the log file

#7880

backportbot bot pushed a commit that referenced this issue Feb 18, 2025
Reported at #7873.

Signed-off-by: Camila Ayres <hello@camilasan.com>
@BBArikL
Copy link
Author

BBArikL commented Feb 18, 2025

Love to see such a fast response! While this will help reduce log files size and speed up the sync progress, the main issue will remain: A single thread needs to react to all the changes, while these can be of multiple thousand of files on each startup of the application. From looking at the source code, this feels quite daunting for a new contributor such as myself, but I would be interested to contribute code if I am pointed to the right spot to start.

nilsding pushed a commit that referenced this issue Feb 19, 2025
Reported at #7873.

Signed-off-by: Camila Ayres <hello@camilasan.com>
@camilasan
Copy link
Member

the main issue will remain: A single thread needs to react to all the changes, while these can be of multiple thousand of files on each startup of the application.

We understood it. Thanks for the report!

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants