Skip to:
Content

Opened 3 years ago

Closed 2 years ago

#6370 closed enhancement (fixed)

Blogs: Improvements to bp_blogs_record_existing_blogs()

Reported by: r-a-y Owned by: r-a-y
Milestone: 2.6 Priority: normal
Severity: normal Version: 2.1
Component: Blogs Keywords: dev-feedback needs-testing has-patch
Cc:

Description (last modified by r-a-y)

bp_blogs_record_existing_blogs() doesn't really do a good job with recording existing blogs especially when the "Repopulate blogs records" option is used from the "Tools > BuddyPress" admin page.

Attached patch improves the function by:

  • Supporting 'limit' and 'offset' in the initial blog query.
  • Saving the offset just in case the function hangs so the function can be resumed from that offset on the next run.
  • Supporting passing selected blog IDs in case a dev wants to re-record specific blogs
  • Fixing blog count caches by using bp_blogs_clear_blog_object_cache()
  • Giving each blog a better last activity timestamp by using the last_updated column from the wp_blogs column

Attachments (3)

6370.01.patch (5.8 KB) - added by r-a-y 3 years ago.
6370.02.patch (7.8 KB) - added by r-a-y 3 years ago.
6370.03.patch (8.1 KB) - added by r-a-y 2 years ago.

Download all attachments as: .zip

Change History (16)

@r-a-y
3 years ago

#1 @r-a-y
3 years ago

  • Description modified (diff)

#2 @r-a-y
3 years ago

  • Milestone changed from 2.3 to 2.4

#3 @DJPaul
3 years ago

  • Keywords dev-feedback needs-testing added

@r-a-y
3 years ago

#4 @r-a-y
3 years ago

Patch refreshed for trunk and adds a unit test, plus an admin notice if there are more blogs left to record.

#5 @DJPaul
3 years ago

I don't have a multisite to test this. Code review of the patch:

  • bp-core-admin-tools.php: use get_current_screen() to get the screen ID rather than look in the global?
  • Same file: I'd suggest the message more like "blog recording in progress, but something went wrong, so click some button here to start it again".
  • Same file: $blogs_offset only used once, remove the variable.

Nothing else major.

This needs to land in trunk very, very soon I think to make 2.4. ;)

#6 @DJPaul
3 years ago

  • Milestone changed from 2.4 to 2.5

#7 @DJPaul
3 years ago

  • Keywords commit added

Let's commit this early so we can get more eyes on it for testing during 2.5.

#8 @r-a-y
2 years ago

  • Keywords needs-refresh added; has-patch commit removed
  • Milestone changed from 2.5 to 2.6

Moving to 2.6.

There was a reported issue importing the blog's last activity timestamp by using the 'wp_blogs' DB table's 'last_updated'` column on an older multisite install. Need to check if the value is empty before using it.

#9 @r-a-y
2 years ago

  • Keywords has-patch added; needs-refresh removed

03.patch is refreshed and includes a few other changes:

  • Fix when setting a timestamp for a blog's last activity (comment:8)
  • Paul's recommendations from comment:5.
  • If the WP install is single site, we only record the blog for the current user only. See also comment:6:ticket:6940 for more info. Recording one blog allows us to populate some blog meta that is required by activity tracking.
  • When fetching users with get_users() use array( 'fields' => 'ID' ). Since we only need the user ID, there is no need for us to fetch the entire WP_User object for all users. This saves us some memory.

@r-a-y
2 years ago

#10 @johnjamesjacoby
2 years ago

These patches are all very much better than what BuddyPress has had. Nice work so far!

One of BuddyPress's selling points and strengths is that it helps scale data across large installations. With that thinking in mind, places where it doesn't should be with great intention, non-fatal, and maybe go as far as providing a function/script that can easily be ran by a site-admin when the server environment allows for it (longer timeouts, more RAM, etc...)

One other concern with these queries is setups with multiple database servers, where global tables may not be on the same hardware as per-site or users tables. I'll need to look at these more, but it's possible we'll need to unjoin them into separate queries.

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


2 years ago

This ticket was mentioned in Slack in #buddypress by dcavins. View the logs.


2 years ago

#13 @r-a-y
2 years ago

  • Owner set to r-a-y
  • Resolution set to fixed
  • Status changed from new to closed

In 10815:

Blogs: Improve how bp_blogs_record_existing_blogs() operates.

When the "Repopulate blogs records" is used from the "Tools > BuddyPress"
page, on large site installs, this option isn't very effective.

This commit improves bp_blogs_record_existing_blogs() by:

  • Recording an offset so the blog record repopulation can be resumed on failure.
  • Only recording the root blog for the current user if run from single site. (See #6940.)
  • Fixing issues with blog count caches.
  • Giving each blog a better last activity timestamp by using the last_updated column from the wp_blogs DB table.
  • Supporting passing selected blog IDs in case a dev wants to re-record specific blogs.

Fixes #6370.

Note: See TracTickets for help on using tickets.