Closed Bug 993894 Opened 10 years ago Closed 10 years ago

the database query in bless_groups is slow

Categories

(Bugzilla :: Bugzilla-General, defect)

Product:
Bugzilla
Component:
Bugzilla-General
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
Bugzilla 5.0

People

(Reporter: glob, Assigned: glob)

References

Details

(Keywords: bmo-goal, perf)

Attachments

(1 file, 1 obsolete file)

993894_2.patch
2.60 KB, patch
dkl
: review+
Details | Diff | Splinter Review 
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.
Attached patch 993894_1.patch (obsolete) — — Splinter Review 

        Attachment #8403781 -
        Flags: review?(dkl)

    
    

    
  
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-

    
    

    
  
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.

    
    

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

Looks good, and is much cleaner. :)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        993894_2.patch
        — 
          — Splinter Review
      

    


  

        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+
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
Closed: 10 years ago
Resolution: --- → FIXED

    
  
Keywords: bmo-goal

    
    

    
  
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.

    
  
Blocks: 1035221

    
    

    
  
(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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: