Context Navigation

← Previous Ticket
Next Ticket →

#33875 closed defect (bug) (fixed)

Comment caching could be improved

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

Description

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

Attachments (1)

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

Download all attachments as: .zip

Change History (8)

@wonderboymusic
11 years ago

Attachment 33875.diff added

#1 follow-up: ↓ 5 @wonderboymusic
11 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
11 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
11 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
11 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
11 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
11 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
11 years ago

Resolution → fixed
Status new → 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.

Download in other formats:

Make WordPress Core

Context Navigation

#33875 closed defect (bug) (fixed)

Comment caching could be improved

Description

Attachments (1)

Change History (8)

@wonderboymusic 11 years ago

#1 follow-up: ↓ 5 @wonderboymusic 11 years ago

#2 @wonderboymusic 11 years ago

#3 @wonderboymusic 11 years ago

#4 @wonderboymusic 11 years ago

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

#6 @wonderboymusic 11 years ago

#7 @wonderboymusic 11 years ago

Download in other formats:

@wonderboymusic
11 years ago

#1 follow-up: ↓ 5 @wonderboymusic
11 years ago

#2 @wonderboymusic
11 years ago

#3 @wonderboymusic
11 years ago

#4 @wonderboymusic
11 years ago

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

#6 @wonderboymusic
11 years ago

#7 @wonderboymusic
11 years ago