Launchpad itself

Merge lp:~wgrant/launchpad/bug-590708-getBuildsByArchIds-timeout into lp:launchpad

  1. bug-590708-getBuildsByArchIds-timeout
  2. Merge into devel
Proposed by William Grant
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 11318
Proposed branch: lp:~wgrant/launchpad/bug-590708-getBuildsByArchIds-timeout
Merge into: lp:launchpad
Diff against target: 151 lines (+20/-21)
6 files modified
lib/lp/registry/model/distribution.py (+1/-1)
lib/lp/registry/model/distroseries.py (+1/-1)
lib/lp/soyuz/doc/binarypackagebuild.txt (+9/-9)
lib/lp/soyuz/interfaces/binarypackagebuild.py (+2/-1)
lib/lp/soyuz/model/binarypackagebuild.py (+5/-8)
lib/lp/soyuz/model/distroarchseries.py (+2/-1)
To merge this branch: bzr merge lp:~wgrant/launchpad/bug-590708-getBuildsByArchIds-timeout
Reviewer Review Type Date Requested Status
Julian Edwards (community) release-critical Approve
Graham Binns (community) code Approve
Review via email: mp+32181@code.launchpad.net

Commit message

Massively speed up the getBuildsByArchIds query, preventing getBuildRecords timeouts.

Description of the change

