Merge lp:~michael.nelson/launchpad/588684-builders-timeout into lp:launchpad

[Michael Nelson]

This branch replaces many heavy queries (one per builder group... eg. 386 virtual) on the /builders page with a single query that gets the stats for all builder groups.

See bug 588684 for the analysis from stub.

To test:
bin/test -vv -t doc/builder.txt -t builder-views.txt

It will need to be cherry-picked to replace the cowboy that currently disables the build queue size information.

[Abel Deuring (adeuring)]

Nice work! Just one nitpick:

=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py 2010-05-18 08:54:36 +0000
+++ lib/lp/buildmaster/model/builder.py 2010-06-04 15:28:29 +0000

[...]

@@ -702,33 +703,34 @@
             DistroArchSeries)
         from lp.soyuz.model.processor import Processor

- store = Store.of(processor)
- origin = (
- Archive,
- BinaryPackageBuild,
- PackageBuild,
- BuildFarmJob,
- BuildPackageJob,
- BuildQueue,
- DistroArchSeries,
+ find_spec = (
+ Count(),
+ Sum(BuildQueue.estimated_duration),
             Processor,
+ Archive.require_virtualized,
             )
- queue = store.using(*origin).find(
- BuildQueue,
+ store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR)
+ results = store.find(
+ find_spec,
             BuildPackageJob.job == BuildQueue.jobID,
             BuildPackageJob.build == BinaryPackageBuild.id,
- BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
             BinaryPackageBuild.package_build == PackageBuild.id,
+ PackageBuild.build_farm_job == BuildFarmJob.id,
             PackageBuild.archive == Archive.id,
- PackageBuild.build_farm_job == BuildFarmJob.id,
+ BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
             DistroArchSeries.processorfamilyID == Processor.familyID,
+ # WHERE

This "WHERE" looks a bit odd ;) I think you can remove it.

review: Approve (code)

[Michael Nelson (michael.nelson)]

On Fri, Jun 4, 2010 at 6:22 PM, Abel Deuring <email address hidden> wrote:
> Review: Approve code
> Nice work! Just one nitpick:
>
> === modified file 'lib/lp/buildmaster/model/builder.py'
> --- lib/lp/buildmaster/model/builder.py 2010-05-18 08:54:36 +0000
> +++ lib/lp/buildmaster/model/builder.py 2010-06-04 15:28:29 +0000
>

> +            BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
>             DistroArchSeries.processorfamilyID == Processor.familyID,
> +            # WHERE
>
> This "WHERE" looks a bit odd ;) I think you can remove it.

Done...(woops, left over from the SQL->Storm conversion). Thanks Abel.

=== modified file 'lib/lp/buildmaster/doc/builder.txt'
--- lib/lp/buildmaster/doc/builder.txt	2010-04-12 05:52:01 +0000
+++ lib/lp/buildmaster/doc/builder.txt	2010-06-05 06:54:27 +0000
@@ -111,29 +111,22 @@
     bob
     frog
 
-'getBuildQueueSizeForProcessor' returns the number of pending builds
-for a given Processor. The callsites can also control which build-farm
-they are interested via the 'virtualized' argument.
-
-    >>> from lp.soyuz.model.processor import Processor
-    >>> p386 = Processor.selectOneBy(name='386')
-    >>> amd64 = Processor.selectOneBy(name='amd64')
-
-'virtualized' defaults to False, so if not passed, it will return the
-size and estimated duration of the non-virtualized (trusted) build queue.
-
-    >>> builderset.getBuildQueueSizeForProcessor(p386)
-    (1, datetime.timedelta(0, 60))
-
-The 'amd64' build queue is empty.
-
-    >>> builderset.getBuildQueueSizeForProcessor(amd64)
-    (0, None)
+'getBuildQueueSizes' returns the number of pending builds for each
+Processor/virtualization.
+
+    >>> queue_sizes = builderset.getBuildQueueSizes()
+    >>> queue_sizes['nonvirt']['386']
+    (1L, datetime.timedelta(0, 60))
+
+There are no 'amd64' build queue entries.
+
+    >>> queue_sizes['nonvirt'].keys()
+    [u'386']
 
 The virtualized build queue for 386 is also empty.
 
-    >>> builderset.getBuildQueueSizeForProcessor(p386, virtualized=True)
-    (0, None)
+    >>> queue_sizes['virt'].keys()
+    []
 
 The queue size is not affect by builds target to disabled
 archives. Builds for disabled archive are not dispatched as well, this
