405970 - checksetup.pl is really slow when there's nothing to do

Status: RESOLVED FIXED

(Bugzilla :: Installation & Upgrading, defect)

Description

[Max Kanat-Alexander]

Attachment: v1

Today on IRC, justdave pointed out to me that checksetup takes almost a minute to run on his bmo test database, even though there's nothing to do.

I found out why! It's because we *always* rederive regexp groups. It took me a lot of research to figure out what happened. Here's what happened:

In bug 240325, we added grant_type, and flattened regexp groups directly into the DB. Then, we removed GRANT_DERIVED type groups, in bug 304583. When we did that, we moved the code that rederived regexps-based group membership into a block of code that *always ran*. There wasn't a reason for this--we just didn't have a particular schema change to wrap it in.

When I moved things into Bugzilla::Install in bug 352593, I assumed that since that code was outside of a block (and originally in the checksetup.pl section where Groups things were done, not Table things), it actually belonged in update_system_groups in Bugzilla::Install and not in Bugzilla::Install::DB, where it really should have gone.

So, for a long time, we've been re-deriving all regexp groups on every run of checksetup.pl when there was no need to. Bugzilla::Group itself does the rederiving correctly when you create a new group, and editgroups.cgi does the rederiving when you change the regex. When you update somebody's login name, their regexp-based memberships are also automatically rederived.

It can be useful if somebody manually changes the userregexp in the DB, but I don't want to add a huge delay to every checksetup run just to add a feature for people who are manually mucking with their DBs.

The delay is so huge because we do "profiles CROSS JOIN groups", which means we get a result, on *every checksetup run*, of a number of rows equal to ALL USERS times ALL GROUPS.

Anyhow, this moves the code back to where it should belong, and makes checksetup nearly instantaneous on landfill.

Comment 1

[Dave Miller [:justdave]]

Comment on attachment 290677 [details] [diff] [review]
v1

That's the ticket. :)

Tested and works on both 3.0 branch and trunk. :)

Comment 2

[Max Kanat-Alexander]

justdave says he wants to do a few more tests.

I don't feel comfortable taking this on the branch, so I think we should only take it on tip. It's just a performance enhancement for checksetup.pl, really, and removes a tiny, probably-unused feature. Still, even a tiny feature removal I think should go into 3.2, not 3.0.

Comment 3

[Dave Miller [:justdave]]

To be specific, what I wanted tested (which I'm not really sure how to do - mkanat maintains all the Really Old DBs on landfill ;) ) is testing an upgrade from the version immediately before each of the two changes were introduced that this code is trying to check for, to make sure that the changes still do happen as designed when upgrading from those versions.

2.16 introduced user_group_map
2.18 introduced grant_type
2.22 removed derived groups

So 2.16 and 2.20 are the versions that we need to test upgrading from.

Comment 4

[Dave Miller [:justdave]]

(because grant_type and derived groups are the things touched by this patch)

Comment 5

[Max Kanat-Alexander]

Comment on attachment 290677 [details] [diff] [review]
v1

This doesn't work on Bugzilla 2.16 upgrades, since the user_group_map table didn't exist and it gets created with its current definition (not its original definition), and there are no derived groups, so the code never runs.

Comment 6

[Max Kanat-Alexander]

Attachment: v2

Okay, I've been working on a large installation recently and this was bugging me, so I fixed it the right way--now we actually only rederive regex groups when there exist userregexp groups and there exist no users with regex-derived group memberships.

Comment 7

[Max Kanat-Alexander]

Checking in Bugzilla/Install.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install.pm,v  <--  Install.pm
new revision: 1.23; previous revision: 1.22
done
Checking in Bugzilla/Install/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v  <--  DB.pm
new revision: 1.66; previous revision: 1.65
done

Comment 8

[Frédéric Buclin]

"Deriving regex group memberships..." is displayed everytime I run checksetup.pl. Is this expected?

Comment 9

[Max Kanat-Alexander]

(In reply to comment #8)
> "Deriving regex group memberships..." is displayed everytime I run
> checksetup.pl. Is this expected?

  There is a case in which that will happen, yes. If you have userregexps set on groups but have no users who match them, you will see the message every time you run checksetup.