Cms: Users Eloquent Driver N+1 query problem

Created on 17 Mar 2020  Â·  14Comments  Â·  Source: statamic/cms

Version: 3.0.0-beta-19

I think there might be a N+1 query problem when setting users with Eloquent driver.

Steps to reproduce:

  • Create a new Statamic project. Laravel debug bar activated by default.
  • Setup storing Users in a Database.
  • Create a dummy Collection like "Posts" and several entries.
  • Create a route for posts index like
Route::statamic('posts', 'posts', [
   'title' => 'Posts'
]);
  • Create a view like
{{ collection:posts }}

  <div>{{ title }}</div>

{{ /collection:posts }}

What I've seen so far:

  • The N+1 query problems shows up in both CMS side and frontend. Didn't even begin to try to debug CMS side.
  • In the posts.index page I get the following results:
0 posts  -  1 query
1 post   -  7 queries
2 posts  -  13 queries
3 posts  -  19 queries

so it looks like a for this simple scenario, for every entry you got 6 queries.

For 2 posts, the queries executed are:

select * from `users` where `id` = 1 limit 1
select * from `users` where `users`.`id` = 1 limit 1
select * from `role_user` where `user_id` = 1
select * from `group_user` where `user_id` = 1
select * from `users` where `users`.`id` = 1 limit 1
select * from `role_user` where `user_id` = 1
select * from `group_user` where `user_id` = 1
select * from `users` where `users`.`id` = 1 limit 1
select * from `role_user` where `user_id` = 1
select * from `group_user` where `user_id` = 1
select * from `users` where `users`.`id` = 1 limit 1
select * from `role_user` where `user_id` = 1
select * from `group_user` where `user_id` = 1

This caught my attention while working on fairly complex site when suddenly I spotted in Telescope a huge number of queries for a specify request.

I've been trying to dig to find the problem but I'm afraid I don't have a deep enough understanding of the codebase yet. Please let me know if I can do anything to help. Will continue to dig into this anyway.

I take this opportunity to wish you health in these difficult times.

bug database

All 14 comments

So it seems the user is requested twice for each post? (The fact that roles and groups aren't cached is a bug we also noticed but didn't report yet.)

Could this be related to https://github.com/statamic/cms/issues/1513? We noticed it had to do with the updated_by field that every entry has. Maybe you can try the workaround I describe in my latest comment there and see if that reduces the number of calls to three per post (user, roles, groups)? Since it seems unnecessary to resolve more then one level, we removed augmentation from inside a value's serialization code. (The value itself is augmented when augmenting the entry the value is contained in.)

Is there any update on this? A fix would be greatly appreciated.

This issue has not had recent activity and has been marked as stale — by me, a robot. Simply reply to keep it open and send me away. If you do nothing, I will close it in a week. I have no feelings, so whatever you do is fine by me.

I've just ran into this same issue. I only have a single user in my project, yet it's making 13 database queries for users.

image

I've tracked the issue down to the updatedBy method on the AugmentedEntry class.

Looks like the workaround for this issue is to set track_last_update in config/statamic/system.php to false and to remove updated_by by all of your entries, then Statamic makes a lot less database requests.

I'm not quite sure if there's a way Statamic could cache the user data instead of making fresh queries every single time it needs something from the DB.

We may be able to get around this by wrapping user::find in blink here:

return Blink::once('eloquent-user-find-'.$id, function () use ($id) {

https://github.com/statamic/cms/blob/a01821977f5e8dc4ed59a2ec20724ee5eee07a14/src/Auth/Eloquent/UserRepository.php#L37-L41

});

We'll want to clear that blink key in the save and delete methods.

You can use this fix now with the following workaround. Create app/UserRepository.php

<?php

 namespace App;

 use Statamic\Facades\Blink;
 use Statamic\Contracts\Auth\User as UserContract;

 class UserRepository extends \Statamic\Auth\Eloquent\UserRepository
 {
     public function find($id): ?UserContract
     {
         return Blink::once('eloquent-user-find-'.$id, function () use ($id) {
             return parent::find($id);
         });
     }
 }

Plop this in your AppServiceProvider

app(\Statamic\Auth\UserRepositoryManager::class)->extend('eloquent', function ($app, $config) {
  $config['model'] = $app['config']['auth.providers.users.model'];
  return new \App\UserRepository($config);
});

Ah, that fixes the issue, there's now only a single database query per request for users. I can make a pull request for this if you'd like @jasonvarga?

I knew you would want to. 😄

I knew you would want to. 😄

Haha. How did you guess?

Hi, thank you for reviewing this.

I do a followup on this since it might be related. I'm currently at 3.0.47 and for every @can statement in a blade template, Laravel Debug Bar shows

select * from `group_user` where `user_id` = 1
select * from `role_user` where `user_id` = 1

so the N+1 is somehow still there. I also noticed that the Debug Bar show dozens of the previous queries in the Statamic Control Panel.

_Me thinking: I'm really not sure what the behavior should be, many times I feel that many parts of the application don't really care about Statamic permissions, and those are only used for the control panel customization. I would even say that my app users and Statamic users shouldn't have anything to do, but that's no relevant here i guess._

Thank you!

Hey @vguerrerobosch, would you mind opening a new issue about the group/role n+1 queries you're seeing? Thanks.

I'm also seeing the same as @vguerrerobosch

This is a separate problem from the initial issue. You can open a new one explaining that you use the can directive.

This is a separate problem from the initial issue. You can open a new one explaining that you use the can directive.

I will do soon I'm just investigating. At first glance I think the blink fix applied above needs to be applied to the all() method on StatamicAuthEloquentUserGroups and StatamicAuthEloquentRoles.

I'm too sure though still looking but thats where the query seems to be duplicating

Was this page helpful?
0 / 5 - 0 ratings

Related issues

skoontastic picture skoontastic  Â·  4Comments

riasvdv picture riasvdv  Â·  4Comments

jimblue picture jimblue  Â·  3Comments

aerni picture aerni  Â·  4Comments

filipac picture filipac  Â·  4Comments