launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06042
[Merge] lp:~rvb/launchpad/builders-timeout-903827-2 into lp:launchpad
Raphaël Badin has proposed merging lp:~rvb/launchpad/builders-timeout-903827-2 into lp:launchpad with lp:~rvb/launchpad/builder-history-lfa as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~rvb/launchpad/builders-timeout-903827-2/+merge/87620
This branch makes buildqueue.specific_job and builder.currentjob cached properties. builder.currentjob is eager loaded in lib/lp/soyuz/browser/builder and buildqueue.specific_job (which should not change over time) is loaded in preloadSpecificJobData.
= Tests =
(modified)
test_builder test_builders_binary_package_build_query_count
test_builder test_builders_recipe_build_query_count
test_builder test_builders_translation_template_build_query_count
= Q/A =
The number of repeated statements of https://launchpad.net/builders should diminish a great deal. It's currently around in [300 - 600] depending on the type of builds building. Note that the repeated statement issued to fetch buildqueue records will still be there.
--
https://code.launchpad.net/~rvb/launchpad/builders-timeout-903827-2/+merge/87620
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/launchpad/builders-timeout-903827-2 into lp:launchpad.
=== modified file 'lib/lp/buildmaster/manager.py'
--- lib/lp/buildmaster/manager.py 2012-01-01 02:58:52 +0000
+++ lib/lp/buildmaster/manager.py 2012-01-05 14:47:35 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Soyuz buildd slave manager logic."""
@@ -34,6 +34,7 @@
BuildBehaviorMismatch,
)
from lp.buildmaster.model.builder import Builder
+from lp.services.propertycache import get_property_cache
BUILDD_MANAGER_LOG_NAME = "slave-scanner"
@@ -49,7 +50,9 @@
def assessFailureCounts(builder, fail_notes):
"""View builder/job failure_count and work out which needs to die. """
# builder.currentjob hides a complicated query, don't run it twice.
- # See bug 623281.
+ # See bug 623281 (Note that currentjob is a cachedproperty).
+
+ del get_property_cache(builder).currentjob
current_job = builder.currentjob
if current_job is None:
job_failure_count = 0
@@ -95,6 +98,7 @@
# but that would cause us to query the slave for its status
# again, and if the slave is non-responsive it holds up the
# next buildd scan.
+ del get_property_cache(builder).currentjob
class SlaveScanner:
=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py 2012-01-02 11:21:14 +0000
+++ lib/lp/buildmaster/model/builder.py 2012-01-05 14:47:35 +0000
@@ -430,6 +430,7 @@
def _getCurrentBuildBehavior(self):
"""Return the current build behavior."""
+ self._clean_currentjob_cache()
if not safe_hasattr(self, '_current_build_behavior'):
self._current_build_behavior = None
@@ -497,11 +498,14 @@
# XXX 2010-08-24 Julian bug=623281
# This should not be a property! It's masking a complicated query.
- @property
+ @cachedproperty
def currentjob(self):
"""See IBuilder"""
return getUtility(IBuildQueueSet).getByBuilder(self)
+ def _clean_currentjob_cache(self):
+ del get_property_cache(self).currentjob
+
def requestAbort(self):
"""See IBuilder."""
return self.slave.abort()
=== modified file 'lib/lp/buildmaster/model/buildqueue.py'
--- lib/lp/buildmaster/model/buildqueue.py 2012-01-02 11:21:14 +0000
+++ lib/lp/buildmaster/model/buildqueue.py 2012-01-05 14:47:35 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=E0611,W0212
@@ -53,6 +53,10 @@
)
from lp.services.job.interfaces.job import JobStatus
from lp.services.job.model.job import Job
+from lp.services.propertycache import (
+ cachedproperty,
+ get_property_cache,
+ )
from lp.services.webapp.interfaces import (
DEFAULT_FLAVOR,
IStoreSelector,
@@ -138,12 +142,15 @@
"""See `IBuildQueue`."""
return IBuildFarmJobBehavior(self.specific_job)
- @property
+ @cachedproperty
def specific_job(self):
"""See `IBuildQueue`."""
specific_class = specific_job_classes()[self.job_type]
return specific_class.getByJob(self.job)
+ def _clear_specific_job_cache(self):
+ del get_property_cache(self).specific_job
+
@staticmethod
def preloadSpecificJobData(queues):
key = attrgetter('job_type')
@@ -158,6 +165,12 @@
if len(list(specific_jobs)) == 0:
continue
specific_class.preloadJobsData(specific_jobs)
+ specific_jobs_dict = dict(
+ (specific_job.job, specific_job)
+ for specific_job in specific_jobs)
+ for queue in queue_subset:
+ cache = get_property_cache(queue)
+ cache.specific_job = specific_jobs_dict[queue.job]
@property
def date_started(self):
@@ -180,6 +193,7 @@
SQLBase.destroySelf(self)
specific_job.cleanUp()
job.destroySelf()
+ self._clear_specific_job_cache()
def manualScore(self, value):
"""See `IBuildQueue`."""
=== modified file 'lib/lp/code/model/sourcepackagerecipebuild.py'
--- lib/lp/code/model/sourcepackagerecipebuild.py 2012-01-05 14:47:31 +0000
+++ lib/lp/code/model/sourcepackagerecipebuild.py 2012-01-05 14:47:35 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
# pylint: disable-msg=F0401,E1002
@@ -291,10 +291,12 @@
# Circular imports.
from lp.code.model.sourcepackagerecipe import SourcePackageRecipe
from lp.services.librarian.model import LibraryFileAlias
+ from lp.buildmaster.model.buildfarmjob import BuildFarmJob
package_builds = load_related(
PackageBuild, builds, ['package_build_id'])
- build_farm_jobs = [
- build.build_farm_job for build in builds]
+ build_farm_jobs = load_related(
+ BuildFarmJob, [build.package_build for build in builds],
+ ['build_farm_job_id'])
load_related(LibraryFileAlias, build_farm_jobs, ['log_id'])
archives = load_related(Archive, package_builds, ['archive_id'])
load_related(Person, archives, ['ownerID'])
=== modified file 'lib/lp/soyuz/browser/builder.py'
--- lib/lp/soyuz/browser/builder.py 2012-01-02 11:21:14 +0000
+++ lib/lp/soyuz/browser/builder.py 2012-01-05 14:47:35 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Browser views for builders."""
@@ -44,7 +44,10 @@
from lp.buildmaster.model.buildqueue import BuildQueue
from lp.services.database.decoratedresultset import DecoratedResultSet
from lp.services.database.lpstorm import IStore
-from lp.services.propertycache import cachedproperty
+from lp.services.propertycache import (
+ cachedproperty,
+ get_property_cache,
+ )
from lp.services.webapp import (
ApplicationMenu,
canonical_url,
@@ -148,11 +151,17 @@
def builders(self):
"""All active builders"""
def do_eager_load(builders):
- # Prefetch the jobs' data.
+ # Populate builders' currentjob cachedproperty.
queues = IStore(BuildQueue).find(
BuildQueue,
BuildQueue.builderID.is_in(
builder.id for builder in builders))
+ queue_builders = dict(
+ (queue.builderID, queue) for queue in queues)
+ for builder in builders:
+ cache = get_property_cache(builder)
+ cache.currentjob = queue_builders.get(builder.id, None)
+ # Prefetch the jobs' data.
BuildQueue.preloadSpecificJobData(queues)
return list(DecoratedResultSet(
=== modified file 'lib/lp/soyuz/browser/tests/test_builder.py'
--- lib/lp/soyuz/browser/tests/test_builder.py 2012-01-05 14:47:31 +0000
+++ lib/lp/soyuz/browser/tests/test_builder.py 2012-01-05 14:47:35 +0000
@@ -1,4 +1,4 @@
-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2012 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for the lp.soyuz.browser.builder module."""
@@ -67,14 +67,10 @@
layer = LaunchpadFunctionalLayer
- # XXX rvb: the 3 additional queries per build are the result of the calls
- # to:
- # - builder.currentjob
- # - buildqueue.specific_job
- # These could be converted into cachedproperty and pre-populated in
- # bulk but several tests assert that the value returned by these
- # these properties are up to date. Since they are not really expensive
- # to compute I'll leave them as regular properties for now.
+ # XXX rvb: the query issued per build is the result of the call to
+ # build.buildqueue_record. It was decided not to make it a cachedproperty
+ # because the code relies on the fact that this property always returns
+ # the current record.
def test_builders_binary_package_build_query_count(self):
def create_build():
@@ -82,12 +78,13 @@
queue = build.queueBuild()
queue.markAsBuilding(build.builder)
+ nb_objects = 2
recorder1, recorder2 = record_two_runs(
- builders_homepage_render, create_build, 2)
+ builders_homepage_render, create_build, nb_objects)
self.assertThat(
recorder2,
- HasQueryCount(LessThan(recorder1.count + 3 * 2 + 1)))
+ HasQueryCount(LessThan(recorder1.count + 1 * nb_objects + 1)))
def test_builders_recipe_build_query_count(self):
def create_build():
@@ -95,12 +92,13 @@
queue = build.queueBuild()
queue.markAsBuilding(build.builder)
+ nb_objects = 2
recorder1, recorder2 = record_two_runs(
- builders_homepage_render, create_build, 2)
+ builders_homepage_render, create_build, nb_objects)
self.assertThat(
recorder2,
- HasQueryCount(LessThan(recorder1.count + 4 * 2 + 1)))
+ HasQueryCount(LessThan(recorder1.count + 1 * nb_objects + 1)))
def test_builders_translation_template_build_query_count(self):
def create_build():
@@ -114,9 +112,10 @@
queue = queueset.get(job_id)
queue.markAsBuilding(self.factory.makeBuilder())
+ nb_objects = 2
recorder1, recorder2 = record_two_runs(
- builders_homepage_render, create_build, 2)
+ builders_homepage_render, create_build, nb_objects)
self.assertThat(
recorder2,
- HasQueryCount(LessThan(recorder1.count + 3 * 2 + 1)))
+ HasQueryCount(LessThan(recorder1.count + 1 * nb_objects + 1)))
