Skipping term cache cleaning when cache invalidation is suspended

[zachop]

Since v4.3, clean_term_cache checks if cache invalidation is suspended, and bails early if it is.

This behavior might be a bit obscure and unexpected, based on the documentation of wp_suspend_cache_invalidation.

I understand that the responsibility of ensuring that the cache is properly invalidated falls on the caller of wp_suspend_cache_invalidation, according to the Codex, but I believe that the word "cache" can be easily misinterpreted as the object cache, not including the (persistent) term cache.

Furthermore, even if persistent caches are not invalidated during invalidation suspension, there should probably be a simple function call to purge them all at once. One might suggest using wp_cache_flush, as its description states that it "removes all cache items", but that is also not exactly true, since the word "cache" does refer to the object cache at this point (expluding persistent caches, e.g. the term cache).

I don't exactly think of this as a bug, and I can imagine the performance-related considerations and decisions that led to that behavior. I just believe that, even if nothing changes, a more detailed description of wp_suspend_cache_invalidation and wp_cache_flush would be really helpful.

[zachop]

Seeing that the ticket is in Awaiting Review status for the past 8 weeks, I'm changing its type from Enhancement to defect, hoping it will be reviewed sooner, given the fact that the described behavior could cause some trouble to developers.

It's not a bug per se, so please feel free to change its type/priority as needed.

Thanks!

[boonebgorges]

Hi @zachop - Sorry for the delayed response - I missed this ticket when it first came in. Thanks for the detailed report, and welcome to WP Trac!

When you refer to a "term cache" that is "persistent", I assume you mean the term hierarchy that WP stores in wp_options. Is that correct? If so, I disagree that it should be treated differently from the "object cache" with respect to suspended invalidation. For one thing, the {$taxonomy}_hierarchy and {$taxonomy}_children caches, like term caches fetched by wp_cache_get(), are not meant to be directly accessed by the developer. From the point of view of the user who's calling get_terms() or get_term() or whatever, the cache implementation is totally invisible. Moreover, the distinction between "persistent" caches and the object cache largely disappears when running a persistent object cache backend like Memcache. For these reasons, I think it's correct that wp_suspend_cache_invalidation() - which *is* developer-facing - would affect both types of caching mechanisms in the same way.

For these reasons, I agree with you that it's confusing for a function like wp_cache_flush() not to clear *all* caches. If the distinction between wp_options-based caching and proper object caching is supposed to be invisible to the developer, then wp_cache_flush() ought to clear both kinds of cache. I think it's worth investigating whether we can come up with a comprehensive list of places in WP where we use the database as a poor-man's-cache, and then to think about whether we can come up with a way of clearing all of these caches with a single method. (This might require something like an internal registry of database-cache keys.) It's probably impractical to modify wp_cache_flush() to handle this kind of invalidation, because cache drop-ins redefine this function, and we don't have control over them. But we could probably have a new wrapper - maybe something like wp_flush_all_caches() - that would wrap wp_cache_flush() and all db caches.

What do you (and others) think about this idea?

[zachop]

Hi @boonebgorges,

When you refer to a "term cache" that is "persistent", I assume you mean the term hierarchy that WP stores in wp_options. Is that correct?

Yes, that is correct.

By "persistent" I refer to any data stored across requests (e.g. in the database, or a dedicated backend such as Memcache), in a way which is transparent to the developer (as opposed to options or transients, for example, where the storage is explicitly requested for).

I agree with you that a way to clear all cached data would be an elegant solution, while at the same time keeping the caching internals invisible to the developer. A wrapper for that purpose sounds good to me.

I also believe that it is equally important for the documentation of wp_cache_flush() to be updated as soon as possible, in order to state clearly that not all data is actually flushed. The way the various caches now work, it is possible (e.g. during imports with cache addition/invalidation suspended) to be left with stale data in the database, without means of clearing it - since wp_cache_flush() won't do so. This, as you pointed out, cancels the invisibility of the caching operations to the developer.

Please let me know if there's any way that I could contribute to this issue. I would be more than happy to do so, although I would need some guidance on how to start :)

[boonebgorges]

I've been thinking about this some more, and I'm less hopeful about the strategy of an uber-cache-invalidation method than I previously was. WordPress uses database-based "caching" in many places beyond what's described so far in this ticket. For example, the database schemas have columns built in - like wp_posts.comment_count and wp_term_taxonomy.count - which essentially serve a caching purpose. Invalidating all of these counts in one fell swoop could create serious stampede conditions, which would be more serious in a way than memory-based cache stampedes, since db i/o tends to be a more serious bottleneck.

That said, I think it's a very good idea to improve documentation where possible. wp_cache_flush() is one place, but I'd suggest it's perhaps more important to add the necessary documentation to functions where invalidation suspension takes place, like clean_term_cache(). So, for example, clean_term_cache() should make clear that (a) term counts are not recalculated, while (b) term hierarchies *are* recalculated.