juliusknorr / nextcloud-docker-dev

Nextcloud development environment using docker-compose
GNU Affero General Public License v3.0
139 stars 74 forks source link

Profiler enabled while application profiler is not #333

Open come-nc opened 3 months ago

come-nc commented 3 months ago

https://github.com/juliushaertl/nextcloud-docker-dev/blob/dca6126821ffdc674333b87901236440669fe2e5/docker/configs/default.config.php#L12

The default configuration enables the profiler, even if the profiler application is not there. Because of that, sync jobs for LDAP users are running out of memory, because the profile data fills it up. I chased a memory leak before finding out this was simply profiling data, as I was not aware the profiler was enabled.

WDYT @juliushaertl ?

juliusknorr commented 3 months ago

Fine to turn it off, though maybe we can also harden the server code a bit in that regard. We could also consider dropping some profiling data instead if it takes up some large percentage of the memory limit

come-nc commented 3 months ago

I’m wondering the same thing, should the server check somehow that the app is enabled?

juliusknorr commented 3 months ago

Only for the config value it seems

https://github.com/nextcloud/server/blob/dae7c159f728a90ffa53247d6e033abdae5d2bd6/lib/private/Profiler/Profiler.php#L28 https://github.com/nextcloud/server/blob/dae7c159f728a90ffa53247d6e033abdae5d2bd6/lib/private/Diagnostics/EventLogger.php#L44-L49

come-nc commented 3 months ago

I know what it does, I was wondering what it should do. I think the reasonning is that other applications could use data from the profiler so it’s too restrictive to only enable it if the app is enabled. But in practice it meanns people may have the profiler enabled and eating resources without knowing it.