-
Notifications
You must be signed in to change notification settings - Fork 2k
fix: remove PHP cache validation disabling example #12079
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
base: master
Are you sure you want to change the base?
Conversation
/backport to stable29 |
/backport to stable28 |
Should we remove the quoted part as well?
Yep. As you wrote, people just copy it because it's on the server tuning page and have no idea about the impact. |
Hmm, do we really want to remove valuable information just because some people do not do what docs are made for: reading? I personally actually do recommend disabling PHP revalidation for serious production Nextcloud systems where you want to max out performance = user experience as much as possible. I personally have a little command which clears No problem with making warnings clearer, putting the whole info into a warning box, whatever, but we are not Apple here (sorry for the bias/cliche 😉) where we offer just one failsafe default to prevent users (and we are talking about "admins" here) from configuring things wrong. People do things wrong, and they can learn from this, especially as admin valuable/essential. Just my two cents 😉.
Makes sense then, as this absolutely belongs to disabling revalidation. |
Is there now a consensus here? |
Signed-off-by: Josh <josh.t.richards@gmail.com>
9c85bd3
to
79342eb
Compare
Another user reported a bug related to opcache.validate_timestamps = 0: nextcloud/server#52977. I am uncertain about how to proceed with this pull request. My observation remains that many people skim through the page, copying suggestions without fully understanding the implications. Therefore, a default setting that works for most cases might be the better option here. On the other hand, the page includes a danger block and two warning blocks and explains the entire topic thoroughly. So it is not that we are failing to raise awareness about it 🤷 |
From reading the bug report, it actually really sounds like a bug. When updating entirely via web interface, Nextcloud should automatically invalidate relevant parts of the OPcache, isn't it? |
☑️ Resolves
The way this is presented it seems people skim and enter this example blindly without reading the surrounding text.
We don't want people thinking we recommend this for all environments.
If one wishes to disable validation, that's fine (as noted in the text), but they presumably know what they're doing and can figure it out from the PHP manual on their own.
I think @kesselb hinted about removing this part during the last edit round of this area. ;-)
🖼️ Screenshots