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

Inits 115 support flushing cache groups #2368

Open
wants to merge 26 commits into
base: trunk
Choose a base branch
from

Conversation

pbearne
Copy link

@pbearne pbearne commented Mar 1, 2022

src/wp-includes/class-wp-object-cache.php Outdated Show resolved Hide resolved
src/wp-includes/class-wp-object-cache.php Outdated Show resolved Hide resolved
src/wp-includes/cache.php Outdated Show resolved Hide resolved
src/wp-includes/cache.php Show resolved Hide resolved
@spacedmonkey
Copy link
Member

@spacedmonkey spacedmonkey commented Mar 1, 2022

If this to live in core there needs to be a compat function found in core in cache-compat.php. This function would need to work for ALL object cache implementation, including and not limited to memcached.

I personally think this is going be nearly impossible, as it is hard to know the implementation details of every object cache. But without that compatible layer, this can not live in core 😞

@tillkruss
Copy link
Member

@tillkruss tillkruss commented Mar 1, 2022

This function would need to work for ALL object cache implementation, including and not limited to memcached.

It's a FAQ and the suggested approch is using prefixes, which I believe wp-memcached is already using heavily.

@spacedmonkey
Copy link
Member

@spacedmonkey spacedmonkey commented Mar 2, 2022

Please use remove test for wp_cache_get_linked_meta and revert changes to comments and spacing that are not required for this change.

@spacedmonkey
Copy link
Member

@spacedmonkey spacedmonkey commented Mar 2, 2022

If this is going to make it into core, then we are going to need some constant, like WP_ALLOW_CACHE_GROUPS. Drop-in providers, will have to define this. That way, developers / core can work out if we can even use this function.

Thoughts?

src/wp-includes/cache-compat.php Outdated Show resolved Hide resolved
@tillkruss
Copy link
Member

@tillkruss tillkruss commented Mar 2, 2022

If this is going to make it into core, then we are going to need some constant, like WP_ALLOW_CACHE_GROUPS. Drop-in providers, will have to define this. That way, developers / core can work out if we can even use this function.

Reminds me of get_theme_support() and add_theme_support().

If we default to flushing the entire cache (in the compatibility method), can you give an example how and where WP_ALLOW_CACHE_GROUPS would be used?

@pbearne
Copy link
Author

@pbearne pbearne commented Mar 2, 2022

I am not sure we do need the global WP_ALLOW_CACHE_GROUPS as I test for a flush_group function
method_exists( $wp_object_cache, 'flush_group' )
and return false if not available so the calling code and handle it

src/wp-includes/cache.php Outdated Show resolved Hide resolved
src/wp-includes/cache.php Outdated Show resolved Hide resolved
src/wp-includes/cache-compat.php Outdated Show resolved Hide resolved
tests/phpunit/tests/cache.php Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants