Opened 8 years ago
Last modified 2 years ago
#37762 assigned defect (bug)
cache_results parameter doesn't prevent queried posts from being added to cache
Reported by: | Dang Vu | Owned by: | boonebgorges |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.6 |
Component: | Query | Keywords: | needs-patch dev-feedback |
Focuses: | performance | Cc: |
Description
Even when cache_results
is set to false
, the queried posts in WP_Query
are still mapped to get_post()
which will always add post instance to cache.
For more info: http://wordpress.stackexchange.com/questions/236653/how-to-prevent-queried-posts-from-being-added-to-cache/236659.
Change History (10)
This ticket was mentioned in Slack in #core by mark-k. View the logs.
8 years ago
#4
in reply to:
↑ 3
@
8 years ago
Replying to achbed:
What's interesting is that the individual posts are loaded and cached to the object cache individually, but the combined query result is what is controlled via the
cache_results
parameter. This seems to meet the defined parameter intent because it's not caching the full result of the query. If the posts themselves change enough that you can't use object caching I'd be truly surprised.
If you want to prevent the posts themselves from caching, that appears to be a different request and intent. Maybe an additional
cache_posts
parameter to control that piece of the caching puzzle?
We should update the docs at the very least to explain this behavior.
You're right! The usage of the cache_results
parameter is unclear. A ticket for cache_posts
parameter looks promising.
Honestly, I don't understand why WordPress needs to add queried posts to both the WP_Query::posts
and the cache since they only exist in a single page load.
#5
follow-ups:
↓ 6
↓ 8
@
8 years ago
- Keywords reporter-feedback added
- Owner set to boonebgorges
- Status changed from new to assigned
Hi @Dang Vu - Thanks for the ticket.
Honestly, I don't understand why WordPress needs to add queried posts to both the WP_Query::posts and the cache since they only exist in a single page load.
If you are running a persistent object cache like Memcached, then the cached post objects are available across page loads. Even within a single pageload and without a persistent cache, other functions in WP fetch data from the 'posts'
cache group. WP_Query::posts
is not intended to be a cache, but is a list of query results.
cache_results
is fairly poorly named. Introduced in [14310], it really means "populate all caches related to query results". This includes the 'posts' cache as well as the taxonomy and metadata caches. See update_post_caches()
. In [14665], cache_results
was set to false
when a persistent object cache is in use, because the repeated (unnecessary) round trips to Memcached were causing performance issues; see #12611. The posts
array in WP_Query
began to touch the 'posts' cache in [21559], which made cache_results
more ambiguous: it now determines whether metadata and taxonomy should automatically be loaded into the cache, rather than the post objects. And this latter fact is confused by the fact that we have separate update_post_meta_cache
and update_post_term_cache
params.
From this, I conclude that cache_results
doesn't really do what it was meant to do. If anything, it should be removed altogether.
What is the reason for wanting to prevent post objects from being loaded into the cache? Is it a performance issue like described in #12611? If so, I wonder if we could mostly mitigate the issue with cache multi-get and multi-set, which would dramatically reduce the number of round-trips necessary to load data into the cache; see #20875.
#6
in reply to:
↑ 5
@
8 years ago
Replying to boonebgorges:
From this, I conclude that
cache_results
doesn't really do what it was meant to do. If anything, it should be removed altogether.
Are there any backwards-compatible reasons to not remove/ignore the parameter? The only thing I can think of is a slight performance hit if we're now no longer updating the metadata and/or taxonomy caches. I think that's acceptable.
What is the reason for wanting to prevent post objects from being loaded into the cache? Is it a performance issue like described in #12611? If so, I wonder if we could mostly mitigate the issue with cache multi-get and multi-set, which would dramatically reduce the number of round-trips necessary to load data into the cache; see #20875.
The only reason I can think of is if the post includes some data that changes very often (for example - some plugin is attaching random or API data to a post at load time). In that case they're probably doing_it_wrong()
and should be discouraged anyway so that we can use the cache properly.
#7
@
8 years ago
- Keywords reporter-feedback removed
- Milestone changed from Awaiting Review to Future Release
Are there any backwards-compatible reasons to not remove/ignore the parameter? The only thing I can think of is a slight performance hit if we're now no longer updating the metadata and/or taxonomy caches. I think that's acceptable.
Currently, cache_results
(when used properly alongside update_post_meta_cache
and update_post_term_cache
) prevents at least two additional cache roundtrips for each fetched post, and possibly many more in the case of lots of custom taxonomies. Removing the option altogether could cause serious performance problems in certain cases. Once #20875 is addressed, I think we can revisit the removal of cache_results
.
The only reason I can think of is if the post includes some data that changes very often (for example - some plugin is attaching random or API data to a post at load time). In that case they're probably doing_it_wrong() and should be discouraged anyway so that we can use the cache properly.
Yeah, filtering the post data pre-cache could cause problems, but I think that in this case the solution is to filter *post* cache. The frequency of updates is an argument *for* better cache leverage, not against it; if there are inconsistency issues in these cases, it's likely due to an invalidation bug, rather than the fact that the cache is being used in the first place.
I'm going to move this ticket to Future Release so it can be reassessed post-#20875.
#10
@
2 years ago
- Keywords needs-patch dev-feedback added
Hi there!
This issue still happens if we use the ticket author's suggested code snippet.
<?php echo count($wp_object_cache->cache['posts']); $query = new \WP_Query([ 'post_type' => 'post', 'cache_results' => false, 'posts_per_page' => 5, 'ignore_sticky_posts' => true ]); echo '<br>' . count($wp_object_cache->cache['posts']);
dev-feedback
added so we get more eyes on this issue and someone from core time will tackle this.
What's interesting is that the individual posts are loaded and cached to the object cache individually, but the combined query result is what is controlled via the
cache_results
parameter. This seems to meet the defined parameter intent because it's not caching the full result of the query. If the posts themselves change enough that you can't use object caching I'd be truly surprised.If you want to prevent the posts themselves from caching, that appears to be a different request and intent. Maybe an additional
cache_posts
parameter to control that piece of the caching puzzle?We should update the docs at the very least to explain this behavior.