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

(server tuning) Clarify opcache settings impact on occ maintenace:mode #10645

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshtrichards
Copy link
Member

…and config.php

Clarify impact of messing with opcache.revalidate_freqand/or disabling revaliation completely (i.e. opcache.validate_timestamps = 0).

☑️ Resolves

🖼️ Screenshots

image

@szaimen szaimen added this to the Nextcloud 28 milestone Jun 18, 2023
@szaimen szaimen requested a review from MichaIng June 18, 2023 17:49
@MichaIng
Copy link
Member

MichaIng commented Jun 18, 2023

I would probably phrase it differently, doing less changes but mostly add "and some occ commands like occ:maintenance:mode" after "config.php". But I am no native speaker and the change is factually correct. So I'll leave approval up to someone else, regarding whether it is good and understandable English 🙂.

In any case, we want this in older NC version docs as well.

@MichaIng
Copy link
Member

/backport to stable27

@MichaIng
Copy link
Member

/backport to stable26

@kesselb
Copy link
Contributor

kesselb commented Jun 18, 2023

Thanks for your pull request 👍

We try to invalidate the cache when changing the configuration file.1

Afaik opcache is different for cli/web and therefor maintenance:mode --off will not invalidate the cache for web requests. I guess we need a web controller to trigger a cache clear from cli for opcache web.

opcache.validate_timestamps = 0

I expect to run into the same issue (no changes without php-fpm restart) for

occ app:install, oc app:update, occ app:remove, occ system:config:set and any other occ command altering php files.

Footnotes

  1. https://github.com/nextcloud/server/blob/79aca5520e966eb15fa82f6cfec089e041f3558c/lib/private/Config.php#L310

@MichaIng
Copy link
Member

MichaIng commented Jun 18, 2023

Afaik opcache is different for cli/web and therefor maintenance:mode --off will not invalidate the cache for web requests.

Exactly, different PHP process (even different executable) means own OPcache. CLI should not have any OPcache at all, unless it has been manually explicitly enabled in PHP configs (not recommended). There is no way of accessing another PHP instance's OPcache, reasonably.

I guess we need a web controller to trigger a cache clear from cli for opcache web.

That would be great. But it needs to be somehow secured, of course no open web API. Some sort of flag in database or on filesystem wouldn't make sense either: If a flag needed to be checked on every web access/action, it would break the little performance benefit of disabling OPcache validation. So either there is a secure way to directly trigger it, or it better remains a documented limitation when disabling OPcache validation.

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

I like this change in general. Maybe we could even add a warning to maintenance:mode command that if o opcache is enabled one might need to wait up to the revalidate_freq to make it visible in the web interface or restart php or the container. (Also just had this conversation with a user: nextcloud/all-in-one#3637

@joshtrichards
Copy link
Member Author

Another related issue (not sure why I didn't link this originally): #9184

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

Successfully merging this pull request may close these issues.

Maintenance mode does not invalidate OPcache [Bug]: Maintenance:mode changes are not respected
8 participants