documentation icon indicating copy to clipboard operation
documentation copied to clipboard

fix: remove PHP cache validation disabling example

Open joshtrichards opened this issue 1 year ago • 7 comments

☑️ 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

joshtrichards avatar Jul 31 '24 12:07 joshtrichards

/backport to stable29

joshtrichards avatar Jul 31 '24 12:07 joshtrichards

/backport to stable28

joshtrichards avatar Jul 31 '24 12:07 joshtrichards

Any Server/app upgrades or changes to config.php will then require restarting PHP (or otherwise manually clearing the cache or invalidating this particular script).

Should we remove the quoted part as well?

I think @kesselb hinted about removing this part during the last edit round of this area. ;-)

Yep. As you wrote, people just copy it because it's on the server tuning page and have no idea about the impact.

kesselb avatar Jul 31 '24 12:07 kesselb

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 config.php from OPcache via opcache-gui web API, for cases where I do not want to restart PHP. When upgrading Nextcloud from console, I restart PHP anyway, as the downtime is there already.

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 😉.

Should we remove the quoted part as well?

Makes sense then, as this absolutely belongs to disabling revalidation.

MichaIng avatar Jul 31 '24 13:07 MichaIng

Is there now a consensus here?

rakekniven avatar Apr 13 '25 13:04 rakekniven

Another user reported a bug related to opcache.validate_timestamps = 0: https://github.com/nextcloud/server/issues/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.

image

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 :shrug:

kesselb avatar May 30 '25 08:05 kesselb

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?

MichaIng avatar May 30 '25 13:05 MichaIng