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

Add to local cache even if memcache add fails #143

Open
p00ya opened this issue Apr 30, 2023 · 0 comments
Open

Add to local cache even if memcache add fails #143

p00ya opened this issue Apr 30, 2023 · 0 comments

Comments

@p00ya
Copy link

p00ya commented Apr 30, 2023

I recently encountered pathological behaviour when Memcache writes succeed but reads fail.

As a summary of what the conditions/behaviour of wp-memcached v4.0.0 and a single key:

  • on request 1, memcache add succeeds and the key is written to both the local cache and memcache.
  • consider request 2 get: initially the local cache lookup fails, the memcache lookup fails†, but memcache add also fails down the ([mc already]) path because the existing cache value is detected. This path does not write to the local cache:
    if ( false !== $result ) {
    $this->cache[ $key ] = [
    'value' => $data,
    'found' => true,
    ];
  • all subsequent lookups in request 2 also fail in an expensive way (there's nothing in the local cache, they go away and do some SQL queries, then they call add again). For keys like alloptions, this means hundreds of SQL queries.

† why does the memcache lookup fail after a successful add? Well, in my case it was because wp-memcached and Google's Memcache API had different assumptions about memcache flag support (I've raised a bug with Google). But there are other realistic scenarios, such as memcache being unavailable (e.g. failing all get operations) or memcache servers that only provide eventual consistency.

I understand that the existing behaviour is intentional: by not setting the local cache when the write fails, we would normally expect the next call to get to succeed in a memcache lookup and then populate the local cache. But admins add the Memcached Object Cache in order to improve performance and reliability, so if the memcached starts failing reads, the performance degradation and SQL server load will actually be much worse than the default WordPress behaviour with a local object cache (which limits key lookups to once per request). The downside seems to be that the add is only going to get retried once per request (rather than once per lookup), which seems like an acceptable trade-off to me.

If the maintainers agree, I can send a PR. Also open to some class option similar to default_expiration that could be set in one place to opt in to my proposed local caching behaviour.

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

No branches or pull requests

1 participant