Automattic / Automattic/wp-memcached

Add to local cache even if memcache add fails

Open
#143 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
PHP
Stars
172
Forks
52
PR merge metrics
No merged PRs in 30d

Description

I recently [encountered pathological behaviour](https://scarff.id.au/blog/2023/slow-wordpress-memcached-object-cache-with-app-engine/) 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: https://github.com/Automattic/wp-memcached/blob/92ed2893b316984b34a527fbda638579c8345381/object-cache.php#L189-L193
* 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.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with object-cache.php at lines 189-193 and trace the request-2 path where a failed Memcache get is followed by a failed add. Check how local caching and repeated lookups behave for keys such as alloptions, then verify that the agreed behavior limits retries to once per request without increasing SQL queries.

Written by the indexing model from the issue text.

Assessment

Tech stack
memcached, php, wordpress
Domain
backend, performance
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.