Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

We need limit the amount of previews generated at once when listing images in Files #15075

Closed
ariselseng opened this issue Apr 11, 2019 · 15 comments · Fixed by #18210
Closed
Labels
4. to release Ready to be released and/or waiting for tests to finish 25-feedback bug feature: previews and thumbnails
Milestone

Comments

@ariselseng
Copy link
Member

Background: ariselseng/camerarawpreviews#31

User has encryption enabled, lists files in the Files app in details mode, scrolls to the bottom, and triggers hundreds of previews being generated. The problem gets even worse due to encryption + big raw files (enabled previews with my app). Preview generator is not the answer due to encryption.

We should make sure not more than x amount of images are being generated preview for at the same time.

@ariselseng ariselseng added bug 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Apr 11, 2019
@violoncelloCH violoncelloCH added this to the Nextcloud 17 milestone Apr 18, 2019
@phpbg
Copy link
Contributor

phpbg commented Sep 21, 2019

it would be interesting if you can test this pull request to see how it improves (or not) : #17028

Indeed, when you browse the files for the first time, two previews are generated for each picture.
I did this patch to generate only the required preview, and saw very interesting results.

I'm being told by nextcloud guys that those two previews are used to improve speed when generating previews from big raw files (they generate a big preview that should be smaller than your raw file, so it should be faster next times).

There are no measurements for this, but IMO generating two previews has a very bad impact on first load (what you are doing here).

Please test it and report back...

@phpbg
Copy link
Contributor

phpbg commented Sep 21, 2019

Also, as a short term solution I reduced my php-fpm pool size to prevent overload

@kesselb
Copy link
Contributor

kesselb commented Sep 21, 2019

We should make sure not more than x amount of images are being generated preview for at the same time.

I agree we should do something like this.

@phpbg
Copy link
Contributor

phpbg commented Sep 21, 2019

We should make sure not more than x amount of images are being generated preview for at the same time.

I agree we should do something like this.

Do you mean on the server, globally, or on the client side (javasript) ?

@rullzer rullzer modified the milestones: Nextcloud 17.0.4, Nextcloud 17.0.5 Mar 11, 2020
@rullzer rullzer modified the milestones: Nextcloud 17.0.5, Nextcloud 17.0.6 Mar 23, 2020
@MorrisJobke MorrisJobke removed this from the Nextcloud 17.0.9 milestone Aug 20, 2020
@skjnldsv skjnldsv added feature: previews and thumbnails and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Sep 9, 2020
@skjnldsv skjnldsv added the 1. to develop Accepted and waiting to be taken care of label Sep 9, 2020
@J0WI J0WI mentioned this issue Aug 4, 2021
@szaimen
Copy link
Contributor

szaimen commented Jan 9, 2023

Hi, please update to 24.0.8 or better 25.0.2 and report back if it fixes the issue. Thank you!

@szaimen szaimen added needs info 0. Needs triage Pending check for reproducibility or if it fits our roadmap and removed 1. to develop Accepted and waiting to be taken care of labels Jan 9, 2023
@lukasmu
Copy link

lukasmu commented Jan 16, 2023

@szaimen 25.0.2 does not fix the issue yet. It seems like #18210 is not included in the 25.0.2 release?

@szaimen
Copy link
Contributor

szaimen commented Jan 16, 2023

It seems like #18210 is not included in the 25.0.2 release?

yes indeed. it will be relesed with nc26.

@MichaIng MichaIng added this to the Nextcloud 26 milestone Feb 24, 2023
@MichaIng MichaIng added 4. to release Ready to be released and/or waiting for tests to finish and removed 0. Needs triage Pending check for reproducibility or if it fits our roadmap labels Feb 24, 2023
@MichaIng
Copy link
Member

Merged and fixed with NC26.

@RTechSn
Copy link

RTechSn commented Aug 25, 2023

Hello.
I have this exact problem on my Nextcloud Hub 5 (27.0.2), freshly installed on Proxmox VM.
I gave it 16 cores and 16 gigs of RAM.
After uploading 5GB of photos and scrolling down Photo Gallery, many apache processes are spawned, bringing all cores to 100% load. After some scrolling the server stops responding or gives Internal Server Error.

@MichaIng
Copy link
Member

MichaIng commented Aug 25, 2023

Did you try to set preview_concurrency_all in config.php explicitly? It should default to 8, so you could try to set it to 4 or 2 and see whether this makes a difference.

I seems you use mod-php? I strongly recommend to switch to php-fpm, so there is no new Apache process with bundled PHP spawned on every connection and task, but things are handled by the dedicated FPM server. This should be much more efficient for concurrent connections.

@RTechSn
Copy link

RTechSn commented Aug 25, 2023

Thank you. I have been looking for that setting, unsuccessefully. I will try it now..

@RTechSn
Copy link

RTechSn commented Aug 25, 2023

I seems you use mod-php? I strongly recommend to switch to php-fpm, so there is no new Apache process with bundled PHP spawned on every connection and task, but things are handled by the dedicated FPM server. This should be much more efficient for concurrent connections.

I have followed this document to install Nextcloud.
https://docs.nextcloud.com/server/latest/admin_manual/installation/example_ubuntu.html

@MichaIng
Copy link
Member

Um, yeah, that should be updated, IMO. For some reason it is also still the default PHP variant in Debian, so it is at least easier to install mod-php.

Let's see first whether limiting the concurrently generated previews works, just in case there has been a bug introduced regarding this.

@RTechSn
Copy link

RTechSn commented Aug 25, 2023

Cannot confirm that it helped, since i've already generated all previews using Preview Generator plugin. I will perform an experiment later, on a copy of the server.
But right now when i scroll the gallery , many new processes are spawned: /usr/sbin/apache2 -k start
There are more than i've set in preview_concurrency_all setting, but there is no overload (perhabs because all previews are generated).

Sorry for off topic, but could you point me to a guide on how to install Nextcloud with PHP-FPM? And should i switch from Apache to Nginx?

@MichaIng
Copy link
Member

A new Apache process spawns for every connection to the backend, not just for preview generation requests. Using FPM implies the other benefit that a more efficient Apache MPM (Multi-Processing Module) can be used, i.e. the event MPM instead of the prefork MPM. It does not create an own process for each request, but does this via threads of a smaller/fixed amount of worker processes.

No need to install Nginx. I is known to be more efficient for static websites, but I'm not aware of any benefit for dynamic websites like Nextcloud. It is probably often confused with PHP-FPM as it is commonly used with Nginx, which has no implementation like the Apache mod-php.

Install it like this:

apt install php-fpm
a2dismod php8.2 mpm_prefork
a2enmod mpm_event proxy_fcgi
a2enconf php8.2-fpm
systemctl restart apache2

If it works, you can uninstall mod-php

apt autopurge libapache2-mod-php

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish 25-feedback bug feature: previews and thumbnails
Projects
None yet
Development

Successfully merging a pull request may close this issue.