Merge lp://staging/~gary/launchpad/bug731099 into lp://staging/launchpad
Status: | Merged |
---|---|
Approved by: | Gary Poster |
Approved revision: | no longer in the source branch. |
Merged at revision: | 12924 |
Proposed branch: | lp://staging/~gary/launchpad/bug731099 |
Merge into: | lp://staging/launchpad |
Diff against target: |
295 lines (+91/-102) 1 file modified
lib/lp/bugs/model/structuralsubscription.py (+91/-102) |
To merge this branch: | bzr merge lp://staging/~gary/launchpad/bug731099 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Graham Binns (community) | code | Approve | |
Review via email:
|
Commit message
[r=gmb][bug=731099] Improve the length and readability of the SQL generated by structural subscription queries when a bug has many bugtasks and/or when it has tags.
Description of the change
This branch addresses bug 731099, which is about the length and readability of the generated SQL in an edge case of many, many bugtasks for a single bug, many of which share the same combination of status and importance. It is tackled now because it is triaged high on the list of bugs that we must close to finish our feature.
I approached this problem by making two changes. First, I aggregated conditions that shared statuses and importances. As would be expected, this will shorten the SQL only when multiple bugtasks are involved that share the same status and importance. Second, I shifted a set of conditions that were duplicated when a bug had tags into a separate query. For the cost of adding a separate query, this improves the total length of the SQL generated for any bug with tags, and gradually approaches halving the generated SQL as the number of bugtasks increases.
Performance-wise, for a bug with tags and a single bugtask, this change is irrelevant: in my tests on my development machine, times went from a single 4ms query to two queries of 1ms and 3ms, respectively. If the bug has tags and several bugtasks (I tested with seven), the speed might have improved slightly, though my small hand-run sample size can only hint at a possible improvement (7ms vs 3ms + 3ms). Performance may or may not be significantly better for bugs with tens of bugtasks like bug 230350, which triggered the bug report this branch addresses; that said, the query in production now is not performing atrociously for being an edge case (about 400ms) and is also not the worst offender on the page.
(NB: Developers who are previewing the current malone features via feature flags should know that the original reported problem is completely hidden from them; https:/
This branch does not change behavior materially--results should be identical, and I actually increased the number of SQL queries made by one in order to reduce the total amount of SQL generated--and so no tests have been added or changed. I examined the output of https:/
As such, I'll summarize the results of my experiments here, and provide some links to pastebins for more information.
I examined the case of a bug with tags, because this generates the most SQL, and matches the bug description. I looked at the case of a bug with one bugtask, and at the case of a bug with several bugtasks (7).
For the case of one bugtask, my branch reduces SQL somewhat by virtue of breaking out the duplicated conditions into a separate query. In contrast, as would be expected, the change to aggregate targets by statuses and importances is irrelevant. Total character count went from 3280 to 2735--the new is about 83% the size of the old. See http://
For the case of seven bugtasks with shared statuses/
Thank you!
Gary