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
=== modified file 'lib/lp/registry/model/distribution.py'
--- lib/lp/registry/model/distribution.py 2010-08-03 14:59:22 +0000
+++ lib/lp/registry/model/distribution.py 2010-08-10 11:01:15 +0000
@@ -817,7 +817,7 @@
817 # Use the facility provided by IBinaryPackageBuildSet to817 # Use the facility provided by IBinaryPackageBuildSet to
818 # retrieve the records.818 # retrieve the records.
819 return getUtility(IBinaryPackageBuildSet).getBuildsByArchIds(819 return getUtility(IBinaryPackageBuildSet).getBuildsByArchIds(
820 arch_ids, build_state, name, pocket)820 self, arch_ids, build_state, name, pocket)
821821
822 def getSourcePackageCaches(self, archive=None):822 def getSourcePackageCaches(self, archive=None):
823 """See `IDistribution`."""823 """See `IDistribution`."""
824824
=== modified file 'lib/lp/registry/model/distroseries.py'
--- lib/lp/registry/model/distroseries.py 2010-08-10 02:56:06 +0000
+++ lib/lp/registry/model/distroseries.py 2010-08-10 11:01:15 +0000
@@ -1130,7 +1130,7 @@
1130 # Use the facility provided by IBinaryPackageBuildSet to1130 # Use the facility provided by IBinaryPackageBuildSet to
1131 # retrieve the records.1131 # retrieve the records.
1132 return getUtility(IBinaryPackageBuildSet).getBuildsByArchIds(1132 return getUtility(IBinaryPackageBuildSet).getBuildsByArchIds(
1133 arch_ids, build_state, name, pocket)1133 self.distribution, arch_ids, build_state, name, pocket)
11341134
1135 def createUploadedSourcePackageRelease(1135 def createUploadedSourcePackageRelease(
1136 self, sourcepackagename, version, maintainer, builddepends,1136 self, sourcepackagename, version, maintainer, builddepends,
11371137
=== modified file 'lib/lp/soyuz/doc/binarypackagebuild.txt'
--- lib/lp/soyuz/doc/binarypackagebuild.txt 2010-07-17 12:56:41 +0000
+++ lib/lp/soyuz/doc/binarypackagebuild.txt 2010-08-10 11:01:15 +0000
@@ -422,33 +422,33 @@
422422
423 >>> hoary = ubuntu.getSeries('hoary')423 >>> hoary = ubuntu.getSeries('hoary')
424 >>> arch_ids = [arch.id for arch in hoary.architectures]424 >>> arch_ids = [arch.id for arch in hoary.architectures]
425 >>> bs.getBuildsByArchIds(arch_ids).count()425 >>> bs.getBuildsByArchIds(ubuntu, arch_ids).count()
426 5426 5
427427
428It still working for empty list or None:428It still working for empty list or None:
429429
430 >>> bs.getBuildsByArchIds([]).count()430 >>> bs.getBuildsByArchIds(ubuntu, []).count()
431 0431 0
432432
433 >>> bs.getBuildsByArchIds(None).count()433 >>> bs.getBuildsByArchIds(ubuntu, None).count()
434 0434 0
435435
436Using build status, only the successfully built ones:436Using build status, only the successfully built ones:
437437
438 >>> bs.getBuildsByArchIds(arch_ids,438 >>> bs.getBuildsByArchIds(ubuntu, arch_ids,
439 ... status=BuildStatus.FULLYBUILT).count()439 ... status=BuildStatus.FULLYBUILT).count()
440 2440 2
441441
442Check the result content:442Check the result content:
443443
444 >>> [b.title for b in bs.getBuildsByArchIds(arch_ids,444 >>> [b.title for b in bs.getBuildsByArchIds(ubuntu, arch_ids,
445 ... status=BuildStatus.FULLYBUILT)]445 ... status=BuildStatus.FULLYBUILT)]
446 [u'i386 build of pmount 0.1-1 in ubuntu hoary RELEASE', u'hppa build446 [u'i386 build of pmount 0.1-1 in ubuntu hoary RELEASE', u'hppa build
447 of pmount 0.1-1 in ubuntu hoary RELEASE']447 of pmount 0.1-1 in ubuntu hoary RELEASE']
448448
449Using optional 'name' filter (matching with SQL LIKE %||filter||%)449Using optional 'name' filter (matching with SQL LIKE %||filter||%)
450450
451 >>> bs.getBuildsByArchIds(arch_ids,451 >>> bs.getBuildsByArchIds(ubuntu, arch_ids,
452 ... status=BuildStatus.FULLYBUILT,452 ... status=BuildStatus.FULLYBUILT,
453 ... name='pmo').count()453 ... name='pmo').count()
454 2454 2
@@ -456,11 +456,11 @@
456Checking optional 'pocket' restriction:456Checking optional 'pocket' restriction:
457457
458 >>> from lp.registry.interfaces.pocket import PackagePublishingPocket458 >>> from lp.registry.interfaces.pocket import PackagePublishingPocket
459 >>> bs.getBuildsByArchIds(arch_ids,459 >>> bs.getBuildsByArchIds(ubuntu, arch_ids,
460 ... pocket=PackagePublishingPocket.UPDATES).count()460 ... pocket=PackagePublishingPocket.UPDATES).count()
461 0461 0
462462
463 >>> bs.getBuildsByArchIds(arch_ids,463 >>> bs.getBuildsByArchIds(ubuntu, arch_ids,
464 ... pocket=PackagePublishingPocket.RELEASE).count()464 ... pocket=PackagePublishingPocket.RELEASE).count()
465 5465 5
466466
@@ -470,7 +470,7 @@
470 >>> breezy = ubuntu.getSeries('breezy-autotest')470 >>> breezy = ubuntu.getSeries('breezy-autotest')
471 >>> arch_ids = [arch.id for arch in breezy.architectures]471 >>> arch_ids = [arch.id for arch in breezy.architectures]
472 >>> [(build.archive.purpose.name, build.title) for build in472 >>> [(build.archive.purpose.name, build.title) for build in
473 ... bs.getBuildsByArchIds(arch_ids, name='commercialpackage')]473 ... bs.getBuildsByArchIds(ubuntu, arch_ids, name='commercialpackage')]
474 [('PARTNER', u'i386 build of commercialpackage 1.0-1 in ubuntu breezy-autotest RELEASE')]474 [('PARTNER', u'i386 build of commercialpackage 1.0-1 in ubuntu breezy-autotest RELEASE')]
475475
476`IBinaryPackageBuildSet` also provides getStatusSummaryForBuilds which476`IBinaryPackageBuildSet` also provides getStatusSummaryForBuilds which
477477
=== modified file 'lib/lp/soyuz/interfaces/binarypackagebuild.py'
--- lib/lp/soyuz/interfaces/binarypackagebuild.py 2010-06-14 09:31:26 +0000
+++ lib/lp/soyuz/interfaces/binarypackagebuild.py 2010-08-10 11:01:15 +0000
@@ -276,7 +276,8 @@
276 :return: a `ResultSet` representing the requested builds.276 :return: a `ResultSet` representing the requested builds.
277 """277 """
278278
279 def getBuildsByArchIds(arch_ids, status=None, name=None, pocket=None):279 def getBuildsByArchIds(distribution, arch_ids, status=None, name=None,
280 pocket=None):
280 """Retrieve Build Records for a given arch_ids list.281 """Retrieve Build Records for a given arch_ids list.
281282
282 Optionally, for a given status and/or pocket, if ommited return all283 Optionally, for a given status and/or pocket, if ommited return all
283284
=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py 2010-08-02 02:13:52 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py 2010-08-10 11:01:15 +0000
@@ -878,8 +878,8 @@
878 BinaryPackageBuild.select(878 BinaryPackageBuild.select(
879 clause, clauseTables=clauseTables, orderBy=orderBy))879 clause, clauseTables=clauseTables, orderBy=orderBy))
880880
881 def getBuildsByArchIds(self, arch_ids, status=None, name=None,881 def getBuildsByArchIds(self, distribution, arch_ids, status=None,
882 pocket=None):882 name=None, pocket=None):
883 """See `IBinaryPackageBuildSet`."""883 """See `IBinaryPackageBuildSet`."""
884 # If not distroarchseries was found return empty list884 # If not distroarchseries was found return empty list
885 if not arch_ids:885 if not arch_ids:
@@ -938,12 +938,9 @@
938938
939 # Only pick builds from the distribution's main archive to939 # Only pick builds from the distribution's main archive to
940 # exclude PPA builds940 # exclude PPA builds
941 clauseTables.append("Archive")941 condition_clauses.append(
942 condition_clauses.append("""942 "PackageBuild.archive IN %s" %
943 Archive.purpose IN (%s) AND943 sqlvalues(list(distribution.all_distro_archive_ids)))
944 Archive.id = PackageBuild.archive
945 """ % ','.join(
946 sqlvalues(ArchivePurpose.PRIMARY, ArchivePurpose.PARTNER)))
947944
948 return self._decorate_with_prejoins(945 return self._decorate_with_prejoins(
949 BinaryPackageBuild.select(' AND '.join(condition_clauses),946 BinaryPackageBuild.select(' AND '.join(condition_clauses),
950947
=== modified file 'lib/lp/soyuz/model/distroarchseries.py'
--- lib/lp/soyuz/model/distroarchseries.py 2010-06-21 07:26:51 +0000
+++ lib/lp/soyuz/model/distroarchseries.py 2010-08-10 11:01:15 +0000
@@ -245,7 +245,8 @@
245 # Use the facility provided by IBinaryPackageBuildSet to245 # Use the facility provided by IBinaryPackageBuildSet to
246 # retrieve the records.246 # retrieve the records.
247 return getUtility(IBinaryPackageBuildSet).getBuildsByArchIds(247 return getUtility(IBinaryPackageBuildSet).getBuildsByArchIds(
248 [self.id], build_state, name, pocket)248 self.distroseries.distribution, [self.id], build_state, name,
249 pocket)
249250
250 def getReleasedPackages(self, binary_name, pocket=None,251 def getReleasedPackages(self, binary_name, pocket=None,
251 include_pending=False, exclude_pocket=None,252 include_pending=False, exclude_pocket=None,