#54511 closed enhancement (fixed)
Remove cache time limitation in WP_Term_Query
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.0 | Priority: | normal |
Severity: | normal | Version: | 4.6 |
Component: | Taxonomy | Keywords: | has-patch has-dev-note |
Focuses: | performance | Cc: |
Description (last modified by )
Currently there is a limitation, that term caches found in WP_Term_Query
will only last one day. See the following snippet.
wp_cache_add( $cache_key, $terms, 'terms', DAY_IN_SECONDS );
If we can ensure that cache invalidation is happening correctly, this limitation can be removed. Once this limitation is removed, cache will live until their are correctly invalidation.
Change History (17)
#3
@
2 years ago
If you'll forgive me, I would be very appreciative if the enhancement proposed in #52710 could be considered alongside this one.
#4
@
2 years ago
I can see the original argument for filling up the cache unnecessarily, but not for invalidation purposes.
+1
This ticket was mentioned in PR #2162 on WordPress/wordpress-develop by spacedmonkey.
23 months ago
#5
- Keywords has-patch added
Trac ticket: https://core.trac.wordpress.org/ticket/54511
#6
@
23 months ago
- Keywords needs-unit-tests added
I have created a patch at #2162, ready for review.
This ticket was mentioned in PR #2196 on WordPress/wordpress-develop by spacedmonkey.
23 months ago
#7
Trac ticket: https://core.trac.wordpress.org/ticket/54511
spacedmonkey commented on PR #2162:
23 months ago
#8
Closed in favour https://github.com/WordPress/wordpress-develop/pull/2196
#9
@
22 months ago
Originally added in https://core.trac.wordpress.org/ticket/35381 - where the day cache was also questioned at the time. Based on the response, the reasons for keeping it seemed week and open for reconsideration. +1 to removing and writing a dev note explaining why.
cc: @boonebgorges hi friend, we are touching your code, feedback welcome!
#10
@
22 months ago
Thanks for the ping!
Looking back at https://core.trac.wordpress.org/ticket/35381#comment:13, I see that we simply inherited this TTL strategy from [15583]. As such, I'm +1 for the change proposed here.
#11
@
22 months ago
- Keywords needs-unit-tests removed
- Milestone changed from Awaiting Review to 6.0
@boonebgorges @adamsilverstein
Are you happy for me to commit this change then?
#12
follow-up:
↓ 13
@
22 months ago
@boonebgorges @adamsilverstein
Are you happy for me to commit this change then?
#13
in reply to:
↑ 12
@
22 months ago
Replying to spacedmonkey:
@boonebgorges @adamsilverstein
Are you happy for me to commit this change then?
yes, +1 go for it!
I don't recall why this was introduced. Maybe there are some limitations in the current cache invalidation for term queries, and this was implemented to more or less cater for that. Or it could have been to not pollute the object cache, since those term query hashes used for cache keys can frequently change.
Anyway, since this isn't present in other object type query implementations, at a high level I agree this should be removed at some point. However, we should make sure it isn't indeed there for a reason where removing it could result in a regression.