launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #05458
[Merge] lp:~rvb/launchpad/builders-timeout-bug-887078 into lp:launchpad
Raphaël Badin has proposed merging lp:~rvb/launchpad/builders-timeout-bug-887078 into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #887078 in Launchpad itself: "Builder:+history timeout"
https://bugs.launchpad.net/launchpad/+bug/887078
For more details, see:
https://code.launchpad.net/~rvb/launchpad/builders-timeout-bug-887078/+merge/81734
This branch is the first in a series of branches aiming to improve the performance of the page builder:+history. This branch adds a method getSpecificJobs to be able to load jobs related to builds in batches. The trick is that the page aggregates different build types so we have to create a method (getByBuildFarmJobs) for each of the possible build types.
= Tests =
./bin/test -vvc test_buildfarmjob test_getSpecificJobs
./bin/test -vvc test_buildfarmjob test_getSpecificJobs_empty
./bin/test -vvc test_buildfarmjob test_getSpecificJobs_sql_queries_count
= Q/A =
None.
--
https://code.launchpad.net/~rvb/launchpad/builders-timeout-bug-887078/+merge/81734
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/builders-timeout-bug-887078 into lp:launchpad.
=== modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
--- lib/lp/buildmaster/interfaces/buildfarmjob.py 2011-10-28 10:26:04 +0000
+++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2011-11-09 14:21:43 +0000
@@ -16,9 +16,7 @@
'ISpecificBuildFarmJobSource',
]
-from lazr.enum import (
- DBEnumeratedType,
- )
+from lazr.enum import DBEnumeratedType
from lazr.restful.declarations import exported
from lazr.restful.fields import Reference
from zope.interface import (
@@ -291,6 +289,13 @@
:param id: An ID of the concrete job class to look up.
"""
+ def getByBuildFarmJobs(build_farm_jobs):
+ """"Look up the concrete `IBuildFarmJob`s for a list of BuildFarmJobs.
+
+ :param build_farm_jobs: A list of BuildFarmJobs for which to get the
+ concrete jobs.
+ """
+
def getByBuildFarmJob(build_farm_job):
""""Look up the concrete `IBuildFarmJob` for a BuildFarmJob.
@@ -317,6 +322,12 @@
class IBuildFarmJobSet(Interface):
"""A utility representing a set of build farm jobs."""
+ def getSpecificJobs(jobs):
+ """Return the specific build jobs associated with each of the jobs
+ in the provided job list.
+
+ """
+
def getBuildsForBuilder(builder_id, status=None, user=None):
"""Return `IBuildFarmJob` records touched by a builder.
=== modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
--- lib/lp/buildmaster/model/buildfarmjob.py 2011-10-31 09:23:47 +0000
+++ lib/lp/buildmaster/model/buildfarmjob.py 2011-11-09 14:21:43 +0000
@@ -11,6 +11,8 @@
import hashlib
+from itertools import groupby
+from operator import attrgetter
from lazr.delegates import delegates
import pytz
@@ -396,6 +398,19 @@
class BuildFarmJobSet:
implements(IBuildFarmJobSet)
+ def getSpecificJobs(self, jobs):
+ """See `IBuildFarmJobSet`."""
+ # Adapt a list of jobs based on their job type.
+ builds = []
+ key = lambda x: attrgetter('name')(attrgetter('job_type')(x))
+ sorted_jobs = sorted(jobs, key=key)
+ for job_type_name, grouped_jobs in groupby(sorted_jobs, key=key):
+ # Fetch the jobs in batches grouped by their job type.
+ source = getUtility(
+ ISpecificBuildFarmJobSource, job_type_name)
+ builds.extend(list(source.getByBuildFarmJobs(list(grouped_jobs))))
+ return builds
+
def getBuildsForBuilder(self, builder_id, status=None, user=None):
"""See `IBuildFarmJobSet`."""
# Imported here to avoid circular imports.
=== modified file 'lib/lp/buildmaster/tests/test_buildfarmjob.py'
--- lib/lp/buildmaster/tests/test_buildfarmjob.py 2011-10-28 10:26:04 +0000
+++ lib/lp/buildmaster/tests/test_buildfarmjob.py 2011-11-09 14:21:43 +0000
@@ -12,6 +12,7 @@
import pytz
from storm.store import Store
+from testtools.matchers import Equals
from zope.component import getUtility
from zope.security.interfaces import Unauthorized
from zope.security.proxy import removeSecurityProxy
@@ -34,10 +35,17 @@
InconsistentBuildFarmJobError,
)
from lp.buildmaster.model.buildfarmjob import BuildFarmJob
+from lp.soyuz.enums import PackagePublishingStatus
+from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
from lp.testing import (
login,
+ StormStatementRecorder,
TestCaseWithFactory,
)
+from lp.testing.matchers import HasQueryCount
+from lp.translations.interfaces.translationtemplatesbuild import (
+ ITranslationTemplatesBuildSource,
+ )
class TestBuildFarmJobMixin:
@@ -181,7 +189,7 @@
# date_created can be passed optionally when creating a
# bulid farm job to ensure we don't get identical timestamps
# when transactions are committed.
- ten_years_ago = datetime.now(pytz.UTC) - timedelta(365*10)
+ ten_years_ago = datetime.now(pytz.UTC) - timedelta(365 * 10)
build_farm_job = getUtility(IBuildFarmJobSource).new(
job_type=BuildFarmJobType.PACKAGEBUILD,
date_created=ten_years_ago)
@@ -229,6 +237,49 @@
self.builder = self.factory.makeBuilder()
self.build_farm_job_set = getUtility(IBuildFarmJobSet)
+ def createTranslationTemplateBuild(self):
+ build_farm_job_source = getUtility(IBuildFarmJobSource)
+ build_farm_job = build_farm_job_source.new(
+ BuildFarmJobType.TRANSLATIONTEMPLATESBUILD)
+ source = getUtility(ITranslationTemplatesBuildSource)
+ branch = self.factory.makeBranch()
+ return source.create(build_farm_job, branch)
+
+ def createSourcePackageRecipeBuild(self):
+ sprb = self.factory.makeSourcePackageRecipeBuild()
+ Store.of(sprb).flush()
+ return sprb
+
+ def createBuilds(self):
+ builds = []
+ for i in xrange(10):
+ # We don't create binary package builds because the test
+ # would be really heavy to setup.
+ builds.append(self.createTranslationTemplateBuild())
+ builds.append(self.createSourcePackageRecipeBuild())
+ return builds
+
+ def test_getSpecificJobs(self):
+ builds = self.createBuilds()
+ specific_jobs = self.build_farm_job_set.getSpecificJobs(
+ [build.build_farm_job for build in builds])
+ self.assertContentEqual(
+ builds, specific_jobs)
+
+ def test_getSpecificJobs_empty(self):
+ self.assertContentEqual(
+ [],
+ self.build_farm_job_set.getSpecificJobs([]))
+
+ def test_getSpecificJobs_sql_queries_count(self):
+ # getSpecificJobs issues one query for each build type.
+ builds = self.createBuilds()
+ build_farm_jobs = [build.build_farm_job for build in builds]
+ with StormStatementRecorder() as recorder:
+ self.build_farm_job_set.getSpecificJobs(
+ build_farm_jobs)
+ self.assertThat(recorder, HasQueryCount(Equals(2)))
+
def test_getBuildsForBuilder_all(self):
# The default call without arguments returns all builds for the
# builder, and not those for other builders.
=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py 2011-10-26 02:14:52 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py 2011-11-09 14:21:43 +0000
@@ -23,7 +23,10 @@
Reference,
Storm,
)
-from storm.store import Store
+from storm.store import (
+ EmptyResultSet,
+ Store,
+ )
from zope.component import getUtility
from zope.interface import (
classProvides,
@@ -280,6 +283,17 @@
PackageBuild.build_farm_job_id == build_farm_job.id).one()
@classmethod
+ def getByBuildFarmJobs(cls, build_farm_jobs):
+ """See `ISpecificBuildFarmJobSource`."""
+ if len(build_farm_jobs) == 0:
+ return EmptyResultSet()
+ build_farm_job_ids = [
+ build_farm_job.id for build_farm_job in build_farm_jobs]
+ return Store.of(build_farm_jobs[0]).find(cls,
+ cls.package_build_id == PackageBuild.id,
+ PackageBuild.build_farm_job_id.is_in(build_farm_job_ids))
+
+ @classmethod
def getRecentBuilds(cls, requester, recipe, distroseries, _now=None):
from lp.buildmaster.model.buildfarmjob import BuildFarmJob
if _now is None:
=== modified file 'lib/lp/code/model/tests/test_sourcepackagerecipebuild.py'
--- lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2011-10-27 11:36:13 +0000
+++ lib/lp/code/model/tests/test_sourcepackagerecipebuild.py 2011-11-09 14:21:43 +0000
@@ -513,6 +513,26 @@
self.assertEquals(build.requester,
build.getUploader(None))
+ def test_getByBuildFarmJob(self):
+ sprb = self.makeSourcePackageRecipeBuild()
+ Store.of(sprb).flush()
+ self.assertEqual(
+ sprb,
+ SourcePackageRecipeBuild.getByBuildFarmJob(sprb.build_farm_job))
+
+ def test_getByBuildFarmJobs(self):
+ sprbs = [self.makeSourcePackageRecipeBuild() for i in range(10)]
+ Store.of(sprbs[0]).flush()
+ self.assertContentEqual(
+ sprbs,
+ SourcePackageRecipeBuild.getByBuildFarmJobs(
+ [sprb.build_farm_job for sprb in sprbs]))
+
+ def test_getByBuildFarmJobs_empty(self):
+ self.assertContentEqual(
+ [],
+ SourcePackageRecipeBuild.getByBuildFarmJobs([]))
+
class TestAsBuildmaster(TestCaseWithFactory):
=== modified file 'lib/lp/soyuz/browser/build.py'
--- lib/lp/soyuz/browser/build.py 2011-11-02 12:04:31 +0000
+++ lib/lp/soyuz/browser/build.py 2011-11-09 14:21:43 +0000
@@ -60,7 +60,8 @@
from lp.buildmaster.enums import BuildStatus
from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJobSet
from lp.code.interfaces.sourcepackagerecipebuild import (
- ISourcePackageRecipeBuildSource)
+ ISourcePackageRecipeBuildSource,
+ )
from lp.services.job.interfaces.job import JobStatus
from lp.services.propertycache import cachedproperty
from lp.soyuz.enums import PackageUploadStatus
=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py 2011-11-07 14:57:57 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py 2011-11-09 14:21:43 +0000
@@ -866,6 +866,19 @@
return None
return resulting_tuple[0]
+ def getByBuildFarmJobs(self, build_farm_jobs):
+ """See `ISpecificBuildFarmJobSource`."""
+ if len(build_farm_jobs) == 0:
+ return EmptyResultSet()
+ clause_tables = (BinaryPackageBuild, PackageBuild, BuildFarmJob)
+ build_farm_job_ids = [
+ build_farm_job.id for build_farm_job in build_farm_jobs]
+ return Store.of(build_farm_jobs[0]).using(*clause_tables).find(
+ BinaryPackageBuild,
+ BinaryPackageBuild.package_build == PackageBuild.id,
+ PackageBuild.build_farm_job == BuildFarmJob.id,
+ BuildFarmJob.id.is_in(build_farm_job_ids))
+
def getPendingBuildsForArchSet(self, archseries):
"""See `IBinaryPackageBuildSet`."""
if not archseries:
=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py 2011-11-01 14:07:57 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py 2011-11-09 14:21:43 +0000
@@ -10,14 +10,11 @@
import pytz
from storm.store import Store
+from twisted.trial.unittest import TestCase as TrialTestCase
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
-from twisted.trial.unittest import TestCase as TrialTestCase
-
-from canonical.launchpad.testing.pages import (
- webservice_for_person,
- )
+from canonical.launchpad.testing.pages import webservice_for_person
from canonical.launchpad.webapp.interaction import ANONYMOUS
from canonical.launchpad.webapp.interfaces import OAuthPermission
from canonical.testing.layers import (
@@ -432,6 +429,18 @@
getUtility(IBinaryPackageBuildSet).getByBuildFarmJob(
sprb.build_farm_job))
+ def test_getByBuildFarmJobs_works(self):
+ bpbs = [self.factory.makeBinaryPackageBuild() for i in xrange(10)]
+ self.assertContentEqual(
+ bpbs,
+ getUtility(IBinaryPackageBuildSet).getByBuildFarmJobs(
+ [bpb.build_farm_job for bpb in bpbs]))
+
+ def test_getByBuildFarmJobs_works_empty(self):
+ self.assertContentEqual(
+ [],
+ getUtility(IBinaryPackageBuildSet).getByBuildFarmJobs([]))
+
class TestBuildSetGetBuildsForArchive(BaseTestCaseWithThreeBuilds):
=== modified file 'lib/lp/translations/interfaces/translationtemplatesbuild.py'
--- lib/lp/translations/interfaces/translationtemplatesbuild.py 2011-05-04 03:49:28 +0000
+++ lib/lp/translations/interfaces/translationtemplatesbuild.py 2011-11-09 14:21:43 +0000
@@ -10,7 +10,6 @@
]
from lazr.restful.fields import Reference
-from zope.interface import Interface
from canonical.launchpad import _
from lp.buildmaster.interfaces.buildfarmjob import (
=== modified file 'lib/lp/translations/model/translationtemplatesbuild.py'
--- lib/lp/translations/model/translationtemplatesbuild.py 2011-10-19 07:42:57 +0000
+++ lib/lp/translations/model/translationtemplatesbuild.py 2011-11-09 14:21:43 +0000
@@ -107,6 +107,16 @@
return match.one()
@classmethod
+ def getByBuildFarmJobs(cls, buildfarmjobs, store=None):
+ buildfarmjob_ids = [buildfarmjob.id for buildfarmjob in buildfarmjobs]
+ """See `ITranslationTemplatesBuildSource`."""
+ store = cls._getStore(store)
+ return store.find(
+ TranslationTemplatesBuild,
+ TranslationTemplatesBuild.build_farm_job_id.is_in(
+ buildfarmjob_ids))
+
+ @classmethod
def findByBranch(cls, branch, store=None):
"""See `ITranslationTemplatesBuildSource`."""
store = cls._getStore(store)
=== modified file 'lib/lp/translations/tests/test_translationtemplatesbuild.py'
--- lib/lp/translations/tests/test_translationtemplatesbuild.py 2011-05-04 03:32:06 +0000
+++ lib/lp/translations/tests/test_translationtemplatesbuild.py 2011-11-09 14:21:43 +0000
@@ -22,12 +22,12 @@
ITranslationTemplatesBuild,
ITranslationTemplatesBuildSource,
)
+from lp.translations.interfaces.translationtemplatesbuildjob import (
+ ITranslationTemplatesBuildJobSource,
+ )
from lp.translations.model.translationtemplatesbuild import (
TranslationTemplatesBuild,
)
-from lp.translations.interfaces.translationtemplatesbuildjob import (
- ITranslationTemplatesBuildJobSource,
- )
class TestTranslationTemplatesBuild(TestCaseWithFactory):
@@ -59,7 +59,7 @@
self.layer.switchDbUser(config.branchscanner.dbuser)
utility = getUtility(ITranslationTemplatesBuildSource)
build_farm_job = self._makeBuildFarmJob()
- specific_job = utility.create(build_farm_job, branch)
+ utility.create(build_farm_job, branch)
# Writing the new objects to the database violates no access
# restrictions.
@@ -73,7 +73,7 @@
source = getUtility(ITranslationTemplatesBuildSource)
branch = self.factory.makeBranch()
- translationtemplatesbuildjob = jobset.create(branch)
+ jobset.create(branch)
builds = list(source.findByBranch(branch))
self.assertEqual(1, len(builds))
@@ -123,6 +123,21 @@
self.assertEqual(build, source.getByBuildFarmJob(build_farm_job))
+ def test_getByBuildFarmJobs(self):
+ source = getUtility(ITranslationTemplatesBuildSource)
+ build_farm_jobs = []
+ builds = []
+ for i in xrange(10):
+ build_farm_job = self._makeBuildFarmJob()
+ branch = self.factory.makeBranch()
+ build = source.create(build_farm_job, branch)
+ build_farm_jobs.append(build_farm_job)
+ builds.append(build)
+
+ self.assertContentEqual(
+ builds,
+ source.getByBuildFarmJobs(build_farm_jobs))
+
def test_getByBuildFarmJob_returns_none_if_not_found(self):
source = getUtility(ITranslationTemplatesBuildSource)
build_farm_job = self._makeBuildFarmJob()
Follow ups
