berlindb / core

All of the required core code
MIT License
252 stars 27 forks source link

Feature Request: Add a filter/configuration to disable cache on queries #139

Open engahmeds3ed opened 2 years ago

engahmeds3ed commented 2 years ago

I know that using cache for queries is very important and makes the process faster but we have a case that we need to disable it for one query.

The case is that we have a code that inserts multiple fake posts on the same request and with pre_post_update hook we have our callback to make a query to make sure that we don't have a record with some criteria to do action with it so when the cache is enabled, it inserts the first one and with the second one the query gets the data from the cache.

We temporary can solve that by using the following hook https://github.com/berlindb/core/blob/master/src/Database/Query.php#L871

then use the WP function wp_cache_flush but this will flush the whole cache :(

I know that you try to eliminate having short circuit filter into the plugin so we may have an argument to disable the cache with default value is false.

If u agree I can start PR for this one.

JJJ commented 2 years ago

Makes sense to me. Good idea 👍

With this implemented, the get_results() method could finally become a wrapper – right now it duplicates a lot of existing stuff (like Queries\Compare, etc...)


In the meanwhile, look into using wp_suspend_cache_addition(). You could suspend cache addition before you start, which would prevent subsequent queries from finding results in the cache (and also writing to it) until you unsuspend it when you are finished.


// Prevent cache additions
$old = wp_suspend_cache_addition( true );

(...do your things...)

// Restore cache additions to whatever the state used to be
wp_suspend_cache_addition( $old );

Relatedly, this reminds me that Berlin uses wp_suspend_cache_invalidation() on adds and sets, but WP_Object_Cache only uses it for calls to add() and not set() or replace(), and it's worth making doubly sure Berlin is using the correct calls in the right places.

JJJ commented 2 years ago

I went outside and did some exercise, and am coming back with more thoughts.

You can tell me truthfully if they are not good ones. 🤣


There are areas of improvement in WordPress itself with how the wp_suspend_cache_addition() and wp_suspend_cache_invalidation() functions (and the $_wp_suspend_cache_invalidation global) are implemented:

All of this is to say, that – I think – if we do this in Berlin that we should add query variables to help influence/control/toggle all of the cache interactions, and not just object/Query reads or writes, and probably not using WordPress core's approaches, because they aren't really all that awesome currently.

Up to and including adding our own Cache.php file with its own Class and variables to help us work with and abstract everything out in some logical way.

Thoughts?

engahmeds3ed commented 2 years ago

WoW that's a great explanation John, many thanks.

I totally agree having a cache class to have all the cache attributes and methods that can be used in all other classes and we can start with the Query one and see how it goes from there.

do u think that this cache class should have static methods like Utility class to be easily embedded into other classes or a normal class that somehow can be injected into their constructors?

I will invest this weekend trying to think about this new class and the data should be there.