Skip to content

Move the counts cache uses to a persistent-counts group, using last_changed - #3748

Open
dd32 wants to merge 15 commits into
WordPress:trunkfrom
dd32:try/persistent-count-group
Open

Move the counts cache uses to a persistent-counts group, using last_changed#3748
dd32 wants to merge 15 commits into
WordPress:trunkfrom
dd32:try/persistent-count-group

Conversation

@dd32

@dd32dd32 commented Dec 14, 2022

Copy link
Copy Markdown
Member

Trac ticket: https://core.trac.wordpress.org/ticket/35430

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

dd32 added 12 commits December 14, 2022 15:56
@dd32
Make the wp_count_posts() cache key flushable upon post changes.
1111830
@dd32
Remove the 'counts' group from being non-persistent.
bae2bf5
@dd32
Move get_adjacent_post() from using the 'counts' group to it's own ca…
6a4aaae
…che-group.
@dd32
Use wp_cache_get_last_changed('comments') to flush the comments count…
c42fa24
… caches.
@dd32
Clear the Privacy table counts based on the posts last-changed.
6844bac
@dd32
The 'counts' group is now persistent.
1b5d0c8
@dd32
Bucket the wp_count_attachments() cache on the posts last-changed date.
4bd0f87
@dd32
Persistently cache get_adjacent_post() and clear it with the posts la…
3acd044
…st_modified date. Partially reverts 6a4aaae.
@dd32
Clean the comment cache upon Trashing/Untrashing/Spamming/Unspamming …
2bec4e1
…a comment.
@dd32
Tests: wp_count_attachments() Generate the expected cache key after t…
eed810a
…he attachments are inserted.
@dd32
Fix a typo inc42fa24, the last_changed group is plural for posts, bu…
5004f8e
…t singular for comment.
@dd32
Move the last_changed in wp_update_comment_count_now() to a more appr…
17fb0d8
…opriate location.
@dd32
dd32 marked this pull request as ready for review December 14, 2022 07:05
peterwilsoncc
peterwilsoncc reviewed Dec 16, 2022
View reviewed changes

@peterwilsonccpeterwilsoncc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few notes inline.

For counts that are moved to persistent caches, some unit tests would be dandy to ensure that the cache clears as expected.

Comment threadsrc/wp-admin/includes/class-wp-privacy-requests-table.php Outdated
Comment threadsrc/wp-includes/link-template.php Outdated
Comment threadsrc/wp-includes/ms-blogs.php Outdated
@dd32

dd32 commented Dec 19, 2022

Copy link
Copy Markdown
MemberAuthor

For counts that are moved to persistent caches, some unit tests would be dandy to ensure that the cache clears as expected.

There's some existing tests that cover some of these, which is the purpose of the myriad of clean_comment_cache() calls that were added to clean comment caches (The existing unit tests failed after the change).

I agree though, that additional unit tests are needed.

@dd32dd32 changed the title Move the counts cache group to a persistent group, through the usage of last_changedMove the counts cache uses to a persistent-counts group, using last_changedDec 19, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peterwilsonccpeterwilsonccpeterwilsoncc left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dd32@peterwilsoncc