launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #04437
[Merge] lp:~gary/launchpad/bug787294 into lp:launchpad
Gary Poster has proposed merging lp:~gary/launchpad/bug787294 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #787294 in Launchpad itself: "structural subscription bug search timeouts (affects API, Person:+patches, etc) when many subscriptions are held timeouts"
https://bugs.launchpad.net/launchpad/+bug/787294
For more details, see:
https://code.launchpad.net/~gary/launchpad/bug787294/+merge/69887
This branch translates the raw SQL optimization work Robert did for bug 787294 into code.
This was attempted once before, but the translation was not close enough. This time, I think we are close enough. I can share the SQL we are generating now if desired. The proof is in the pudding: the SQL from the previous attempt consistently takes > 35 seconds on staging; using the SQL generated in this branch, I've gotten < 1 second on staging after the cache is warm. (Of course, with a cold cache, staging takes some gargantuan amount of time.)
lint is happy. ec2 test is happy.
Thanks
Gary
--
https://code.launchpad.net/~gary/launchpad/bug787294/+merge/69887
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~gary/launchpad/bug787294 into lp:launchpad.
=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py 2011-07-27 04:34:19 +0000
+++ lib/lp/bugs/model/bugtask.py 2011-07-30 13:24:38 +0000
@@ -128,7 +128,6 @@
)
from lp.bugs.model.bugnomination import BugNomination
from lp.bugs.model.bugsubscription import BugSubscription
-from lp.bugs.model.structuralsubscription import StructuralSubscription
from lp.registry.interfaces.distribution import (
IDistribution,
IDistributionSet,
@@ -1946,55 +1945,55 @@
sqlvalues(personid=params.subscriber.id))
if params.structural_subscriber is not None:
- ssub_match_product = (
- BugTask.productID ==
- StructuralSubscription.productID)
- ssub_match_productseries = (
- BugTask.productseriesID ==
- StructuralSubscription.productseriesID)
- # Prevent circular import problems.
- from lp.registry.model.product import Product
- ssub_match_project = And(
- Product.projectID ==
- StructuralSubscription.projectID,
- BugTask.product == Product.id)
- ssub_match_distribution = (
- BugTask.distributionID ==
- StructuralSubscription.distributionID)
- ssub_match_sourcepackagename = (
- BugTask.sourcepackagenameID ==
- StructuralSubscription.sourcepackagenameID)
- ssub_match_null_sourcepackagename = (
- StructuralSubscription.sourcepackagename == None)
- ssub_match_distribution_with_optional_package = And(
- ssub_match_distribution, Or(
- ssub_match_sourcepackagename,
- ssub_match_null_sourcepackagename))
- ssub_match_distribution_series = (
- BugTask.distroseriesID ==
- StructuralSubscription.distroseriesID)
- ssub_match_milestone = (
- BugTask.milestoneID ==
- StructuralSubscription.milestoneID)
-
- join_clause = Or(
- ssub_match_product,
- ssub_match_productseries,
- ssub_match_project,
- ssub_match_distribution_with_optional_package,
- ssub_match_distribution_series,
- ssub_match_milestone)
-
- join_tables.append(
- (Product, LeftJoin(Product, And(
- BugTask.productID == Product.id,
- Product.active))))
- join_tables.append(
- (StructuralSubscription,
- Join(StructuralSubscription, join_clause)))
- extra_clauses.append(
- 'StructuralSubscription.subscriber = %s'
+ # See bug 787294 for the story that led to the query elements
+ # below. Please change with care.
+ with_clauses.append(
+ '''ss as (SELECT * from StructuralSubscription
+ WHERE StructuralSubscription.subscriber = %s)'''
% sqlvalues(params.structural_subscriber))
+ # Prevent circular import problems.
+ from lp.registry.model.product import Product
+ join_tables.append(
+ (Product, LeftJoin(Product, And(
+ BugTask.productID == Product.id,
+ Product.active))))
+ join_tables.append(
+ (None,
+ LeftJoin(
+ SQL('ss ss1'),
+ BugTask.product == SQL('ss1.product'))))
+ join_tables.append(
+ (None,
+ LeftJoin(
+ SQL('ss ss2'),
+ BugTask.productseries == SQL('ss2.productseries'))))
+ join_tables.append(
+ (None,
+ LeftJoin(
+ SQL('ss ss3'),
+ Product.project == SQL('ss3.project'))))
+ join_tables.append(
+ (None,
+ LeftJoin(
+ SQL('ss ss4'),
+ And(BugTask.distribution == SQL('ss4.distribution'),
+ Or(BugTask.sourcepackagename ==
+ SQL('ss4.sourcepackagename'),
+ SQL('ss4.sourcepackagename IS NULL'))))))
+ join_tables.append(
+ (None,
+ LeftJoin(
+ SQL('ss ss5'),
+ BugTask.distroseries == SQL('ss5.distroseries'))))
+ join_tables.append(
+ (None,
+ LeftJoin(
+ SQL('ss ss6'),
+ BugTask.milestone == SQL('ss6.milestone'))))
+ extra_clauses.append(
+ "NULL_COUNT("
+ "ARRAY[ss1.id, ss2.id, ss3.id, ss4.id, ss5.id, ss6.id]"
+ ") < 6")
has_duplicate_results = True
# Remove bugtasks from deactivated products, if necessary.
@@ -2488,9 +2487,10 @@
origin = [BugTask]
already_joined = set(origin)
for table, join in join_tables:
- if table not in already_joined:
+ if table is None or table not in already_joined:
origin.append(join)
- already_joined.add(table)
+ if table is not None:
+ already_joined.add(table)
for table, join in prejoin_tables:
if table not in already_joined:
origin.append(join)
@@ -2544,9 +2544,11 @@
[query, clauseTables, ignore, decorator, join_tables,
has_duplicate_results, with_clause] = self.buildQuery(arg)
origin = self.buildOrigin(join_tables, [], clauseTables)
+ localstore = store
if with_clause:
- store = orig_store.with_(with_clause)
- next_result = store.using(*origin).find(inner_resultrow, query)
+ localstore = orig_store.with_(with_clause)
+ next_result = localstore.using(*origin).find(
+ inner_resultrow, query)
resultset = resultset.union(next_result)
# NB: assumes the decorators are all compatible.
# This may need revisiting if e.g. searches on behalf of different
