The default bug view has changed. See this FAQ.

the database query in bless_groups is slow

RESOLVED FIXED in Bugzilla 5.0

Status

()

Bugzilla
Bugzilla-General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: glob, Assigned: glob)

Tracking

({bmo-goal, perf})

unspecified
Bugzilla 5.0
bmo-goal, perf
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
the database query in bless_groups is slow and can be rewritten to be faster.
it's called on every page load for users who are not admins.

sample of the current query:

> SELECT DISTINCT groups.id 
>   FROM groups, 
>        user_group_map, 
>        group_group_map 
>  WHERE user_group_map.user_id = 3881 
>        AND (
>            (user_group_map.isbless = 1 AND groups.id=user_group_map.group_id)
>            OR (groups.id = group_group_map.grantor_id
>                AND group_group_map.grant_type = 1
>                AND group_group_map.member_id IN (20,19,10,9,94,23,49,2,119,69,130,131,84,158,98,144,145,61,32,6,42,22,26,120,33,29,27,96,38,113,46,52,51))
>        )

*************************** 1. row ***************************
           id: 1
  select_type: SIMPLE
        table: user_group_map
         type: ref
possible_keys: user_group_map_user_id_idx,fk_user_group_map_group_id_groups_id
          key: user_group_map_user_id_idx
      key_len: 3
          ref: const
         rows: 17
        Extra: Using index; Using temporary
*************************** 2. row ***************************
           id: 1
  select_type: SIMPLE
        table: groups
         type: index
possible_keys: PRIMARY,groups_name_idx
          key: groups_name_idx
      key_len: 767
          ref: NULL
         rows: 148
        Extra: Using index; Using join buffer (Block Nested Loop)
*************************** 3. row ***************************
           id: 1
  select_type: SIMPLE
        table: group_group_map
         type: index
possible_keys: group_group_map_member_id_idx,fk_group_group_map_grantor_id_groups_id
          key: fk_group_group_map_grantor_id_groups_id
      key_len: 3
          ref: NULL
         rows: 511
        Extra: Using where; Using index; Distinct; Using join buffer (Block Nested Loop)

rewriting it as the following should be quicker:

> SELECT distinct groups.id
> FROM groups
>        INNER JOIN user_group_map ON user_group_map.user_id = 3881
>        INNER JOIN group_group_map ON group_group_map.grantor_id = groups.id
>  WHERE (user_group_map.group_id = groups.id AND user_group_map.isbless = 1)
>        OR (group_group_map.grant_type = 1
>            AND group_group_map.member_id IN (20,19,10,9,94,23,49,2,119,69,130,131,84,158,98,144,145,61,32,6,42,22,26,120,33,29,27,96,38,113,46,52,51))

*************************** 1. row ***************************
           id: 1
  select_type: SIMPLE
        table: groups
         type: index
possible_keys: PRIMARY,groups_name_idx
          key: groups_name_idx
      key_len: 767
          ref: NULL
         rows: 148
        Extra: Using index; Using temporary
*************************** 2. row ***************************
           id: 1
  select_type: SIMPLE
        table: group_group_map
         type: ref
possible_keys: group_group_map_member_id_idx,fk_group_group_map_grantor_id_groups_id
          key: fk_group_group_map_grantor_id_groups_id
      key_len: 3
          ref: bugs_bmo_201312.groups.id
         rows: 1
        Extra: Using index; Distinct
*************************** 3. row ***************************
           id: 1
  select_type: SIMPLE
        table: user_group_map
         type: ref
possible_keys: user_group_map_user_id_idx,fk_user_group_map_group_id_groups_id
          key: user_group_map_user_id_idx
      key_len: 3
          ref: const
         rows: 17
        Extra: Using where; Using index; Distinct


on my development system average times dropped from 0.053 seconds to 0.002 seconds.
(Assignee)

Comment 1

3 years ago
Created attachment 8403781 [details] [diff] [review]
993894_1.patch
Attachment #8403781 - Flags: review?(dkl)

Comment 2

3 years ago
Comment on attachment 8403781 [details] [diff] [review]
993894_1.patch

>+        SELECT DISTINCT groups.id
>+          FROM groups
>+               INNER JOIN user_group_map AS ugm ON ugm.user_id = ?
>+               INNER JOIN group_group_map AS ggm ON ggm.grantor_id = groups.id
>+         WHERE (ugm.group_id = groups.id AND ugm.isbless = 1)
>+               OR (ggm.grant_type = " . GROUP_BLESS . "
>+                   AND " . $dbh->sql_in('ggm.member_id', \@group_ids) . ")";

