WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#11726 closed defect (bug) (duplicate)

wp-admin/users.php crashes on sites with large number of subscribers

Reported by: prettyboymp Owned by:
Milestone: Priority: normal
Severity: major Version: 2.9.1
Component: General Keywords: has-patch needs-testing
Focuses: Cc:

Description

Even though the results are being paged in display, the wp-admin/users.php calls get_users_of_blog() which returns all users. On a site setup with the default PHP memory limit of 32M and 86000+ users, the page crashes, disabling the ability to manage users.

I think the get_users_of_blog function should add an arguments parameter so places that use it, such as wp-admin/users.php, can apply paging and other search criteria as needed to limit the resource usage.

ie:

function get_users_of_blog($id = '', $args, &$tot_users) {}

Attachments (2)

11726.patch (1.5 KB) - added by prettyboymp 5 years ago.
11726.2.patch (1.5 KB) - added by prettyboymp 5 years ago.

Download all attachments as: .zip

Change History (15)

comment:1 @hakre5 years ago

  • Keywords reporter-feedback added

Can you please report which PHP version you are using?

It might be necessary to provide paging on the users page? A search is provided.

comment:2 @prettyboymp5 years ago

  • Keywords reporter-feedback removed

I've run into this on my dev machine and on the live server before the config was modified to up the memory limit. Locally I'm running PHP v5.2.11.

While looking at this more, I realized that the actual user list being display is retrieved using WP_User_Search{}. get_users_of_blog() is only being used to display the roles:

$role_links = array();
$avail_roles = array();
$users_of_blog = get_users_of_blog();
$total_users = count( $users_of_blog );
foreach ( (array) $users_of_blog as $b_user ) {
	$b_roles = unserialize($b_user->meta_value);
	foreach ( (array) $b_roles as $b_role => $val ) {
		if ( !isset($avail_roles[$b_role]) )
			$avail_roles[$b_role] = 0;
		$avail_roles[$b_role]++;
	}
}
unset($users_of_blog);

get_users_of_blog() should probably be deprecated in favor of WP_User_Search{} and places that need to get all users should probably switch to using WP_User_Search{} set limits to the amount of users in memory at once.

comment:3 @prettyboymp5 years ago

  • Summary changed from wp-admin/users.php crashes on sites with large number of subscribers to Update

I came across another issue where I'm unable to delete a user from this blog because the browser crashes while loading the list of 86000+ users to attribute posts and links to. I'm changing the description of this ticket to better show what needs to be done, which is to update all interfaces to better handle large lists of users to avoid crashes on both the server and client.

Actions I found that are potential cause for server or client crashes:
-Users listing page (pulls in all users just to get roles)
-Deleting a user (displays all users to client to map posts and links)

comment:4 @nacin5 years ago

  • Summary changed from Update to wp-admin/users.php crashes on sites with large number of subscribers

Restoring borked ticket summary/title.

@prettyboymp5 years ago

@prettyboymp5 years ago

comment:5 @prettyboymp5 years ago

  • Keywords has-patch needs-testing added

Patch added. Splitting it up into multiple queries isn't my favorite fix for this, but it is at least better than loading every registered user of the site into memory.

comment:6 @voyagerfan57615 years ago

  • Cc WordPress@… added

comment:7 @miqrogroove5 years ago

Just to update things here:

I've been approved to re-factor the entire users.php script under #11914. I will not be using the patch provided by prettyboymp because there is an opportunity here to address the strategy for role counting at a more fundamental level, even without schema changes.

I do not intend to post a separate patch for this ticket. I have the users.php file pulled all apart and it's full of debug stubs and personal commentary. I can only assure you that the patch for get_users_of_blog() eliminates a table join and also reduces the size and data type of the query result set.

The schema is prohibitively non-optimal for 106 users per blog, but the script re-factor will at least give you some room to grow.

comment:8 @miqrogroove5 years ago

Good news. I put an extra hour into benchmarking the MySQL code with time/memory trade-offs. I have a version now that uses no memory in PHP, and can approach 107 users per blog before blowing up the database server.

comment:9 @prettyboymp5 years ago

  • Milestone changed from Unassigned to 3.0

Sounds good to me. A refactoring of the Users class is definitely a better solution, but will it be done by the 3.0 cutoff?

If not, can we include my patch in the 3.0 release so that blogs with this many users don't have a broken/severely slow users page in the meantime?

comment:10 @miqrogroove5 years ago

I have a patch for the upcoming 3.0 beta posted in the other thread now. The best thing you can do is test the alpha and/or beta and offer feedback on that thread.

comment:11 @dd325 years ago

miqrogroove: Do you want to post some patches of your progress as you reach each milestone in your mind? They dont have to work 100% or be commit worthy, But it'd be interesting to watch, and potentially a good time for someone to chime in with some hidden functionality you may not be aware of (Or didnt expect)

comment:12 @miqrogroove5 years ago

The first several alphas of that patch didn't even work :P So, they were not milestones in my mind. I am of course open to feedback about hidden functionality.

comment:13 @miqrogroove5 years ago

  • Milestone 3.0 deleted
  • Resolution set to duplicate
  • Status changed from new to closed

#11914 has the more general description, showing problems in schema, flow of control, and SQL security in users.php. Please add comments there, and thanks for the info :)

Note: See TracTickets for help on using tickets.