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

Explore improving WP_Object_Cache #36

Closed
tillkruss opened this issue Dec 3, 2021 · 6 comments
Closed

Explore improving WP_Object_Cache #36

tillkruss opened this issue Dec 3, 2021 · 6 comments
Assignees
Labels
[Type] Enhancement A suggestion for improvement of an existing feature

Comments

@tillkruss
Copy link
Member

tillkruss commented Dec 3, 2021

Looking at WP_Object_Cache to see if we can improve it overall, including the handling of mega objects, such as the alloptions key.

@tillkruss tillkruss added [Type] Enhancement A suggestion for improvement of an existing feature [Focus] Object caching labels Dec 3, 2021
@tillkruss tillkruss self-assigned this Dec 3, 2021
@JanThiel
Copy link

I would like to add the suggestion to explore options to explicitly add 1st class citizen support for WordPress Multisite Setups.
The Object Caching generally works but it currently lies within the implementation layer to decide how to (and if) handle MS support.

The issue with the current API becomes obvious with the wp_cache_flush() method. It doesn't define how to handle MS. Take the available implementations for Redis as caching backend into account and all the implementations do call a "simple" redis->flush on the current active redis database. This flushes all data from the cache. For all sites / blogs.
There isn't a (standardized) way to flush the cache for one site only. Which is kind of a huge issue for large setups where flushing - or better: rebuilding - the whole cache can be kind of expensive.
As there is no explicit API for doing this none of the implementations actually "cared" for this.

In the wild I saw some 3rd party plugins expecting the wp_cache_flush function to be called in the context of the current site and thus using switch_to_blog($id) before and after. Something I can understand and would expect myself.
But truth is there is no standardized handling of Multisite setups in the current Object Caching API. And I would strongly suggest to add one.

MVP: Add a function to explicitly flush one particular sites cache. Could be done via optional $site_id / $blog_id parameter for wp_cache_flush as well. This would allow full backward compat.

@JanThiel
Copy link

Another issue we identified in the current way the Object Caching is setup are the "global cache groups". They are not really ment to be changed or configured in the way they are currently implemented.

Again: This only affects Multisite Setups. But it is also another indicator that Object Cache + Multisite does not go along that well currently.
Details are described here already: https://core.trac.wordpress.org/ticket/54303

@tillkruss
Copy link
Member Author

@JanThiel Both fantastic points, I'll split these into separate issues to explore soon.

@tillkruss
Copy link
Member Author

@JanThiel See #76

@JanThiel
Copy link

JanThiel commented Jan 4, 2022

I do have one more thing :-) ... But it's a complex one...

wp_cache_get and wp_cache_get_multiple support a bool $force flag to force direct cache layer access without the intermediate WordPress in-memory cache. This is quite useful (and the only way I am aware of) to circumvent concurrency issues when having longer running background tasks (WP-Cron triggered). For example image processing tasks.

Having multiple processes like WP-CLI + WEB will lead to situations where changed cached data will not be updated within a running process.

This can be solved by using the $force = true flag in the Caching layer.
Yet ... this function is not exposed anywhere and thus not usable within the regular WordPress API to access data.

For example:
get_post_meta( int $post_id, string $key = '', bool $single = false )
https://developer.wordpress.org/reference/functions/get_post_meta/

This is the WordPress way to access post meta. But there is no way to tell WP to force flush the Object cache. Even when you know that there are potential issues.
As a workaround you have to call wp_cache_get constructing the cache key and passing $force = true. This is in no way reliable, scalable or sane.

I like to propose considering adding a $force_object_cache_flush = false argument to the API layer accessing cached data. Like get_post_meta et cetera.

This is kind a big thing. I know. But maybe there is a smarter way which is less intrusive?

@tillkruss
Copy link
Member Author

@JanThiel: There might be a better way: woocommerce/action-scheduler#790

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

2 participants