Merge lp:~wgrant/launchpad/upcoming-optimisation into lp:launchpad

Proposed by William Grant on 2017-06-06
Status: Merged
Merged at revision: 18400
Proposed branch: lp:~wgrant/launchpad/upcoming-optimisation
Merge into: lp:launchpad
Diff against target: 62 lines (+33/-7)
1 file modified
lib/lp/registry/model/person.py (+33/-7)
To merge this branch: bzr merge lp:~wgrant/launchpad/upcoming-optimisation
Reviewer Review Type Date Requested Status
Colin Watson 2017-06-06 Approve on 2017-06-06
Review via email: mp+325141@code.launchpad.net

Commit message

Optimise Person.getAssignedSpecificationWorkItemsDueBefore main query.

Description of the change

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.

To post a comment you must log in.
Colin Watson (cjwatson) :
review: Approve
William Grant (wgrant) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/model/person.py'
2--- lib/lp/registry/model/person.py 2017-04-22 13:09:01 +0000
3+++ lib/lp/registry/model/person.py 2017-06-06 11:13:31 +0000
4@@ -1433,25 +1433,51 @@
5 from lp.registry.model.product import Product
6 from lp.registry.model.distribution import Distribution
7 store = Store.of(self)
8- WorkItem = SpecificationWorkItem
9+
10+ # Since a workitem's assignee defaults to its specification's
11+ # assignee, the PostgreSQL planner isn't always able to work out
12+ # the selectivity of the filter. Put that in a CTE to force it
13+ # to calculate the workitems up front, rather than doing a hash
14+ # join over all of Specification and SpecificationWorkItem.
15+ assigned_specificationworkitem = With(
16+ 'assigned_specificationworkitem',
17+ Union(
18+ Select(
19+ SpecificationWorkItem.id,
20+ where=And(
21+ SpecificationWorkItem.assignee_id.is_in(
22+ self.participant_ids),
23+ Not(SpecificationWorkItem.deleted))),
24+ Select(
25+ SpecificationWorkItem.id,
26+ where=And(
27+ SpecificationWorkItem.specification_id.is_in(
28+ Select(
29+ Specification.id,
30+ where=Specification._assigneeID.is_in(
31+ self.participant_ids))),
32+ Not(SpecificationWorkItem.deleted))),
33+ all=True))
34+
35 origin = [Specification]
36 productjoin, query = get_specification_active_product_filter(self)
37 origin.extend(productjoin)
38 query.extend(get_specification_privacy_filter(user))
39 origin.extend([
40- Join(WorkItem, WorkItem.specification == Specification.id),
41+ Join(SpecificationWorkItem,
42+ SpecificationWorkItem.specification == Specification.id),
43 # WorkItems may not have a milestone and in that case they inherit
44 # the one from the spec.
45 Join(Milestone,
46- Coalesce(WorkItem.milestone_id,
47+ Coalesce(SpecificationWorkItem.milestone_id,
48 Specification.milestoneID) == Milestone.id)])
49 today = datetime.today().date()
50 query.extend([
51 Milestone.dateexpected <= date, Milestone.dateexpected >= today,
52- WorkItem.deleted == False,
53- Or(WorkItem.assignee_id.is_in(self.participant_ids),
54- Specification._assigneeID.is_in(self.participant_ids))])
55- result = store.using(*origin).find(WorkItem, *query)
56+ SpecificationWorkItem.id.is_in(Select(
57+ SQL('id'), tables='assigned_specificationworkitem'))])
58+ result = store.with_(assigned_specificationworkitem).using(
59+ *origin).find(SpecificationWorkItem, *query)
60 result.config(distinct=True)
61
62 def eager_load(workitems):