Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#33875 closed defect (bug) (fixed)

Comment caching could be improved

Reported by: wonderboymusic Owned by:
Milestone: 4.4 Priority: normal
Severity: normal Version:
Component: Comments Keywords: has-patch
Focuses: Cc:

Description

There are some caches that are never updated properly, etc.

Attachments (1)

33875.diff (2.8 KB) - added by wonderboymusic 6 years ago.

Download all attachments as: .zip

Change History (8)

@wonderboymusic
6 years ago

#1 follow-up: @wonderboymusic
6 years ago

In 34131:

The "counts" cache for comments by post id is never invalidated. Neither wp_update_comment_count() nor wp_update_comment_count_now() touch the cache.

Adds unit test.
See #33875.

#2 @wonderboymusic
6 years ago

In 34132:

In the edit-comments.php admin handler, toggle wp_defer_comment_counting() so that only unique post IDs have their comment count updated. Currently, if you delete 50 comments from the same post, the count would get reset 50 times. Not joking.

See #33875.

#3 @wonderboymusic
6 years ago

In 34136:

Ensure that the count cache for all is in sync with comment-{$post_id} values. These are the values most often relied-upon by the list table for comments.

Adds unit tests.

See #33875.

#4 @wonderboymusic
6 years ago

In 34158:

More bonkers comment cache cleanup: toggle wp_defer_comment_counting() in wp_insert_post() and wp_insert_attachment(). Move the cache deletion in wp_update_comment_count_now() to before the get_post() call, so that the caches get deleted even if the post has already been deleted and the function returns early.

See #33875.

#5 in reply to: ↑ 1 @westi
6 years ago

Replying to wonderboymusic:

In 34131:

The "counts" cache for comments by post id is never invalidated. Neither wp_update_comment_count() nor wp_update_comment_count_now() touch the cache.

Adds unit test.
See #33875.

I thought that these counts were non-persistent and that was why we didn't both invalidating the cache because it only lasted for the current page load.

#6 @wonderboymusic
6 years ago

The List Table for comments is one huge race condition and passes the total back and forth via AJAX without ever checking that it is correct. Even if it's not persistent, it can be invalidated on destructive actions. The use-case here is the person looking at 20 pending comments and approving them one-by-one, or sending a list of comments from spam to trash one-at-a-time

#7 @wonderboymusic
6 years ago

  • Resolution set to fixed
  • Status changed from new to closed

I attacked the things I intended - there are other tickets for the other cache things that need attention

Note: See TracTickets for help on using tickets.