Allow object caches to degrade gracefully

[markjaquith]

Because of the way object caches are loaded, if a custom object cache can't run (say, Memcached or APC is not actually installed), it cannot gracefully degrade to the built-in object cache.

Witness this code in wp_start_object_cache():

 	if ( ! function_exists( 'wp_cache_init' ) ) {
		if ( file_exists( WP_CONTENT_DIR . '/object-cache.php' ) ) {
			require_once ( WP_CONTENT_DIR . '/object-cache.php' );
			$_wp_using_ext_object_cache = true;
		} else {
			require_once ( ABSPATH . WPINC . '/cache.php' );
			$_wp_using_ext_object_cache = false;
		}
		$first_init = true;
	} else if ( !$_wp_using_ext_object_cache && file_exists( WP_CONTENT_DIR . '/object-cache.php' ) ) {
		// Sometimes advanced-cache.php can load object-cache.php before it is loaded here.
		// This breaks the function_exists check above and can result in $_wp_using_ext_object_cache
		// being set incorrectly. Double check if an external cache exists.
		$_wp_using_ext_object_cache = true;
	}

So a custom object cache is loaded. If it wants to bail and defer to the built in object caching, it can do that by doing the include itself. But then WordPress sets $_wp_using_ext_object_cache = true; after that require. So WordPress thinks it is using an external object cache when it's actually not. This leads to oddness.

This can sometimes be hacked around by adding a callback to the very first WP action available that sets $_wp_using_ext_object_cache = false;, but that has issues: calls to the object cache might be made before that code can run, and add_action() is not always available at that point (because some advanced-cache.php drop-ins load the object cache really early. See Batcache.

Proposed solution: change the order of the require_once() and the setting of $_wp_using_ext_object_cache. That way, the external object cache can override the variable.

[wonderboymusic]

this is mildly related: #21401

[wonderboymusic]

Unless I'm missing something, we did this here: [25289].

[SergeyBiryukov]

We still call wp_using_ext_object_cache( true ) after the require_once(), so this ticket was not addressed in [25289]. I guess 22661.patch​ should resolve this.

[SergeyBiryukov]

Looks like 22661.patch​ might not work after [27634] (it was relying upon the initial null value).

22661.2.patch​ is another take. A custom object cache should be able to bail and defer to the built-in caching by including the file and calling wp_using_ext_object_cache( false ).

[nacin]

[27634] can be reverted if necessary, but it felt like proper behavior to me. 22661.2.patch​ sounds good to me.

[nacin]

Well, there is some additional code here:

	} else if ( ! wp_using_ext_object_cache() && file_exists( WP_CONTENT_DIR . '/object-cache.php' ) ) {
		// Sometimes advanced-cache.php can load object-cache.php before it is loaded here.
		// This breaks the function_exists check above and can result in $_wp_using_ext_object_cache
		// being set incorrectly. Double check if an external cache exists.
		wp_using_ext_object_cache( true );
	}

So this wouldn't work in the case of Batcache loading object-cache.php earlier.

[nacin]

I'm going to punt this until it has an approach that handles all cases.

[jipmoors]

A solution would be to set a reserved key (_wp_cache_active?) and retrieve it to see if the cache is actually active before setting the wp_using_ext_object_cache blindly to enabled.

As a counter argument I realise that you do create overhead, but cache should be fast and lean in it's nature.

I guess the problem with an external cache provider that is not working, is that the functions would be defined in the implementation of that external cache. Which means that all cache should be disabled and we can't revert to core cache.php.

[jipmoors]

Another idea is to always include the cache.php from core and add filters to the public functions.
This way the functions act as a permanent gateway into the custom cache implementations, which would give us more security and testing abilities.

Also this solution would supply a clean way for the cache to have one or more fallbacks, which should give cache plugin creators more freedom and cleaner code.

So far the only problem with this approach is that wp_cache_get has the 4th parameter &$found, which a null check would provide for a correct response.

[mpvanwinkle77]

As far as design goes, I generally agree with @jipmoors that, given the WordPress API, any class "overloading" should take place in the context of a filter and NOT simply placing the file in a certain location.This change would solve both this issue and the rather clumsy way that external object caches are currently installed which can be problematic on locked-down filesystems. This comment isn't intended to block this patch but only to suggest a ticket for overhauling this in the future. Obviously the implementation would have to be to respect the file-based overloading first so as not to create any backward compatibility issue. But over time that approach could be deprecated.

[jipmoors]

If you use an object-cache.php (or advanced-cache.php) without the wp_cache_init function then the wp_using_ext_object_cache would remain false and you can attach to the filters and have the core cache.php being loaded. Which in turn calls the apply_filters.

So without any modifications in file loading you can move forward by just enabling the apply_filters in all the cache.php functions.

The only note with this is that first_init would be called every time. Which is actually a good thing, because we need the WP_Object_Cache to be instantized to provide fallback functionality.

I see some ways of combating this problem:

1. (favorable) Check for a define or function to verify a filter implementation is present
2. Using a new file to check against for cache implementation via filters

Adding to that the notion of writing unit-tests on the public functions of the Cache API instead of the class would make even more sense.

[jipmoors]

implemented filters which enable cache fall through

[jipmoors]

cache via filters + filter to enable external object cache

