launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

William Grant has proposed merging lp:~wgrant/launchpad/upcoming-optimisation into lp:launchpad.

Commit message:
Optimise Person.getAssignedSpecificationWorkItemsDueBefore main query.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #1692120 in Launchpad itself: "Upcoming work pages appear to time out a lot"
  https://bugs.launchpad.net/launchpad/+bug/1692120

For more details, see:
https://code.launchpad.net/~wgrant/launchpad/upcoming-optimisation/+merge/325141

Optimise Person.getAssignedSpecificationWorkItemsDueBefore main query.

PostgreSQL was unable to effectively estimate the number of assigned
workitems when the conditions were combined. Moving that calculation
into a CTE acts as an optimisation barrier and forces PostgreSQL to
calculate the assigned set up front, making the rest much faster.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/upcoming-optimisation into lp:launchpad.

=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py	2017-04-22 13:09:01 +0000
+++ lib/lp/registry/model/person.py	2017-06-06 08:18:21 +0000
@@ -1434,6 +1434,32 @@
         from lp.registry.model.distribution import Distribution
         store = Store.of(self)
         WorkItem = SpecificationWorkItem
+
+        # Since a workitem's assignee defaults to its specification's
+        # assignee, the PostgreSQL planner isn't always able to work out
+        # the selectivity of the filter. Put that in a CTE to force it
+        # to calculate the workitems up front, rather than doing a hash
+        # join over all of Specification and SpecificationWorkItem.
+        assigned_specificationworkitem = With(
+            'assigned_specificationworkitem',
+            Union(
+                Select(
+                    SpecificationWorkItem.id,
+                    where=And(
+                        SpecificationWorkItem.assignee_id.is_in(
+                            self.participant_ids),
+                        Not(SpecificationWorkItem.deleted))),
+                Select(
+                    SpecificationWorkItem.id,
+                    where=And(
+                        SpecificationWorkItem.specification_id.is_in(
+                            Select(
+                                Specification.id,
+                                where=Specification._assigneeID.is_in(
+                                    self.participant_ids))),
+                        Not(SpecificationWorkItem.deleted))),
+                all=True))
+
         origin = [Specification]
         productjoin, query = get_specification_active_product_filter(self)
         origin.extend(productjoin)
@@ -1448,10 +1474,10 @@
         today = datetime.today().date()
         query.extend([
             Milestone.dateexpected <= date, Milestone.dateexpected >= today,
-            WorkItem.deleted == False,
-            Or(WorkItem.assignee_id.is_in(self.participant_ids),
-               Specification._assigneeID.is_in(self.participant_ids))])
-        result = store.using(*origin).find(WorkItem, *query)
+            WorkItem.id.is_in(Select(
+                SQL('id'), tables='assigned_specificationworkitem'))])
+        result = store.with_(assigned_specificationworkitem).using(
+            *origin).find(WorkItem, *query)
         result.config(distinct=True)
 
         def eager_load(workitems):