Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#25178 closed defect (bug) (fixed)

multisite network admin to delete user attempts to load millions of users into memory

Reported by: _ck_'s profile _ck_ Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 3.8 Priority: normal
Severity: major Version: 3.1
Component: Multisite Keywords: has-patch commit
Focuses: Cc:

Description

There is an unbound mysql query in multisite network admin.

$blog_users = get_users( array( 'blog_id' => $details->userblog_id ) );  

around line 53 in wp-admin/network/users.php

If there are millions of users with wp_capabilities, it will attempt to load all of them into memory at once.

Then it attempts to do a one-by-one comparison in a php loop.

This entire section of code needs to be rewritten to simply load the users by id and simply do a LIKE '%\_capabilities' then compare their capabilities against the requested blog ids. Or even better since you know the blog ids and capabilities, just request the specific meta_key ie. wp_123_capabilities which would be way faster because it has an index.

Attachments (1)

users.php.patch (869 bytes) - added by rodrigosprimo 11 years ago.
Fetch only the required field in confirm_delete_users()

Download all attachments as: .zip

Change History (13)

#1 @DrewAPicture
11 years ago

Sounds like an excellent candidate for using wp_is_large_network( 'users' ), maybe with a chunked query if we really do need to load all users here.

#2 @duck_
11 years ago

#25184 was marked as a duplicate.

#3 @SergeyBiryukov
11 years ago

  • Milestone changed from Awaiting Review to 3.7
  • Version changed from 3.6 to 3.1

Related: [17084] (#15854)

ticket:25184:users.php.patch looks like an improvement, we could probably do that in 3.7 and look for further optimization in a future release.

#4 @rodrigosprimo
11 years ago

  • Cc rodrigosprimo@… added

#5 follow-up: @wonderboymusic
11 years ago

  • Milestone changed from 3.7 to 3.8

Unfortunately, no patch

#6 in reply to: ↑ 5 @rodrigosprimo
11 years ago

Replying to wonderboymusic:

Unfortunately, no patch

Have you checked ticket:25184:users.php.patch? Should I attach the same patch to this issue as well?

@rodrigosprimo
11 years ago

Fetch only the required field in confirm_delete_users()

#7 @rodrigosprimo
11 years ago

  • Keywords has-patch added

I added the patch originally attached to ticket:25184 here in case it makes the review process easier.

#8 @jeremyfelt
11 years ago

users.php.patch still applies to trunk and would be somewhat of a stopgap until we can come up with something.

The unfortunate part here is that this user query is done only to fill in a select menu for where to attribute a user's posts when they are deleted from a site. If a user is a member of multiple sites, a similar query may be done several times and multiple cumbersome select lists will be shown.

Beyond the query itself being huge, the result would be ridiculous to deal with if you really did want to select a new user for attribution.

I wonder if there's a way (especially with wp_is_large_network( 'users' ), to either hide this attribution option completely or to provide a different interface that made use of incremental search or something.

Suggest patching with users.php.patch in 3.8 and then pushing a larger decision around workflow to 3.9 or later.

#9 @SergeyBiryukov
11 years ago

  • Keywords commit added

Related: #19867

#10 @vmassuchetto
11 years ago

This bug affects some of the sites that I used to run and also the Brazilian WordPress community website that was recently spammed with tons of new users.

Today I had to apply this patch in production to make it possible to delete a bunch of users. It's a quite simple fix for a very important feature, and I'm failing to see why it wasn't integrated into core yet.

#11 @SergeyBiryukov
11 years ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 26499:

Fetch only the required fields in confirm_delete_users().

props rodrigosprimo.
fixes #25178.

Note: See TracTickets for help on using tickets.