[jipmoors]

Using filters it not going to work. The only problem is wp_cache_get which needs to be able to return a 'null' when the cache contains this value for a key.

A method that allows you to registers a cache handler with a priority would be a solution.
The public functions will work through the registered handlers until the request is completed.
The handlers should extend the WP_CacheHandler abstract class to ensure minimal function compliance.

I think this is a concept that has a lot of potential and will see if I can make an elegant implementation.

Please forget/ignore/remove my previous patches.

[jipmoors]

• reverted to silence -

[SergeyBiryukov]

Replying to jipmoors:

The code referenced to is no longer actual.

There has been a modification to check for the existence of wp_cache_init after including the object-cache.php which allows for core cache usage. (r25289)

Right, but [25289] did not resolve this ticket, see comment:5.

[jipmoors]

When object-cache.php has been loaded and wp_cache_init exists but wp_using_ext_object_cache(false) is used to disable external cache from within this code. Then wp_cache_init is already defined.

How is cache.php going to be loaded when it depends on it's own implementation of that function.

[jtsternberg]

Related discussion/workaround: ​https://webdevstudios.com/2015/11/03/loading-the-optimal-wordpress-object-cache-implementation-in-your-production-staging-and-local-development-environments/#comment-85993

[markjaquith]

Replying to wonderboymusic:

Unless I'm missing something, we did this here: [25289].

You are right.

If your object-cache.php does not define the wp_cache_init() function, wp_using_ext_object_cache( true ); will not be called, and this will run, loading WP's in-memory cache:

	if ( ! wp_using_ext_object_cache() ) {
		require_once ( ABSPATH . WPINC . '/cache.php' );
	}

So you can just wrap your object-cache.php content (including all the function definitions) in a prerequisites check. Like:

if ( class_exists( 'Redis' ) ) {
    // Original file contents here.
}

Or, better, move the object cache file you're using to redis-object-cache.php and do:

if ( class_exists( 'Redis' ) ) {
   include( __DIR__ . '/redis-object-cache.php' );
}

So you can keep the original file unadulterated.

Closing this, as the original intent ("allow custom object caches to gracefully degrade") is possible now.

[jtsternberg]

Replying to markjaquith:

If your object-cache.php does not define the wp_cache_init() function, wp_using_ext_object_cache( true ); will not be called, and this will run, loading WP's in-memory cache:

I’m still seeing issues with this approach, but only on multisite. Because ms-settings.php calls wp_start_object_cache() again, it makes WP think an external object-cache exists: ​http://b.ustin.co/zc3syn

So it means WP did load it's default implementation (wp-includes/cache.php), which is good as some cache will be used some times, but $_wp_using_ext_object_cache is set to true, which is bad because wp_using_ext_object_cache() is used quite a bit to determine if some things will attempt to be cached to a persistent object cache (causing extra queries), and to determine if some things don't need to be cached (causing extra queries). So something like this ​https://github.com/jtsternberg/object-cache.php-replacement/blob/master/object-cache.php is needed to reset that global during the first available hook.

It might be a decent solution for wp_using_ext_object_cache() to have a filter, as I think you could then override it for this scenario.

OR, allow the object-cache.php file to define define( 'USING_EXT_OBJECT_CACHE', false ); which could override any logic that occurs downstream.
Then wp_using_ext_object_cache would look something like:

function wp_using_ext_object_cache( $using = null ) {
	global $_wp_using_ext_object_cache;
	$current_using = $_wp_using_ext_object_cache;

	if ( defined( 'USING_EXT_OBJECT_CACHE' ) ) {
		$using = USING_EXT_OBJECT_CACHE;
	}

	if ( null !== $using ) {
		$_wp_using_ext_object_cache = $using;
	}
	return $current_using;
}

[markjaquith]

Replying to jtsternberg:

I’m still seeing issues with this approach, but only on multisite.

I conferred with @jtsternberg a bit on this, and yeah, that "else if" block gets triggered on subsequent runs of the function (which happens in multisite), which causes it to think an external cache is being used when it is not.

We worked on a refactor, and came up with 22661-logic-refactor.diff​, which converts $first_init to a static, which enables us to tell if it is a first run of the function (during which we should try to load object-cache.php or detect that advanced-cache.php has already loaded it) or a secondary run of the function (during which we should do neither).

@jtsternberg tested with Batcache, multisite, and with an object-cache.php that does not define wp_cache_init() (our "fall back to normal in-memory caching" scenario).

[markjaquith]

In 42723:

Cache API: Allow external object caches to gracefully degrade to the default object cache.

Rework logic for how external object caches are detected, so that if
an external cache does not define a wp_cache_init(), the built-in
object cache will be used.

Object caches can now wrap their entire contents in logic checks. So a
Redis caching backend could make sure that the Redis PHP class is
available before defining all the caching functions. And if Redis is
not available, the site doesn't break or throw errors or think it is
using caching when it isn't. This is particularly useful for doing
local development, where you might want to develop on a site without
running Memcache or Redis like you are in production.

• Accounts for multisite, which may re-initialize the object cache

multiple times.

• Accounts for object caches that may include object-cache.php during

advanced-cache.php (before WP loads it).

Props jtsternberg, markjaquith.
Fixes #22661.