WordPress.org

Make WordPress Core

Opened 7 months ago

Last modified 7 months 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.

Change History (2)

#1 @johnbillion
7 months ago

  • Component changed from General to Comments
  • Version trunk deleted

Thanks for the report @sormano , and welcome.

#2 @SergeyBiryukov
7 months ago

  • Focuses performance added
Note: See TracTickets for help on using tickets.