@@ -145,23 +138,28 @@
     >>> from lp.registry.interfaces.distribution import IDistributionSet
     >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
     >>> ubuntu.main_archive.disable()
+    >>> import transaction
+    >>> transaction.commit()
     >>> login(ANONYMOUS)
 
 That done, the non-virtualized queue for i386 becomes empty.
 
-    >>> builderset.getBuildQueueSizeForProcessor(p386)
-    (0, None)
+    >>> queue_sizes = builderset.getBuildQueueSizes()
+    >>> queue_sizes['nonvirt'].keys()
+    []
 
 Let's re-enable the ubuntu primary archive.
 
     >>> login('[e-mail]')
     >>> ubuntu.main_archive.enable()
+    >>> transaction.commit()
     >>> login(ANONYMOUS)
 
 The build for the ubuntu primary archive shows up again.
 
-    >>> builderset.getBuildQueueSizeForProcessor(p386)
-    (1, datetime.timedelta(0, 60))
+    >>> queue_sizes = builderset.getBuildQueueSizes()
+    >>> queue_sizes['nonvirt']['386']
+    (1L, datetime.timedelta(0, 60))
 
 
 Resuming buildd slaves
@@ -306,6 +304,6 @@
 instead simply cleaned, ready for the next build.
 
     >>> rescue_slave_if_lost(WaitingSlave(build_id='bad'))
-    Cleaning slave 
+    Cleaning slave
     WARNING:root:Builder 'mock' rescued from 'bad': 'Bad value'
 

=== modified file 'lib/lp/buildmaster/interfaces/builder.py'
--- lib/lp/buildmaster/interfaces/builder.py	2010-04-12 05:52:01 +0000
+++ lib/lp/buildmaster/interfaces/builder.py	2010-06-05 06:54:27 +0000
@@ -327,14 +327,20 @@
     def getBuildersByArch(arch):
         """Return all configured builders for a given DistroArchSeries."""
 
