Support storage of cache in an option #1
Conversation
There was a problem hiding this comment.
Nice start but I think this needs a bit more work in the following areas:
- The cache might be set in an option now (optionally), but the expiry time is still a cache option, which makes it unpredicatable when redis falls away. Shouldn't we move all cache operations to options?
- Same for the lock
- We'd have to replicate cache group deletion as option group deletion
It might be more maintainable to extend a storage class and pass that into the various functions via dependency injection so that we can simplify the number of function arguments in various places.
This commit involves updating the StorageProvider class and related classes such as CacheStorageProvider and TransoptionStorageProvider. Changes include adding new functions for lock verification and cache group registration. Also, OptionStorageProvider was renamed to TransoptionStorageProvider and several adjustments were made to the `namespace.php` file. These updates aim to enhance the caching functionality and improve data consistency.
This change updates the variable name from "cache_group" to "group" in several function parameters for consistency and simplicity. This change affects the register function in multiple files and helps to streamline the code, making it easier to read and understand.
|
I think we made the assumption that using options as a storage provider the option is stored in the database so that if the cache layer isn't there, it would keep working. but that's incorrect in the case of a site with an object-cache. It uses the cache anyway, so when that falls away, there's nothing, and the option table will be populated for the first time. which means in our case, the fallback message is shown. which means might as well keep the simple implementation of using the caching layer. In particular: "This ensures it's persistent even after cache being flushed." This is incorrect as |
I don't think this is correct. Options will always update the database even if option reads are cached in Redis to keep things fast. This is not the case for transients, which you are correct will use Redis cache exclusively if available. In this PR I stored the cached form in the options table using update_option. But I did use transients to store the cache expiry. Maybe better to use options here too to ensure behaviour is consistent. |
|
Your improvements here to have different storage providers is really nice too. |
|
Yeah you're right. https://github.com/WordPress/wordpress-develop/blob/50af37a9083f003f8e98d089091d2cc428797cc5/src/wp-includes/option.php#L1120 Shows I'll have another look at this PR soon. |
Allow specifying storage of cache data in an option. This ensures it's persistent even after cache being flushed.