wp_load_alloptions() fails to set object cache when persistent alloptions cache is "0"

[danielbachhuber]

When alloptions persistent object cache is set to 0, wp_load_alloptions() fails to reset it with appropriate values. This results in every use of get_option() to produce the load alloptions query SELECT option_name, option_value FROM wp_options WHERE autoload = 'yes' because the object cache never gets appropriately populated. Specifically, wp_cache_add()won't set the alloptions value when it's already seen to be set.

The end result is a massive impact on site performance when the object cache backend failboats. I'd expect WordPress to handle this scenario a bit more gracefully, possibly falling back to its internal object cache if it detects failures with the persistent object cache. However, it's debatable as to whether the solution lies within a drop-in, or whether it's the responsibility of core.

wp_cache_add() is used over wp_cache_set() for performance reasons with *external* object caches — it doesn't matter one way or another for the internal object cache (genesis #4138). One option is to distinguish between them, and offer different behaviors. However, the most technically appropriate solution is likely to check that the data coming from wp_cache_get() is as expected.

To answer your question before you ask it, I'm not sure how the Memcached persistent object cache value gets set to zero. It seems to come and go, sometimes occurring regularly when I restart Memcached, and I've seen the issue reproduce with both Ryan Boren's and Zack Tollman's drop-ins (meaning real bug is most likely outside of WordPress).

[nacin]

We could do a wp_cache_set() if $alloptions is !== false in wp_load_alloptions(). (The proper way to do this would be to use the &$found argument in wp_cache_get(), but it's not reliably in cache backends and is really only useful for crazier environments like WordPress.com.)

That said, I've never seen another report on this, and this kind of cache poisoning is really tough to account for. I'd rather not sweep the problem under the rug if only because if this is happening to you for alloptions, it'll probably happen for other keys eventually, and this is a slippery slope.

If $alloptions_db ends up being an empty array, and an empty array gets passed to wp_cache_add(), is it possible to get back a 0 with these backends? I assumed not, but I'm not sure. (And if so, are your queries failing? We suppress DB errors there and it may be hiding a problem.)

[danielbachhuber]

If $alloptions_db ends up being an empty array, and an empty array gets passed to wp_cache_add(), is it possible to get back a 0 with these backends? ... (And if so, are your queries failing? We suppress DB errors there and it may be hiding a problem.)

That's a good question — I'd assume WordPress wasn't involved in setting the bad data (because I had just restarted Memcached), but that could be the source. If/when this starts happening again, I'll do more debugging.

[danielbachhuber]

Related discussion: ​https://github.com/Automattic/vip-quickstart/issues/216

[jipmoors]

Applied the discussed code with added type and result checking

[jipmoors]

I think the most critical time is when you have to restart your memcache. If your cache is not booting up properly downtime is not far away. Especially on sites that are never empty.

I know most of the sites that run on Wordpress do not apply to this, but I think we should need to think big and make it as solid as possible. These kind of problems can make or break a decision on a platform.

[jipmoors]

#31147 was marked as a duplicate.

[danielbachhuber]

I ran into this again, with a different object cache implementation.

[desrosj]

Re-adding a milestone to this one.

Definitely would like to see some unit tests demonstrating this issue. The patch also needs to be refreshed.