Changing category doesn't invalidate cache

[Whissi]

Hi,

I am using object-cache.php Memcached v2.0.2 (Dropins) with the current WordPress version, but I don't think this has anything to do with the dropin.

I noticed that category changes don't invalidate/update the cache.

For example:

1. Create a category "Foo".

2. Assign some postings to "Foo".

3. View your postings in backend and frontend, so that results get cached.

4. Rename "Foo" category to "Bar".

5. Reload some postings, which are in that category. But they still display the old "Foo" category instead the new "Bar" category name.

Flushing memcached would "solve" this problem, that's why I think renaming categories don't invalidate/update the cache.

[wonderboymusic]

Still an issue?

[Whissi]

Yes, still an issue with 3.6.

[wonderboymusic]

In 27099:

Add a unit test demonstrating the failure to invalidate a post's term cache when the term is updated.

See #22526.

[wonderboymusic]

Bugs like this give me nightmares. 22526.diff​ keeps the post terms cache in sync with changes to a given taxonomy.

Introduces 3 new functions:
get_taxonomy_last_changed( $taxonomy )
set_taxonomy_last_changed( $taxonomy )
post_taxonomy_is_fresh( $id, $taxonomy )

post_taxonomy_is_fresh() only needs to be called in get_object_term_cache(). Added some unit tests for the functions and for the condition described in the ticket which, surprise surprise, was reproducible with a basic unit test.

[wonderboymusic]

In 27101:

Add cache invalidation when updating a term, example: create a category, assign it to a post, edit the category. Currently, the post's term cache is not updated. When updating terms in a given taxonomy, invalidate the object term caches linked to that taxonomy.

Introduce get_taxonomy_last_changed(), set_taxonomy_last_changed(), and post_taxonomy_is_fresh().

post_taxonomy_is_fresh() is only called in get_object_term_cache() - at which point the taxonomy's last_changed value is checked against the post's {$taxonomy}_last_changed value.

set_taxonomy_last_changed() is called whenever directory database queries are made that insert new terms or affect existing terms.

Fixes #22526.

[DrewAPicture]

Sorry to reopen, but the docs on get_taxonomy_last_changed(), set_taxonomy_last_changed(), and post_taxonomy_is_fresh() are incomplete.

• All of the function parameters and returns need descriptions.
• All function descriptions should end in periods.

[DrewAPicture]

In 27141:

Improve inline documentation for several 'last_changed'-related functions introduced in [27101] and [27102].

Functions include:

• get_taxonomy_last_changed()
• set_taxonomy_last_changed()
• post_taxonomy_is_fresh()
• taxonomy_hierarchy_is_fresh()

Fixes #22526, #14485.

[kovshenin]

r27101 introduces a bunch of new tax_last_changed cache groups, and each one will most likely have a single key. Perhaps we can use the existing terms group to hold that? In get_taxonomy_last_changed you can use wp_cache_add() instead of _set() which I believe can be faster on some cache backends.

Also, this seems to introduce quite a lot of new stuff, all of it for the little bug we're trying to fix. Why can't we just use clean_object_term_cache like we do in wp_delete_term? See 22526.2.diff​, it passes the unit test added in r27099.

[wonderboymusic]

You are correct about the terms bucket, did that here: 22526.3.diff​. Not using _set requires _delete and _add, because every falsely value needs to be overwritten. Will look at your other fix now.

[wonderboymusic]

I applied your patch and turned off my checks in get_object_term_cache() and _get_term_hierarchy(). Unit tests fail for both. Is .2.diff additional or a replacement?

[wonderboymusic]

After thinking about it, I don't think delete/add is an upgrade from set. I can see the benefit of add when your goal is "I want something in the cache, no matter what, so as long as its there, we're good" but set is more appropriate for "the value needs to be overwritten, no matter what."

Also, for cleanliness' sake, I can see the reason to use the terms bucket, so I will probably make that change, but WP doesn't give you any way to invalidate by group, so the change is really arbitrary.

[wonderboymusic]

In 27142:

Reuse the terms cache group for taxonomy cache invalidation.

See #22526, #14485, [27101], [27102].

[kovshenin]

Sorry, re add vs set, I was only referring to the get_taxonomy_last_changed() function. You already check $last_changed. If that's false, then the value is not in cache, because microtime() can not return false, so you don't really need to delete.

[kovshenin]

Re .2.diff, it's only a fix for this ticket and you'll need to revert your changesets before applying it. I added 22526.4.diff​ which reverts (or partially reverts) r27142, r27141, r27102 and r27101, adds the fix in .2.diff:

$ phpunit --group 22526
OK (1 test, 2 assertions)

This also reverts the fix in #14485, I'll reopen it and add a new patch with a simpler fix.

[wonderboymusic]

Uh, I just applied your patch and did the following:

1) define( 'WP_TESTS_FORCE_KNOWN_BUGS', true );
2) phpunit tests/phpunit/tests/term/cache.php

Results:
Tests: 3, Assertions: 8, Failures: 2.

[wonderboymusic]

In 27163:

Partially revert [27101], [27102], [27141], and [27142]. Those commits introduced new functions to sync up cache invalidation events. The current commit alters existing internals.

"The cache invalidation with static was introduced in r9102 with version 2.7. Multisite wasn't in core back then, so something like switch_to_blog() wasn't a concern, but now it breaks if you switch the blog in between calls to clean_term_cache."

This solution is simpler. All unit tests pass. Removes unnecessary tests linked to removed functions.

Props kovshenin.
Fixes #14485, #22526.