Negative performance impact in the User Model

[jasonbahl]

When querying for Posts (or custom post types) or Users, we first check to see if the user is considered private or not.

To check this, we check to see if the user has published posts.

In WordPress, if a User has published a Post, they're considered a publicly accessible entity (not all fields of the User are considered public though).

The current approach for making this check is to use the core method count_user_posts

On small sites, this is a quick check. On large sites, with large datasets, this can be slow.

To reduce the performance impact, we should provide an alternative method that queries for published posts by the post_author more efficiently. We can do this by first limiting the query to 1 item. We don't care how many posts the author has published, because a single published post makes them non-private. We can also cache that so that we don't have to query each time we check. WordPress.com VIP has a cached version of count_user_posts we can draw inspiration from: https://github.com/svn2github/wordpress-vip-plugins/blob/9c0a32757bdd78a0e2babf72a593c2c6a83287df/vip-do-not-include-on-wpcom/wpcom-caching.php#L403-L425

We don't want to rely on the VIP function itself, as more often than not, a site will not be running the VIP code.

In addition to the more efficient, cached query, we should also provide a filter that allows for developers to bypass the check altogether and provide their own way of determining if a User should be considered public.

[jasonbahl]

I opened a PR #962 to address this.

After discussing, the PR has been closed and I will give it another go.

The current PR attempts to make this more performant by making the query to count user posts more efficient, and caching the query.

@CodeProKid brings up the point that WPGraphQL shouldn't really in the business of making cache decisions, but should instead provide proper hooks and filters so that cache can be implemented.

I would agree.

So instead of caching the query, we should refactor a bit to use DataLoader to batch fetch the published post counts of users, which should significantly reduce the number of MySQL connections per request.

See similar DataLoader implementation here: https://github.com/wp-graphql/wp-graphql/blob/develop/src/Data/Loader/PostObjectLoader.php#L117-L123

Some pseudo SQL we can use for a new Loader might be:

SELECT 
    post_author, COUNT(*)
FROM
    wp_posts
WHERE ( ( post_type = 'post' AND ( post_status = 'publish' ) ) OR ( post_type = 'page' AND ( post_status = 'publish' ) ) )
AND post_author IN (17,1)
GROUP BY post_author;

Basically, what we'll need to do is accept a list of Author IDs, run a SQL Query to find out if they have at least 1 published post, then return an array in the same order as the Author IDs, but with values of the number of published posts they have (either 1 or 0).

If 1 (or more), that means they're a public entity, if 0 they are unpublished author and considered private.

[zacscott]

Hello there,

This is a significant problem on a project I am currently working on. This method is called dozens of times per query and it is running a COUNT(*) on hundreds of thousands of records.

Some hooks/filters for this would be greatly appreciated.

Here are some screenshot from our New Relic.

[zacscott]

Hello,

I have raised another pull request to hopefully help resolve this issue. This was based on the PR you originally opened @jasonbahl, simply adding the graphql_user_model_is_private filter with no caching / query changes etc. #1685

I see @chriszarate has also opened a pull request a short time ago. I think there is a place for both of our solutions.

Being able to completely bypass the is_private() is ideal for our situation as we'll either have all users available or have it based on something much simpler, e.g. the user role.

[zacscott]

Sorry to bump this thread but can we get this one prioritised. This issue is generating huge load for us.