This query gives different results than the original one. Your INNER JOIN on group_group_map forces the group ID to exist as grantor_id, else it's skipped. But the original query says that you either have direct privs to bless a group (the isbless = 1 bit) *OR* you belong to a group which has privs to bless another group (the ggm.grantor_id = groups.id AND ggm.grant_type = GROUP_BLESS bit).

I think things would work better with an UNION (one query for (user) direct privs, one for (group) inherited privs), because the groups table is of no use here, except to glue both queries into a single one.
Attachment #8403781 - Flags: review?(dkl) → review-
(Assignee)

Comment 3

3 years ago
lpsolit, could i get your thoughts on the following:

> SELECT DISTINCT group_id
>   FROM user_group_map
>  WHERE user_id = 3881
>        AND isbless = 1
> UNION
> SELECT DISTINCT grantor_id
>   FROM group_group_map
>  WHERE grant_type = 1
>        AND member_id IN (20,19,10,9,94,23,49,2,119,69,130,131,84,158,98,144,145,61,32,6,42,22,26,120,33,29,27,96,38,113,46,52,51)

*************************** 1. row ***************************
           id: 1
  select_type: PRIMARY
        table: user_group_map
         type: ref
possible_keys: user_group_map_user_id_idx,fk_user_group_map_group_id_groups_id
          key: user_group_map_user_id_idx
      key_len: 3
          ref: const
         rows: 5
        Extra: Using where; Using index
*************************** 2. row ***************************
           id: 2
  select_type: UNION
        table: group_group_map
         type: index
possible_keys: group_group_map_member_id_idx,fk_group_group_map_grantor_id_groups_id
          key: fk_group_group_map_grantor_id_groups_id
      key_len: 3
          ref: NULL
         rows: 441
        Extra: Using where; Using index
*************************** 3. row ***************************
           id: NULL
  select_type: UNION RESULT
        table: <union1,2>
         type: ALL
possible_keys: NULL
          key: NULL
      key_len: NULL
          ref: NULL
         rows: NULL
        Extra: Using temporary

this is even faster on my dev box: 0.001 sec.

Comment 4

3 years ago
(In reply to Byron Jones ‹:glob› from comment #3)
> lpsolit, could i get your thoughts on the following:

Looks good, and is much cleaner. :)
(Assignee)

Comment 5

3 years ago
Created attachment 8404086 [details] [diff] [review]
993894_2.patch
Attachment #8403781 - Attachment is obsolete: true
Attachment #8404086 - Flags: review?(dkl)
Comment on attachment 8404086 [details] [diff] [review]
993894_2.patch

Review of attachment 8404086 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Attachment #8404086 - Flags: review?(dkl) → review+

Updated

3 years ago
Flags: approval?
Target Milestone: --- → Bugzilla 5.0
Flags: approval? → approval+
went ahead and committed in your absence so to be able to get into this devel snapshot being released.

[master 6f4b262] Bug 993894 - the database query in bless_groups is slow r=dkl,a=justdave
 Author: Byron Jones <glob@mozilla.com>
 1 files changed, 30 insertions(+), 22 deletions(-)

To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   e93a383..6f4b262  master -> master
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Keywords: bmo-goal

Comment 8

3 years ago
This patch causes my Bugzilla installation to throw errors on every page unless the user is part of the editusers group.

>First error: undef error - DBD::mysql::db selectcol_arrayref failed: You have an error in your SQL syntax;
>check the manual that corresponds to your MySQL server version for the right syntax to use near ')' at line 4 [for Statement "
>       SELECT DISTINCT group_id
>         FROM user_group_map
>        WHERE user_id = ?
>              AND isbless = 1 AND  group_id IN () "] at Bugzilla/User.pm line 1013>
>Bugzilla::User::bless_groups('Bugzilla::User=HASH(0xa0eaff8)') called at Bugzilla/User.pm line 1712
>Bugzilla::User::can_bless('Bugzilla::User=HASH(0xa0eaff8)') called at Bugzilla/User.pm line 1463
>Bugzilla::User::can_administer('Bugzilla::User=HASH(0xa0eaff8)') called at template/en/default/global/common-links.html.tmpl line 58

(which continues on to all the template files.)

Reversing the patch fixes the errors.
(Assignee)

Updated

3 years ago
Blocks: 1035221
(Assignee)

Comment 9

3 years ago
(In reply to David Corry from comment #8)
> This patch causes my Bugzilla installation to throw errors on every page
> unless the user is part of the editusers group.

filed this as bug 1035221.
You need to log in before you can comment on or make changes to this bug.