wp_suspend_cache_additions() doesn't block term_relationships being added to cache

[leewillis77]

1. Register a custom taxonomy, and create one or more posts associated with this taxonomy
2. Place the attached code in your theme's functions.php [Tweak the args to get_posts accordingly to retrieve the posts created in (1)]
3. Load your blog and inspect your error log.

The output should confirm that while wp_suspend_cache_additions() blocks most cache additions, cache entries are still added for {$taxonomy}_relationships. E.g. in my case, the output is:

[29-Dec-2011 22:49:06] --------------
[29-Dec-2011 22:49:06] options - 6 items
[29-Dec-2011 22:49:06] default - 1 items
[29-Dec-2011 22:49:06] users - 1 items
[29-Dec-2011 22:49:06] userlogins - 1 items
[29-Dec-2011 22:49:06] useremail - 1 items
[29-Dec-2011 22:49:06] userslugs - 1 items
[29-Dec-2011 22:49:06] user_meta - 1 items
[29-Dec-2011 22:49:06] --------------
[29-Dec-2011 22:49:06] --------------
[29-Dec-2011 22:49:06] options - 6 items
[29-Dec-2011 22:49:06] default - 1 items
[29-Dec-2011 22:49:06] users - 1 items
[29-Dec-2011 22:49:06] userlogins - 1 items
[29-Dec-2011 22:49:06] useremail - 1 items
[29-Dec-2011 22:49:06] userslugs - 1 items
[29-Dec-2011 22:49:06] user_meta - 1 items
[29-Dec-2011 22:49:06] wpsc_product_category_relationships - 50 items
[29-Dec-2011 22:49:06] product_tag_relationships - 50 items
[29-Dec-2011 22:49:06] wpsc-variation_relationships - 50 items
[29-Dec-2011 22:49:06] wpec_product_option_relationships - 50 items
[29-Dec-2011 22:49:06] --------------

As you can see entries have been added to the cache for the various {taxonomy}_relationships.

This appears to be either because the call to wp_cache_set inside update_object_term_cache() in taxonomy.php should be wp_cache_add - and/or wp_cache_set should check the value of wp_suspend_cache_additions() before adding to the cache.

Given that update_object_term_cache declares in its documentation that it "Will only update the cache for terms not already cached.", the attached patch amends it to call wp_cache_add instead of wp_cache_set. On reading the code this appears to make sense.

On reflection - I suspect the title of this bug is inaccurate - I believe this affects all taxonomies, not just custom ones.

My initial observation that only custom taxonomies had their relationships inserted into the cache was I believe because the posts I was querying were limited to ones which only had custom taxonomies.

I'll do some testing to confirm when I'm back at a suitable location, and update either way. but just wanted to note it here on the off-chance someone looks at this in the meantime.

Revised reproduction details

Confirmed that this affects all taxonomies, not just custom taxonomies. Steps to reproduce therefore are just:

1. Place the attached code (test2.php) in your theme's functions.php
2. Load your blog and then inspect your error log.

The results should show that entries have been added to the cache for category_relationships, post_tag_relationships, and post_format_relationships despite the call to wp_suspend_cache_addition. Proposed patch is still the same - just clarifying the affected taxonomies.

Thanks for the review - is there anything we can do to bring this in before 3.4? This (together with #19708) are preventing some pretty significant performance wins in a number of projects, and it'd be great to get them in core sooner rather than later.

Happy to do whatever's needed to help with that...

Thanks again.

Why is wp_suspend_cache_additions() being used? Currently, its role is limited to reducing cache activity during imports, where we know we are doing strict adds for new data. It is not safe to use in most situations.

That's been using wp_cache_set() since it was introduced in [6243]. It could probably change to add, but plugin use of update_object_term_cache() would have to be investigated.

BTW, get_posts() has a cache_results argument specifically to disable updating these caches.

Core calls update_object_term_cache() strictly to prime caches, not propagate changes. Any plugin using this function to propagate changes to a post's relationships to the cache must be doing very naughty things like bare SQL queries. In general, we prefer the delete/add model to the set model. So, the use of wp_cache_set() instead of wp_cache_add() here seems to be an unintended artifact.

Hi Ryan,

Thanks for your comments. The usage is in plugins that wish to iterate over all posts (For some reason). The specific use case that caused this is generating an XML feed of product information from e-Commerce stores for Google Merchant Centre. In this context you want to iterate over all posts, but there's no specific advantage in adding those posts (Or related information) to the cache.

Background here:  http://code.google.com/p/wp-e-commerce/issues/detail?id=885

My reading of the code in update_object_term_cache() is that the final bit will only be reached where the item doesn't already exist in the cache anyway - so changing to _add from _set should be realistically functionally equivalent?

Replying to leewillis77:

In this context you want to iterate over all posts, but there's no specific advantage in adding those posts (Or related information) to the cache.

Background here:  http://code.google.com/p/wp-e-commerce/issues/detail?id=885

As Ryan points out, the plugin should be setting cache_results to false for WP_Query.

My reading of the code in update_object_term_cache() is that the final bit will only be reached where the item doesn't already exist in the cache anyway - so changing to _add from _set should be realistically functionally equivalent?

Yes.

I wasn't aware of a cache_results option to get_posts, it's not in the codex, and a quick Google didn't return any obvious results - can you post a reference so I can compare that behaviour to this?

I found some *_relationships cache bugs while I was in there. Updated unit tests:  http://unit-tests.trac.wordpress.org/changeset/496

get_posts()/WP_Query has three cache control flags: cache_results, update_post_term_cache, and update_post_meta_cache. cache_results is a master flag that will turn off the other two. If an object cache backend such as memcached is used, the flags are set to false by default since there is no need to update the cache every page load when a persistent cache exists.

http://core.trac.wordpress.org/browser/trunk/wp-includes/query.php#L1967

In [19729]:

Use wp_cache_add() instead of wp_cache_set() when priming the object term cache in update_object_term_cache(). Pass the real post_type to clean_object_term_cache() instead of hard-coding post. Call clean_object_term_cache() from clean_bookmark_cache(). Props leewillis77. fixes #19690

@leewillis77 - see http://core.trac.wordpress.org/browser/tags/3.3.1/wp-includes/query.php#L1968.

Thanks Justin/Ryan/Andrew - I've updated the codex with the info on cache_results for others' reference.