launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #02299
[Merge] lp:~adeuring/launchpad/bug-607958 into lp:launchpad
Abel Deuring has proposed merging lp:~adeuring/launchpad/bug-607958 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
#607958 timeouts on Distribution:+bugtarget-portlet-bugfilters-stats
https://bugs.launchpad.net/bugs/607958
For more details, see:
https://code.launchpad.net/~adeuring/launchpad/bug-607958/+merge/45675
This branch fixes bug 607958: timeouts on Distribution:+bugtarget-portlet-bugfilters-stats
The query to find expirable bugtasks is/was quite slow. An EXPLAIN ANALYZE showed that the sub-query to find all possible bug targets having bug expiration enabled retrieved _all_ bugtargets, even if findExpirableBugTasks() is called with a target. This branch modifies this sub-query so that only those bug targets are retrieved that are related to the given target parameter. (Note the difference between a target of a bug task and parameter "target" of findExpirableBugtasks(): If the method parameter is a distribution or a product, we want to retrieve bugtasks for a distroseries or a productseries too).
So I added a method _getJoinForTargets() which builds the JOIN expression for several targets. This method is called by _getTargetJoinAndClause() with the "right" selection of target types for a given parameter "target" of findExpirableBugTasks()
This makes the query twice as a fast as before for findExpriableBugTasks(target=ubuntu). (Note that the timeouts mentioned in the bug report already disappeared when we switched to PostgesQL 8.3)
test: ./bin/test -vvt test_bugtask.BugTaskSetFindExpirableBugTasksTest
no lint
--
https://code.launchpad.net/~adeuring/launchpad/bug-607958/+merge/45675
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~adeuring/launchpad/bug-607958 into lp:launchpad.
=== modified file 'lib/lp/bugs/model/bugtask.py'
--- lib/lp/bugs/model/bugtask.py 2011-01-05 08:10:10 +0000
+++ lib/lp/bugs/model/bugtask.py 2011-01-10 11:19:16 +0000
@@ -2620,6 +2620,60 @@
AND RelatedBugTask.status IN %s)
""" % sqlvalues(unexpirable_status_list)
+ TARGET_SELECT = {
+ IDistribution: """
+ SELECT Distribution.id, NULL, NULL, NULL,
+ Distribution.id, NULL
+ FROM Distribution
+ WHERE Distribution.enable_bug_expiration IS TRUE""",
+ IDistroSeries: """
+ SELECT NULL, DistroSeries.id, NULL, NULL,
+ Distribution.id, NULL
+ FROM DistroSeries
+ JOIN Distribution
+ ON DistroSeries.distribution = Distribution.id
+ WHERE Distribution.enable_bug_expiration IS TRUE""",
+ IProduct: """
+ SELECT NULL, NULL, Product.id, NULL,
+ NULL, Product.id
+ FROM Product
+ WHERE Product.enable_bug_expiration IS TRUE""",
+ IProductSeries: """
+ SELECT NULL, NULL, NULL, ProductSeries.id,
+ NULL, Product.id
+ FROM ProductSeries
+ JOIN Product
+ ON ProductSeries.Product = Product.id
+ WHERE Product.enable_bug_expiration IS TRUE""",
+ }
+
+ TARGET_JOIN_CLAUSE = {
+ IDistribution: "BugTask.distribution = target.distribution",
+ IDistroSeries: "BugTask.distroseries = target.distroseries",
+ IProduct: "BugTask.product = target.product",
+ IProductSeries: "BugTask.productseries = target.productseries",
+ }
+
+ def _getJoinForTargets(self, *targets):
+ """Build the UNION of the sub-query for the given set of targets."""
+ selects = ' UNION '.join(
+ self.TARGET_SELECT[target] for target in targets)
+ join_clause = ' OR '.join(
+ self.TARGET_JOIN_CLAUSE[target] for target in targets)
+ return """
+ JOIN (
+ -- We create this rather bizarre looking structure
+ -- because we must replicate the behaviour of BugTask since
+ -- we are joining to it. So when distroseries is set,
+ -- distribution should be NULL. The two pillar columns will
+ -- be used in the WHERE clause.
+ SELECT 0 AS distribution, 0 AS distroseries,
+ 0 AS product , 0 AS productseries,
+ 0 AS distribution_pillar, 0 AS product_pillar
+ UNION %s
+ ) target
+ ON (%s)""" % (selects, join_clause)
+
def _getTargetJoinAndClause(self, target):
"""Return a SQL join clause to a `BugTarget`.
@@ -2632,54 +2686,23 @@
:raises AssertionError: If the target is not a known implementer of
`IBugTarget`
"""
- target_join = """
- JOIN (
- -- We create this rather bizarre looking structure
- -- because we must replicate the behaviour of BugTask since
- -- we are joining to it. So when distroseries is set,
- -- distribution should be NULL. The two pillar columns will
- -- be used in the WHERE clause.
- SELECT 0 AS distribution, 0 AS distroseries,
- 0 AS product , 0 AS productseries,
- 0 AS distribution_pillar, 0 AS product_pillar
- UNION
- SELECT Distribution.id, NULL, NULL, NULL,
- Distribution.id, NULL
- FROM Distribution
- WHERE Distribution.enable_bug_expiration IS TRUE
- UNION
- SELECT NULL, DistroSeries.id, NULL, NULL,
- Distribution.id, NULL
- FROM DistroSeries
- JOIN Distribution
- ON DistroSeries.distribution = Distribution.id
- WHERE Distribution.enable_bug_expiration IS TRUE
- UNION
- SELECT NULL, NULL, Product.id, NULL,
- NULL, Product.id
- FROM Product
- WHERE Product.enable_bug_expiration IS TRUE
- UNION
- SELECT NULL, NULL, NULL, ProductSeries.id,
- NULL, Product.id
- FROM ProductSeries
- JOIN Product
- ON ProductSeries.Product = Product.id
- WHERE Product.enable_bug_expiration IS TRUE) target
- ON (BugTask.distribution = target.distribution
- OR BugTask.distroseries = target.distroseries
- OR BugTask.product = target.product
- OR BugTask.productseries = target.productseries)"""
if target is None:
+ target_join = self._getJoinForTargets(
+ IDistribution, IDistroSeries, IProduct, IProductSeries)
target_clause = "TRUE IS TRUE"
elif IDistribution.providedBy(target):
+ target_join = self._getJoinForTargets(
+ IDistribution, IDistroSeries)
target_clause = "target.distribution_pillar = %s" % sqlvalues(
target)
elif IDistroSeries.providedBy(target):
+ target_join = self._getJoinForTargets(IDistroSeries)
target_clause = "BugTask.distroseries = %s" % sqlvalues(target)
elif IProduct.providedBy(target):
+ target_join = self._getJoinForTargets(IProduct, IProductSeries)
target_clause = "target.product_pillar = %s" % sqlvalues(target)
elif IProductSeries.providedBy(target):
+ target_join = self._getJoinForTargets(IProductSeries)
target_clause = "BugTask.productseries = %s" % sqlvalues(target)
elif (IProjectGroup.providedBy(target)
or ISourcePackage.providedBy(target)
=== modified file 'lib/lp/bugs/model/tests/test_bugtask.py'
--- lib/lp/bugs/model/tests/test_bugtask.py 2010-11-12 03:58:03 +0000
+++ lib/lp/bugs/model/tests/test_bugtask.py 2011-01-10 11:19:16 +0000
@@ -1204,14 +1204,16 @@
Four BugTarget types may passed as the target argument:
Distribution, DistroSeries, Product, ProductSeries.
"""
- supported_targets = [self.distribution, self.distroseries,
- self.product, self.productseries]
- for target in supported_targets:
+ supported_targets_and_task_count = [
+ (self.distribution, 2), (self.distroseries, 1), (self.product, 2),
+ (self.productseries, 1), (None, 4)]
+ for target, expected_count in supported_targets_and_task_count:
expirable_bugtasks = self.bugtaskset.findExpirableBugTasks(
0, self.user, target=target)
- self.assertNotEqual(expirable_bugtasks.count(), 0,
- "%s has %d expirable bugtasks." %
- (self.distroseries, expirable_bugtasks.count()))
+ self.assertEqual(expected_count, expirable_bugtasks.count(),
+ "%s has %d expirable bugtasks, expected %d." %
+ (self.distroseries, expirable_bugtasks.count(),
+ expected_count))
def testUnsupportedBugTargetParam(self):
"""Test that unsupported targets raise errors.
