WordPress.org

Make WordPress Core

Opened 8 months ago

Closed 5 months ago

Last modified 5 months ago

#25178 closed defect (bug) (fixed)

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

Reported by: _ck_ Owned by: 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 6 months ago.
Fetch only the required field in confirm_delete_users()

Download all attachments as: .zip

Change History (13)

comment:1 DrewAPicture8 months 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.

comment:2 duck_8 months ago

#25184 was marked as a duplicate.

comment:3 SergeyBiryukov8 months 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.

comment:4 rodrigosprimo8 months ago

  • Cc rodrigosprimo@… added

comment:5 follow-up: wonderboymusic7 months ago

  • Milestone changed from 3.7 to 3.8

Unfortunately, no patch

comment:6 in reply to: ↑ 5 rodrigosprimo7 months 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?

rodrigosprimo6 months ago

Fetch only the required field in confirm_delete_users()

comment:7 rodrigosprimo6 months ago

  • Keywords has-patch added

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

comment:8 jeremyfelt5 months 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.

comment:9 SergeyBiryukov5 months ago

  • Keywords commit added

Related: #19867

comment:10 vmassuchetto5 months 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.

comment:11 SergeyBiryukov5 months 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.