Make WordPress Core

Opened 7 years ago

Last modified 3 weeks ago

#42280 accepted defect (bug)

Multisite: get_blogs_of_user has no useful caching on large installs

Reported by: westi's profile westi Owned by: pbearne's profile pbearne
Milestone: Awaiting Review Priority: normal
Severity: major Version: 4.7
Component: Networks and Sites Keywords: 2nd-opinion
Focuses: multisite, performance Cc:

Description

While thinking about the changes that were happening in #40228 we started to investigate other places where functions had been switched to use get_sites.

Looking back through "recent" changes I found: [38682] / #37061.

Previously we would fetch all the user meta for a user and then use get_blog_details (and it’s inherent caching) to populate the list of blogs.

Now we fetch all the user meta for a user and then create a query for get_sites which as previously discussed has pointless caching on active multisites inside WP_Site_Query.

I'm opening this ticket to ensure that the caching is also reviewed here too.

Change History (10)

#1 follow-up: @flixos90
7 years ago

  • Keywords 2nd-opinion added

@westi We are currently looking into improving caching in WP_Site_Query so that it is less aggressively invalidated for certain queries. By making changes to WP_Site_Query, we would ensure that all functions using get_sites() would get these benefits. #42252 is the ticket where this is being worked on, if you are interested in that.

I'm not opposed to adding special caches to certain functions that require more unique lookups though. For the case of get_blogs_of_user(), it may likely be the best option to make it work closer to what it was like before:

  • Upon having all the site IDs, call _prime_site_caches() for it in order to not require countless DB queries.
  • Then, for each site ID, call get_site() and build the array with it.

#2 in reply to: ↑ 1 @westi
7 years ago

Replying to flixos90:

@westi We are currently looking into improving caching in WP_Site_Query so that it is less aggressively invalidated for certain queries. By making changes to WP_Site_Query, we would ensure that all functions using get_sites() would get these benefits. #42252 is the ticket where this is being worked on, if you are interested in that.

Great, I'm trying to document specific cases I find where switching to get_sites seems to have caused a significant regression in cache performance so as to make sure each case can be reviewed for a better solution.

I'm not opposed to adding special caches to certain functions that require more unique lookups though. For the case of get_blogs_of_user(), it may likely be the best option to make it work closer to what it was like before:

  • Upon having all the site IDs, call _prime_site_caches() for it in order to not require countless DB queries.
  • Then, for each site ID, call get_site() and build the array with it.

I think that is probably a great solution, for now I have just reverted us back to using get_blog_details but happy to test out a patch for a better solution and see how it performs in our use cases.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


7 years ago

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


2 years ago

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


2 years ago

This ticket was mentioned in Slack in #hosting-community by javier. View the logs.


2 years ago

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


2 years ago

#8 @flixos90
15 months ago

  • Priority changed from high to normal

Given that this ticket hasn't seen any update in 6 years, it's probably not justified to have "high" priority.

#9 @pbearne
3 weeks ago

looking in the current code we now have

<?php
                // Prime site network caches.
                if ( $this->query_vars['update_site_cache'] ) {
                        _prime_site_caches( $site_ids, false );
                }

Does this fix the issue?

#10 @pbearne
3 weeks ago

  • Owner set to pbearne
  • Status changed from new to accepted
Note: See TracTickets for help on using tickets.