This branch fixes getBuildRecords API timeouts (bug #590708) by working around a really bad query plan.

In the current code, the query produced by getBuildsByArchIds takes roughly 6 seconds to run (see the last analysis on http://pastebin.com/AT04PS1P). This turns out to be because of the join against Archive, which somehow throws the planner's cost estimates off. Replacing it with a basic equivalent subquery does not help significantly, but an unnecessarily complicated one speeds it up massively (https://bugs.edge.launchpad.net/soyuz/+bug/590708/comments/8). There's further discussion in the bug which I shall not repeat here.

Rather than taking that route, however, I opted to use a solution employed frequently elsewhere in Soyuz: precalculating the archive IDs from the distribution, and passing them into the query directly. This takes it down to ~280ms on staging, and ~170ms on production (http://paste.ubuntu.com/475813/).

To post a comment you must log in.
Revision history for this message
Graham Binns (gmb) :
review: Approve (code)
Revision history for this message
Julian Edwards (julian-edwards) wrote :

I'm really happy to see another timeout nailed.

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/registry/model/distribution.py'
2--- lib/lp/registry/model/distribution.py 2010-08-03 14:59:22 +0000
3+++ lib/lp/registry/model/distribution.py 2010-08-10 11:01:15 +0000
4@@ -817,7 +817,7 @@
5 # Use the facility provided by IBinaryPackageBuildSet to
6 # retrieve the records.
7 return getUtility(IBinaryPackageBuildSet).getBuildsByArchIds(
8- arch_ids, build_state, name, pocket)
9+ self, arch_ids, build_state, name, pocket)
10
11 def getSourcePackageCaches(self, archive=None):
12 """See `IDistribution`."""
13
14=== modified file 'lib/lp/registry/model/distroseries.py'
15--- lib/lp/registry/model/distroseries.py 2010-08-10 02:56:06 +0000
16+++ lib/lp/registry/model/distroseries.py 2010-08-10 11:01:15 +0000
17@@ -1130,7 +1130,7 @@
18 # Use the facility provided by IBinaryPackageBuildSet to
19 # retrieve the records.
20 return getUtility(IBinaryPackageBuildSet).getBuildsByArchIds(
21- arch_ids, build_state, name, pocket)
22+ self.distribution, arch_ids, build_state, name, pocket)
23
24 def createUploadedSourcePackageRelease(
25 self, sourcepackagename, version, maintainer, builddepends,
26
27=== modified file 'lib/lp/soyuz/doc/binarypackagebuild.txt'
28--- lib/lp/soyuz/doc/binarypackagebuild.txt 2010-07-17 12:56:41 +0000
29+++ lib/lp/soyuz/doc/binarypackagebuild.txt 2010-08-10 11:01:15 +0000
30@@ -422,33 +422,33 @@
31
32 >>> hoary = ubuntu.getSeries('hoary')
33 >>> arch_ids = [arch.id for arch in hoary.architectures]
34- >>> bs.getBuildsByArchIds(arch_ids).count()
35+ >>> bs.getBuildsByArchIds(ubuntu, arch_ids).count()
36 5
37
38 It still working for empty list or None:
39
40- >>> bs.getBuildsByArchIds([]).count()
41+ >>> bs.getBuildsByArchIds(ubuntu, []).count()
42 0
43
44- >>> bs.getBuildsByArchIds(None).count()
45+ >>> bs.getBuildsByArchIds(ubuntu, None).count()
46 0
47
48 Using build status, only the successfully built ones:
49
50- >>> bs.getBuildsByArchIds(arch_ids,
51+ >>> bs.getBuildsByArchIds(ubuntu, arch_ids,
52 ... status=BuildStatus.FULLYBUILT).count()
53 2
54
55 Check the result content:
56
57- >>> [b.title for b in bs.getBuildsByArchIds(arch_ids,
58+ >>> [b.title for b in bs.getBuildsByArchIds(ubuntu, arch_ids,
59 ... status=BuildStatus.FULLYBUILT)]
60 [u'i386 build of pmount 0.1-1 in ubuntu hoary RELEASE', u'hppa build
61 of pmount 0.1-1 in ubuntu hoary RELEASE']
62
63 Using optional 'name' filter (matching with SQL LIKE %||filter||%)
64
65- >>> bs.getBuildsByArchIds(arch_ids,
66+ >>> bs.getBuildsByArchIds(ubuntu, arch_ids,
67 ... status=BuildStatus.FULLYBUILT,
68 ... name='pmo').count()
69 2
70@@ -456,11 +456,11 @@
71 Checking optional 'pocket' restriction:
72
73 >>> from lp.registry.interfaces.pocket import PackagePublishingPocket
74- >>> bs.getBuildsByArchIds(arch_ids,
75+ >>> bs.getBuildsByArchIds(ubuntu, arch_ids,
76 ... pocket=PackagePublishingPocket.UPDATES).count()
77 0
78
79- >>> bs.getBuildsByArchIds(arch_ids,
80+ >>> bs.getBuildsByArchIds(ubuntu, arch_ids,
81 ... pocket=PackagePublishingPocket.RELEASE).count()
82 5
83
84@@ -470,7 +470,7 @@
85 >>> breezy = ubuntu.getSeries('breezy-autotest')
86 >>> arch_ids = [arch.id for arch in breezy.architectures]
87 >>> [(build.archive.purpose.name, build.title) for build in
88- ... bs.getBuildsByArchIds(arch_ids, name='commercialpackage')]
89+ ... bs.getBuildsByArchIds(ubuntu, arch_ids, name='commercialpackage')]
90 [('PARTNER', u'i386 build of commercialpackage 1.0-1 in ubuntu breezy-autotest RELEASE')]
91
92 `IBinaryPackageBuildSet` also provides getStatusSummaryForBuilds which
93
94=== modified file 'lib/lp/soyuz/interfaces/binarypackagebuild.py'
95--- lib/lp/soyuz/interfaces/binarypackagebuild.py 2010-06-14 09:31:26 +0000
96+++ lib/lp/soyuz/interfaces/binarypackagebuild.py 2010-08-10 11:01:15 +0000
97@@ -276,7 +276,8 @@
98 :return: a `ResultSet` representing the requested builds.
99 """
100
101- def getBuildsByArchIds(arch_ids, status=None, name=None, pocket=None):
102+ def getBuildsByArchIds(distribution, arch_ids, status=None, name=None,
103+ pocket=None):
104 """Retrieve Build Records for a given arch_ids list.
105
106 Optionally, for a given status and/or pocket, if ommited return all
107
108=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
109--- lib/lp/soyuz/model/binarypackagebuild.py 2010-08-02 02:13:52 +0000
110+++ lib/lp/soyuz/model/binarypackagebuild.py 2010-08-10 11:01:15 +0000
111@@ -878,8 +878,8 @@
112 BinaryPackageBuild.select(
113 clause, clauseTables=clauseTables, orderBy=orderBy))
114
115- def getBuildsByArchIds(self, arch_ids, status=None, name=None,
116- pocket=None):
117+ def getBuildsByArchIds(self, distribution, arch_ids, status=None,
118+ name=None, pocket=None):
119 """See `IBinaryPackageBuildSet`."""
120 # If not distroarchseries was found return empty list
121 if not arch_ids:
122@@ -938,12 +938,9 @@
123
124 # Only pick builds from the distribution's main archive to
125 # exclude PPA builds
126- clauseTables.append("Archive")
127- condition_clauses.append("""
128- Archive.purpose IN (%s) AND
129- Archive.id = PackageBuild.archive
130- """ % ','.join(
131- sqlvalues(ArchivePurpose.PRIMARY, ArchivePurpose.PARTNER)))
132+ condition_clauses.append(
133+ "PackageBuild.archive IN %s" %
134+ sqlvalues(list(distribution.all_distro_archive_ids)))
135
136 return self._decorate_with_prejoins(
137 BinaryPackageBuild.select(' AND '.join(condition_clauses),
138
139=== modified file 'lib/lp/soyuz/model/distroarchseries.py'
140--- lib/lp/soyuz/model/distroarchseries.py 2010-06-21 07:26:51 +0000
141+++ lib/lp/soyuz/model/distroarchseries.py 2010-08-10 11:01:15 +0000
142@@ -245,7 +245,8 @@
143 # Use the facility provided by IBinaryPackageBuildSet to
144 # retrieve the records.
145 return getUtility(IBinaryPackageBuildSet).getBuildsByArchIds(
146- [self.id], build_state, name, pocket)
147+ self.distroseries.distribution, [self.id], build_state, name,
148+ pocket)
149
150 def getReleasedPackages(self, binary_name, pocket=None,
151 include_pending=False, exclude_pocket=None,