Opened 8 years ago
Last modified 5 weeks ago
#40362 new defect (bug)
Remove `blog-id-cache` cache group
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | 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 (16)
This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.
8 years ago
This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.
8 years ago
#5
follow-up:
↓ 13
@
8 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.
8 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.
6 weeks ago
This ticket was mentioned in Slack in #core-multisite by heikomamerow. View the logs.
5 weeks ago
This ticket was mentioned in PR #8908 on WordPress/wordpress-develop by @Heiko_Mamerow.
5 weeks ago
#10
- Keywords has-unit-tests added
#13
in reply to:
↑ 5
;
follow-up:
↓ 14
@
5 weeks 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_Query
caches are invalidated on every change to any site (vialast_changed
), whileblog-id-cache
keys are only invalidated on updates to the specific site. So theblog-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.
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
@
5 weeks 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_Query
caches are invalidated on every change to any site (vialast_changed
), whileblog-id-cache
keys are only invalidated on updates to the specific site. So theblog-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.
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()
.
#15
in reply to:
↑ 14
@
5 weeks 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?
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.