WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#15854 closed defect (bug) (fixed)

get_users/WP_User_Query is not scalable

Reported by: duck_ Owned by:
Milestone: 3.1 Priority: highest omg bbq
Severity: blocker Version: 3.1
Component: General Keywords: has-patch commit
Focuses: Cc:

Description

Related: #15786, #14572

Use of get_users (and by extension WP_User_Query) is not scalable for sites with a large number of users. Fatal errors occur when caching all the resulting users to memory.

Example usage which causes fatal errors in 3.1 (no fatal error in 3.0.3 for a site with even more users):


And probably others, grep for get_users.

Patches for first to examples attached, but I can't help but feel this is just papering over the issue.

Attachments (3)

class-wp-xmlrpc-server.php.diff (776 bytes) - added by duck_ 4 years ago.
wp-admin.users.php.diff (1.3 KB) - added by duck_ 4 years ago.
15854.diff (3.4 KB) - added by scribu 4 years ago.
Deprecated get_users_of_blog()

Download all attachments as: .zip

Change History (19)

duck_4 years ago

comment:1 scribu4 years ago

(In [17010]) Use wp_dropdown_users() in the delete confirmation screen. Props duck_. See #15854

comment:2 scribu4 years ago

(In [17011]) Get only required fields in wp_getAuthors(). Props duck_. See #15854

comment:3 scribu4 years ago

I guess we should add a 'cache_results' parameter to WP_User_Query and set it to false by default.

comment:4 scribu4 years ago

(In [17012]) Get only required fields in populate_network(). See #15854

comment:5 scribu4 years ago

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

(In [17013]) Make WP_User_Query return regular objects by default. Fixes #15854

comment:6 westi4 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This change breaks backwards compat for get_users_of_blog

In 3.0 this joined on the usermeta table and returned and array of stdclass with the caps as a properties too.

It doesn't anymore after this last set of changes.

comment:7 scribu4 years ago

  • Keywords has-patch added

In 3.0 this joined on the usermeta table and returned and array of stdclass with the caps as a properties too.

More precisely, it had a 'meta_value' property, which contained the capabilities:

http://core.trac.wordpress.org/browser/tags/3.0.3/wp-includes/user.php#L328

comment:8 scribu4 years ago

I propose we revert get_users_of_blog() to it's 3.0 state and deprecate it.

comment:9 scribu4 years ago

  • Keywords has-patch removed

scribu4 years ago

Deprecated get_users_of_blog()

comment:10 scribu4 years ago

  • Keywords has-patch added

comment:11 westi4 years ago

deprecating the function and restoring the old direct SQL is lame.

Is there no way to wrap get_users and still return the old data structure?

comment:12 scribu4 years ago

deprecating the function and restoring the old direct SQL is lame.

To that I will reply that get_users_of_blog() is lame. :)

Is there no way to wrap get_users and still return the old data structure?

Only if we loop through each result, serialize back the 'capabilities' property and add it as 'meta_value'.

comment:13 ryan4 years ago

Per IRC discussion, going with 15854.diff.

comment:14 scribu4 years ago

  • Keywords commit added

comment:15 ryan4 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

(In [17084]) Revert get_users_of_blog() to 3.0 behavior and deprecate. Use get_users() in core. Props scribu. fixes #15854

comment:16 nacin3 years ago

Glad things get tested before commit. [17010] was borked and wasn't caught until now. #16361

Note: See TracTickets for help on using tickets.