Opened 9 years ago
Last modified 4 weeks ago
#40362 accepted defect (bug)
Remove `blog-id-cache` cache group
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | Future Release | Priority: | normal |
| Severity: | normal | Version: | |
| Component: | Networks and Sites | Keywords: | has-patch 2nd-opinion has-unit-tests |
| Focuses: | multisite | Cc: |
Description
The function get_blog_id_from_url() is the only function that uses the old blog-id-cache group. Since the introduction of WP_Site_Query it's basically redundant in there though, since site query results are cached anyway.
Therefore I think we should remove the cache read and write actions in that function. We can then also remove the cache group completely (from clean_blog_cache() and registration as a "global group").
Attachments (1)
Change History (21)
This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.
9 years ago
This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.
9 years ago
#5
follow-up:
↓ 13
@
9 years ago
- Keywords 2nd-opinion added; dev-feedback removed
Per bug-scrub discussion:
Maybe we should hold off removing that cache, although it's redundant. That is because WP_Site_Query caches are invalidated on every change to any site (via last_changed), while blog-id-cache keys are only invalidated on updates to the specific site. So the blog-id-cache keys are much more "stable".
I'm not sure whether this is actually a reason to hold off, but we need to discuss and further investigate.
This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.
9 years ago
This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-multisite by realloc. View the logs.
9 months ago
This ticket was mentioned in Slack in #core-multisite by heikomamerow. View the logs.
9 months ago
This ticket was mentioned in PR #8908 on WordPress/wordpress-develop by @Heiko_Mamerow.
9 months ago
#10
- Keywords has-unit-tests added
#13
in reply to:
↑ 5
;
follow-up:
↓ 14
@
9 months ago
Replying to flixos90:
Per bug-scrub discussion:
Maybe we should hold off removing that cache, although it's redundant. That is because
WP_Site_Querycaches are invalidated on every change to any site (vialast_changed), whileblog-id-cachekeys are only invalidated on updates to the specific site. So theblog-id-cachekeys are much more "stable".
I'm not sure whether this is actually a reason to hold off, but we need to discuss and further investigate.
Link to original discussion: https://wordpress.slack.com/archives/C03BVB47S/p1491848375654869
last_changed cache invalidation is still an issue.
Merging this PR will cause increased churn, specifically because of wpmu_update_blogs_date(), by way of _update_blog_date_on_post_publish() and _update_blog_date_on_post_delete().
#14
in reply to:
↑ 13
;
follow-up:
↓ 15
@
9 months ago
Ok, see the point. Should we close the ticket then?
Replying to johnjamesjacoby:
Replying to flixos90:
Per bug-scrub discussion:
Maybe we should hold off removing that cache, although it's redundant. That is because
WP_Site_Querycaches are invalidated on every change to any site (vialast_changed), whileblog-id-cachekeys are only invalidated on updates to the specific site. So theblog-id-cachekeys are much more "stable".
I'm not sure whether this is actually a reason to hold off, but we need to discuss and further investigate.
Link to original discussion: https://wordpress.slack.com/archives/C03BVB47S/p1491848375654869
last_changedcache invalidation is still an issue.
Merging this PR will cause increased churn, specifically because of
wpmu_update_blogs_date(), by way of_update_blog_date_on_post_publish()and_update_blog_date_on_post_delete().
#15
in reply to:
↑ 14
@
9 months ago
Replying to heikomamerow:
Ok, see the point. Should we close the ticket then?
I reached out to Barry at WordPress.com to see what his thoughts are (and if our hunch is correct or not).
Let's leave this open until we hear back?
#16
@
4 months ago
- Milestone changed from Awaiting Review to 7.0
- Owner set to johnjamesjacoby
- Status changed from new to accepted
- Type changed from defect (bug) to task (blessed)
This ticket was mentioned in PR #10888 on WordPress/wordpress-develop by @mukesh27.
4 weeks ago
#17
Trac ticket: https://core.trac.wordpress.org/ticket/40362
The previous PR (#8908) was closed, so this PR proposes the same changes again for review.
## Use of AI Tools
#18
@
4 weeks ago
@johnjamesjacoby Have you get any feedback from Barry? Is this in your radar for 7.0?
Q: Why it moved to type task is there any specific reason behind that?
#19
@
4 weeks ago
@mukesh27 have not heard back.
I would like to revisit the last_changed cache churn now that items are salted via r60697.
I switched it to a Task because it isn’t a bug/defect, and it isn’t necessarily enhancing any existing functionality, but is work that has merit to complete. I don’t feel strongly about ticket organization though so feel free to adjust.
OK to revisit this in 7.1 IMO.
My patch removes uses of the group in core and in tests. I have removed the global group, as plugins may be using it for something.
I believe we should be getting rid of the all cache look ups that use domain / path. I believe most domain mapping solutions out there will break the cache invalidation.