Opened 4 years ago
Last modified 4 years ago
#52582 new defect (bug)
wp_cache_* duplicate/redundant storage and insufficient clearing of cache
Reported by: | sormano | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | normal | Version: | |
Component: | Comments | Keywords: | 2nd-opinion needs-testing |
Focuses: | performance | Cc: |
Description
This is something I'm currently encountering, but have not been able to fully investigate myself (yet).
Summarized backstory;
Using Redis, sometimes maxes memory > needs flush. Started investigating whats taking up soo much memory (relatively simple sites).
~42,000/67,000 records stored are for get_comment_child_ids
and get_comments
.
(By far not that many comments on my sites)
Looking for a single specific comment ID it is repeated between 30-50 times, while in theory it should be just once (unless using it in different contexts? (->query_vars))
Possible cause;
For the methods get_comments
and fill_descendants
the wp_cache_get_last_changed( 'comment' );
is used to create a cache key for the comments / childs.
https://core.trac.wordpress.org/browser/tags/5.6.1/src/wp-includes/class-wp-comment-query.php#L432
https://core.trac.wordpress.org/browser/tags/5.6.1/src/wp-includes/class-wp-comment-query.php#L998
When a new comment is inserted for example, there is a attempt to delete the comment cache by calling clean_comment_cache()
, but it seems this is based on the comment ID, which is not the same as the key / doesn't target the childs.
The 'last_changed' is however changed at the same time in that function;
https://core.trac.wordpress.org/browser/tags/5.6.1/src/wp-includes/comment.php#L3195
Which I think makes it impossible for the prior cached data to be found because it uses that in the key.
This causes it to store the same data over and over (redundant), even when it hasn't changed for those comments, with new cache keys without clearing the old ones (duplicate).
Could be I'm completely off, but wanted to get another pair of eyes on it. If whats described is actually happening it looks pretty major for caching efficiency.
Thanks for the report @sormano , and welcome.