-    def getBuildQueueSizeForProcessor(processor, virtualized=False):
-        """Return the number of pending builds for a given processor.
-
-        :param processor: IProcessor;
-        :param virtualized: boolean, controls which queue to check,
-            'virtualized' means PPA.
-
-        :return: a tuple containing the size of the queue, as an integer,
+    def getBuildQueueSizes():
+        """Return the number of pending builds for each processor.
+
+        :return: a dict of tuples with the queue size and duration for
+            each processor and virtualisation. For example:
+            {
+                'virt': {
+                            '386': (1, datetime.timedelta(0, 60)),
+                            'amd64': (2, datetime.timedelta(0, 30)),
+                        },
+                'nonvirt':...
+            }
+
+            The tuple contains the size of the queue, as an integer,
             and the sum of the jobs 'estimated_duration' in queue,
             as a timedelta or None for empty queues.
         """

=== modified file 'lib/lp/buildmaster/model/builder.py'
--- lib/lp/buildmaster/model/builder.py	2010-05-18 08:54:36 +0000
+++ lib/lp/buildmaster/model/builder.py	2010-06-05 06:54:27 +0000
@@ -24,6 +24,7 @@
 
 from sqlobject import (
     BoolCol, ForeignKey, IntCol, SQLObjectNotFound, StringCol)
+from storm.expr import Count, Sum
 from storm.store import Store
 from zope.component import getUtility
 from zope.interface import implements
@@ -36,7 +37,7 @@
 from canonical.launchpad.webapp import urlappend
 from canonical.launchpad.webapp.interfaces import NotFoundError
 from canonical.launchpad.webapp.interfaces import (
-    IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
+    IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR, SLAVE_FLAVOR)
 from canonical.lazr.utils import safe_hasattr
 from canonical.librarian.utils import copy_and_close
 from lp.buildmaster.interfaces.buildbase import BuildStatus
@@ -693,7 +694,7 @@
                               % arch.processorfamily.id,
                               clauseTables=("Processor",))
 
-    def getBuildQueueSizeForProcessor(self, processor, virtualized=False):
+    def getBuildQueueSizes(self):
         """See `IBuilderSet`."""
         # Avoiding circular imports.
         from lp.soyuz.model.archive import Archive
@@ -702,33 +703,33 @@
             DistroArchSeries)
         from lp.soyuz.model.processor import Processor
 
-        store = Store.of(processor)
-        origin = (
-            Archive,
-            BinaryPackageBuild,
-            PackageBuild,
-            BuildFarmJob,
-            BuildPackageJob,
-            BuildQueue,
-            DistroArchSeries,
+        find_spec = (
+            Count(),
+            Sum(BuildQueue.estimated_duration),
             Processor,
+            Archive.require_virtualized,
             )
-        queue = store.using(*origin).find(
-            BuildQueue,
+        store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR)
+        results = store.find(
+            find_spec,
             BuildPackageJob.job == BuildQueue.jobID,
             BuildPackageJob.build == BinaryPackageBuild.id,
-            BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
             BinaryPackageBuild.package_build == PackageBuild.id,
+            PackageBuild.build_farm_job == BuildFarmJob.id,
             PackageBuild.archive == Archive.id,
-            PackageBuild.build_farm_job == BuildFarmJob.id,
+            BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
             DistroArchSeries.processorfamilyID == Processor.familyID,
             BuildFarmJob.status == BuildStatus.NEEDSBUILD,
-            Archive._enabled == True,
-            Processor.id == processor.id,
-            Archive.require_virtualized == virtualized,
-            )
-
-        return (queue.count(), queue.sum(BuildQueue.estimated_duration))
+            Archive._enabled == True).group_by(
+                Processor, Archive.require_virtualized)
+
+        result_dict = {'virt': {}, 'nonvirt': {}}
+        for size, duration, processor, virtualized in results:
+            virt_str = 'virt' if virtualized else 'nonvirt'
+            result_dict[virt_str][processor.name] = (
+                size, duration)
+
+        return result_dict
 
     def pollBuilders(self, logger, txn):
         """See IBuilderSet."""

=== modified file 'lib/lp/soyuz/browser/builder.py'
--- lib/lp/soyuz/browser/builder.py	2010-05-26 09:12:34 +0000
+++ lib/lp/soyuz/browser/builder.py	2010-06-05 06:54:27 +0000
@@ -149,12 +149,18 @@
     def number_of_building_builders(self):
         return len([b for b in self.builders if b.currentjob is not None])
 
+    @cachedproperty
+    def build_queue_sizes(self):
+        """Return the build queue sizes for all processors."""
+        builderset = getUtility(IBuilderSet)
+        return builderset.getBuildQueueSizes()
+
     @property
     def ppa_builders(self):
         """Return a BuilderCategory object for PPA builders."""
         builder_category = BuilderCategory(
             'PPA build status', virtualized=True)
-        builder_category.groupBuilders(self.builders)
+        builder_category.groupBuilders(self.builders, self.build_queue_sizes)
         return builder_category
 
     @property
@@ -162,7 +168,7 @@
         """Return a BuilderCategory object for PPA builders."""
         builder_category = BuilderCategory(
             'Official distributions build status', virtualized=False)
-        builder_category.groupBuilders(self.builders)
+        builder_category.groupBuilders(self.builders, self.build_queue_sizes)
         return builder_category
 
 
@@ -199,7 +205,7 @@
         return sorted(self._builder_groups,
                       key=operator.attrgetter('processor_name'))
 
-    def groupBuilders(self, all_builders):
+    def groupBuilders(self, all_builders, build_queue_sizes):
         """Group the given builders as a collection of Buildergroups.
 
         A BuilderGroup will be initialized for each processor.
@@ -214,10 +220,10 @@
             else:
                 grouped_builders[builder.processor] = [builder]
 
-        builderset = getUtility(IBuilderSet)
         for processor, builders in grouped_builders.iteritems():
-            queue_size, duration = builderset.getBuildQueueSizeForProcessor(
-                processor, virtualized=self.virtualized)
+            virt_str = 'virt' if self.virtualized else 'nonvirt'
+            queue_size, duration = build_queue_sizes[virt_str].get(
+                processor.name, (0, None))
             builder_group = BuilderGroup(
                 processor.name, queue_size, duration,
                 sorted(builders, key=operator.attrgetter('title')))

=== modified file 'lib/lp/soyuz/browser/tests/builder-views.txt'
--- lib/lp/soyuz/browser/tests/builder-views.txt	2010-04-09 15:46:09 +0000
+++ lib/lp/soyuz/browser/tests/builder-views.txt	2010-06-05 06:54:27 +0000
@@ -390,10 +390,13 @@
     >>> any_failed_build.retry()
     >>> removeSecurityProxy(
     ...     any_failed_build.buildqueue_record).estimated_duration = one_minute
+    >>> transaction.commit()
     >>> login(ANONYMOUS)
 
 Now the pending build is included in the right category and group.
 
+    >>> builderset_view = getMultiAdapter(
+    ...     (builderset, empty_request), name="+index")
     >>> builder_category = builderset_view.ppa_builders
     >>> print_category(builder_category)
     386    2  2  0:01:00