824262 - Querying for strings in comments is now ultra slow

Status: RESOLVED FIXED

(Bugzilla :: Query/Bug List, defect)

Description

[Frédéric Buclin]

This is a regression due to bug 780820. Bugzilla 4.4rc1 is not affected as that bug landed after the release of 4.4rc1.

build_subselect() in Bugzilla/Search.pm now executes subselects itself and returs a hardcoded list of bug IDs instead of returning the subselect itself which will be executed as part of the larger query. This means that the subselect is executed independently of the other criteria used in the main search. For instance, using the GCC Bugzilla DB, looking for bugs assigned to me with "testing" in a comment returns 3 bugs in 0.006 second with the old subselect code, but takes 12.2 seconds with the new code!! The reason is that I have very few bugs assigned to me on GCC Bugzilla, and so the DB optimizer will restrict the search to these few bugs to execute the subselect. Now as the subselect is executed independently, it scans all the bugs and comments, which takes 12 seconds by itself! This is a major regression.

This also means that queries which will return no bugs due to impossible criteria will also take ages to complete instead of letting the DB optimizer skip the subselects immediately because it already knows that the query has no chance to return something.

So we have no other choice than to backout changes made in build_subselect(), at least for comments.

Comment 1

[Frédéric Buclin]

Attachment: patch, v1

I only revert the changes that glob made in build_subselect() for comments (aka the longdescs table) as the perf regression is very obvious there while it's hard to determine how it affects performance for other tables as most of them have indexes. But I would be fine to revert the changes unconditionally so that the DB optimizer has a chance to skip these subselects if it already knows that no data will be returned, or restrict the subselects to some given buglist if it already have this information.

justdave, what's your opinion on this?

Comment 2

[:glob ✱]

gah, sorry about that :(

this looks like an excellent optimistaion, however i can't test it fully right now, so please make sure you verify the feature added in bug 780820 still works with this change.

Comment 3

[Frédéric Buclin]

Comment on attachment 695242 [details] [diff] [review]
patch, v1

Asking dkl as glob is away till January 2, but glob should feel free to test it too. :)

Comment 4

[Frédéric Buclin]

(In reply to Byron Jones ‹:glob› (away until 2nd Jan) from comment #2)
> this looks like an excellent optimistaion

Yeah, looks like a good compromise. I tried to force this change for all subselects, but then other subselects make everything slow again, so it seems the benefit is only for comments.

Comment 5

[:glob ✱]

Comment on attachment 695242 [details] [diff] [review]
patch, v1

r=glob

Comment 6

[Frédéric Buclin]

Committing to: bzr+ssh://[e-mail]/bugzilla/trunk/
modified Bugzilla/Search.pm
Committed revision 8547.

Committing to: bzr+ssh://[e-mail]/bugzilla/4.4/
modified Bugzilla/Search.pm
Committed revision 8497.