wp-memcached
wp-memcached copied to clipboard
Add to local cache even if memcache add fails
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 memcacheadd
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 likealloptions
, 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.