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

Clarification: Is APCu acceptable for memcache.locking? #43895

Open
5 tasks
joshtrichards opened this issue Feb 28, 2024 · 2 comments
Open
5 tasks

Clarification: Is APCu acceptable for memcache.locking? #43895

joshtrichards opened this issue Feb 28, 2024 · 2 comments
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 29-feedback bug feature: caching Related to our caching system: scssCacher, jsCombiner... feature: filesystem pending documentation This pull request needs an associated documentation update

Comments

@joshtrichards
Copy link
Member

Is APCu acceptable for usage for transactional file locking under any circumstances we consider "supported"? If it's acceptable, are there caveats?

Background: Last year @linuxserver set a default value of for memcache.locking that utilizes APCu in their Nextcloud image: linuxserver/docker-nextcloud@aaa5539 via linuxserver/docker-nextcloud#317.

This was apparently in response to the performance warning added in v27 (when the database is being used for locking - i.e. when Redis isn't configured): linuxserver/docker-nextcloud#319

The end result is that all LinuxServer image users are now using APCu for transactional file locking rather than the database (unless the admin has explicitly configured Redis). This achieved their goal: no longer a warning about using the database for locking.

But is this a valid configuration?

Our documentation is not 100% clear whether we support the use of APCu for memcache.locking:

We have fallback code that appears to prefer a NULL_CACHE (over using APCu as a fallback) for locking:

if (!($lockingCacheClass && class_exists($lockingCacheClass) && $lockingCacheClass::isAvailable())) {
// don't fall back since the fallback might not be suitable for storing lock
$lockingCacheClass = self::NULL_CACHE;

My understanding is that APCu can maybe work for mod_php and some PHP-FPM operating modes. The key requirement being that the operating mode in-use must permit all processes to use a shared APCu cache. However, even if the application server layer is fine, anything happening at the command-line will not use the same APCu cache (effectively being blind to Transaction File Locking). This could lead to problems in areas like occ files commands (and possibly others) I'd think.

So is APCu acceptable at all? Is it worse - or better - than a NULL_CACHE?

TODO:

  • Confirm / get feedback
  • Create an issue in docs to track clarification / Clarify the docs in a PR
  • If "no": Adding a check that explicitly disallows APCu for memcache.locking even if specified explicitly in the config?
  • If "no": Coordinating with LinuxServer to change their image behavior
  • If "yes": Should we consider changing our default fallback behavior?
@joshtrichards joshtrichards added enhancement 0. Needs triage Pending check for reproducibility or if it fits our roadmap pending documentation This pull request needs an associated documentation update bug feature: filesystem feature: caching Related to our caching system: scssCacher, jsCombiner... and removed enhancement labels Feb 28, 2024
@solracsf
Copy link
Member

This answers part of it:

* Because most memcache backends can clean values without warning using redis
* is highly recommended to *avoid data loss*.

Basically, every time PHP get's restarted, APCu is cleared, so are the locks. This is not the case with Redis by default.

Adding a check that explicitly disallows APCu for memcache.locking even if specified explicitly in the config?

I would say push a warning basically with the same message than above 👍

@kesselb
Copy link
Contributor

kesselb commented Mar 2, 2024

#38796 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
0. Needs triage Pending check for reproducibility or if it fits our roadmap 29-feedback bug feature: caching Related to our caching system: scssCacher, jsCombiner... feature: filesystem pending documentation This pull request needs an associated documentation update
Projects
None yet
Development

No branches or pull requests

4 participants