Voyager: excessive queries

[stoutZero]

• Laravel Version: 5.6.11
• Voyager Version: dev-1.x-dev#b873c6f2
• PHP Version: 7.2.3
• Database Driver & Version: mysql Ver 15.1 Distrib 10.2.13-MariaDB

Description:

Excessive (70+ / 200+) queries to users, roles, and permissions tables.
This happens on every page.

If I do this on User model:

    protected $with = [
        'role', 'role.permissions',
        'roles', 'roles.permissions',
    ];

Queries executed went down from 200+ to 70+ queries.

This is the offending query:

select `roles`.*, `user_roles`.`user_id` as `pivot_user_id`, `user_roles`.`role_id` as `pivot_role_id`
from `roles`
    inner join `user_roles` on `roles`.`id` = `user_roles`.`role_id`
where `user_roles`.`user_id` in ('1')

This query seems to be generated from: VoyagerUser.php@loadPermissionsRelations#L126.

From what I can glance from debugbar's data, seems like almost half of it is
MenuItemPolicy queries, how do I disable this policy?

I think I don't need to check for permission when rendering menu,
just check for permission when that Url is being requested,
isn't that what Policy is?

Btw, why isn't User's roles & permissions cached?

Steps To Reproduce:

I don't know how to reproduce, so, here is a screenshot

74 Queries

215 queries, 172 of which were duplicated, 43 unique

[stoutZero]

With a lot of _hack_ here and there, I finally got roles relation on User model
and permission relation on Role model to cache to redis with tags...

On dashboard page, queries came down to 14 (no duplicate), but the cache was hit 25 times, this is _somewhat_ okay

But when viewing a BREAD page for a table that has _34 rows_ in it,
something lags, on a paginated page that renders _10 rows_ things should go much faster...

turns out permission checking via Gate was run _140 times_, and the cache was hit _700+ times_...
this happens on a page that renders _10 rows_

I finally understand this whole BREAD system, and what's wrong with it...
the permission system for BREAD is just way too granular,
I can't come up with a situation where column based permission is ever needed...

The idea of not having to code your own Index, Create, Update, and Delete page for a table is really enticing...
But not when you have to check for permission for every columns in each rows of every BREAD enabled tables 😢
this makes me feel bery bad...

Btw, this is what happens when I try to load a BREAD page for a table that has _80k+ rows_ in it

nginx screams 504 LOL (on localhost 😲 )

Anyway, spent an entire day trying out voyager, time to cut loose...
Have a nice day guys... see ya... :)

[marktopper]

The only way to work with Voyager using that many rows, is to use server-side pagination instead of frontend pagination. You can change the setting for the tables BREAD in order to server-side pagination in order to get rid of the 504 Gateway Time-out issue.

[marktopper]

Besides that then the queries could be optimized and we are open to pull requests.

[jannejava]

First of all, I like Voyager and especially the BREAD system, and the great effort you have done with Voyager. 👍

But indeed that there are many queries going on. 69 and 28 of them are duplicates, and I only have two rows in my custom table. Also, the use case of granular permissions is non-existent for me too. I will take a look around if I can make a pull request.

[fletch3555]

Can you share the query list for that page? I'm willing to bet many of those are menu queries, and the rest are permissions checks. I'm certain that we're not fully leveraging caching. We do have local per-request caches set up, which does help significantly, but of course it's not perfect. The problem we run into is that since this package isn't a full product itself, we're trying to minimize the number of decisions we make on your behalf, including page caching. Once we start working with a larger cache system (i.e. redis or even Laravel's file/database cache providers), we start having to make those decisions.

[jannejava]

I did some experimenting with caching in two places and got it down from 69 to 7 queries.

You are indeed right; it's just related to the menu and hasPermission($name) in \Trait\VoyagerUser.php in particular. That one I think you could only cache this in memory within a static property for each request.

I did also cache the whole menu, which did cut a lot of queries.

I completely understand the challenge of invalidating the cache and keeping this in sync and avoiding side effects. But Laravel the framework does a lot of caching of routes, views, and configs on its own so from my point of view caching is not a controversial decision if you are using Laravel.

If you are interested I can make a pull request, but it's not complicated. Like this:

// \Trait\VoyagerUser.php
public function hasPermission($name)
{
        return Cache::remember('has_permission_' . $name, 60, function () use ($name) {
            $this->loadPermissionsRelations();

            $_permissions = $this->roles_all()
                              ->pluck('permissions')->flatten()
                              ->pluck('key')->unique()->toArray();

            return in_array($name, $_permissions);
        });
    }

// \Models\Menu.php
public static function display($menuName, $type = null, array $options = [])
{
        $cacheKey = self::cacheKey($menuName, $type, $options);

        if (Cache::has($cacheKey)) {
            return Cache::get($cacheKey);
        }
        ....or build the html for the menu
}

public static function cacheKey($menuName, $type, $options)
{
        return sprintf(
            '%s_%s_%s',
            $menuName,
            $type,
            md5(serialize($options))
        );
}

Before

After

[fletch3555]

We're always open for pull requests. No guarantee it'll get merged in, but efficiency bumps like this are definitely welcomed. Thanks!

[emptynick]

I just opened a PR which should significantly decrease your queries.
Maybe you can try it out and report how much Statements you have left

[jannejava]

Yes tried the last commit where your pull seems to be merge. It has cut down the queries to 45 for my two rows.

25 of the unique queries are invoked these two lines for rendering the menu. Is it possible to load all the permissions and slugs at once instead?

`` select * fromdata_typeswhereslug` = '' limit 1
(/vendor/tcg/voyager/src/Policies/MenuItemPolicy.php:29)

select exists(select * from permissions where key = 'browse_users') as exists
(/vendor/tcg/voyager/src/Policies/MenuItemPolicy.php:40)

[jannejava]

Just did a PR, see if it works in the bigger picture. This cut down the queries from 44 to 22.

[emptynick]

I see that you did a commit to your fork, do you want to open a PR here?

[jannejava]

Forgot to do that! Done it now, check out if it works. I have not a complete understanding of permission system in Voyager, but it's working for me now. Loads a bit faster.

[fletch3555]

PR mentioned last week is #2832, which has already been merged. I'm going to close this.

[fimaruf]

I just run the command 'composer update' and I am getting this error. It was never before. Any help ?

[github-actions[bot]]

This issue has been automatically locked since there has not been any recent activity after it was closed. If you have further questions please ask in our Slack group.