Make WordPress Core

Opened 8 years ago

Last modified 7 weeks ago

#15565 reopened enhancement

More context for clean_post_cache()

Reported by: mdawaffe Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 3.1
Component: Cache API Keywords: needs-patch, dev-feedback, bulk-reopened
Focuses: Cc:


I'd like more context to be available when the clean_post_cache hook is run.

Scenario: I have a plugin caches the post_IDs of most post queries that go through WP_Query. Invalidating that cache is done via the clean_post_cache hook, but requires a bunch of fragile hacks to prevent cache invalidation for things like comment inserts, which update the post's comment_count (which could, in theory, affect a WP_Query, but that's another story).

Option 1: Add extra actions to provide context. This is the simpler option. Patch 1 does this for the above scenario.

Option 2: Add an optional context parameter to clean_post_cache(). This is more general, but I can't think of anyplace else WordPress uses as similar approach. Patch two.

Attachments (3)

15565.1.diff (597 bytes) - added by mdawaffe 8 years ago.
Option 1
15565.2.diff (2.3 KB) - added by mdawaffe 8 years ago.
Option 2
15565.diff (1.7 KB) - added by wonderboymusic 6 years ago.

Download all attachments as: .zip

Change History (18)

8 years ago

Option 1

8 years ago

Option 2

#1 @mdawaffe
8 years ago

Option 2 also provides a way to detect if clean_post_cache() is being run recursively, another case that the scenario I mentioned above has to hack around.

#2 @scribu
8 years ago

We have a $context arg for escaping functions like sanitize_term(), but it's just a string.

#3 @westi
8 years ago

  • Keywords 3.2-early added
  • Milestone changed from Awaiting Review to Future Release

Option 2 looks good.

We should add context to all call to clean_post_cache so that a caching plugin can always know what is going on.

#4 @batmoo
7 years ago

  • Cc batmoo@… added

#5 @johnjamesjacoby
7 years ago

Note: this would be great for bbPress and anywhere custom post-types are being used creatively. If you invalidate tens of thousands of child-post caches, you're going to have a bad time.

#6 @nacin
7 years ago

I like this in theory. What would all of the contexts look like, for the various uses of clean_post_cache()?

#7 @nacin
7 years ago

  • Type changed from feature request to enhancement

#8 @jkudish
7 years ago

related: #11399

#9 @ryan
7 years ago

Related: #22176

6 years ago

#10 @wonderboymusic
6 years ago

  • Milestone changed from Future Release to 3.7

This may no longer be a thing since nacin fixed clean_post_cache() in 3.5, but I refreshed the patch against trunk - let's make a decision on it in 3.7

#11 @nacin
6 years ago

It might still be helpful to include a context for some uses of clean_post_cache(), but it no longer needs to be an array as it is no longer called in recursive fashion.

It seems like the only context you may wish to know is comment_count. Other kinds of cache clears (post and attachment CRUD, set_post_type(), publishing a post) should really not be messed with. Even post reassignments in wp_delete_user() should probably be cleared, though those have the potential to be expensive so that might be the only other situation where context is desired.

#12 @nacin
6 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone changed from 3.7 to Future Release

#13 @chriscct7
4 years ago

  • Keywords dev-feedback added; 3.2-early removed

#14 @iseulde
3 months ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

This ticket has not seen any activity in over *two* years, so I'm closing it as "wontfix".

The ticket may lack decisiveness, may have become irrelevant, or may not have gathered enough interest.

If you think this ticket does deserve some attention again, feel free to reopen.

For bugs, it would be great if you could provide updated steps to reproduce against the latest version of WordPress (5.0.2 at the time of writing). Remember images or a video can be superior to explain a problem. At the very least, quickly test again to make sure the bug still exists.

If it’s an enhancement or feature, some extra motivation may help.

Thank you for your contributions to WordPress! <3

#15 @JeffPaul
7 weeks ago

  • Keywords bulk-reopened added
  • Milestone set to Awaiting Review
  • Resolution wontfix deleted
  • Status changed from closed to reopened

A decision was made to reopen tickets that were closed in the bulk edit that this ticket was affected by. This ticket is being placed back into the Awaiting Review milestone so it can be individually evaluated and verified to determine if it is still relevant/valid or reproducible.

Note: See TracTickets for help on using tickets.