Launchpad itself

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

  1. 588684-builders-timeout
  2. Merge into devel
Proposed by Michael Nelson
Status: Merged
Merged at revision: 10952
Proposed branch: lp:~michael.nelson/launchpad/588684-builders-timeout
Merge into: lp:launchpad
Diff against target: 277 lines (+74/-60)
5 files modified
lib/lp/buildmaster/doc/builder.txt (+23/-25)
lib/lp/buildmaster/interfaces/builder.py (+14/-8)
lib/lp/buildmaster/model/builder.py (+22/-21)
lib/lp/soyuz/browser/builder.py (+12/-6)
lib/lp/soyuz/browser/tests/builder-views.txt (+3/-0)
To merge this branch: bzr merge lp:~michael.nelson/launchpad/588684-builders-timeout
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+26818@code.launchpad.net

Description of the change

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.

To post a comment you must log in.
Revision history for this message
Abel Deuring (adeuring) wrote :

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)
Revision history for this message
Michael Nelson (michael.nelson) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
111 bob111 bob
112 frog112 frog
113113
114'getBuildQueueSizeForProcessor' returns the number of pending builds114'getBuildQueueSizes' returns the number of pending builds for each
115for a given Processor. The callsites can also control which build-farm115Processor/virtualization.
116they are interested via the 'virtualized' argument.116
117117 >>> queue_sizes = builderset.getBuildQueueSizes()
118 >>> from lp.soyuz.model.processor import Processor118 >>> queue_sizes['nonvirt']['386']
119 >>> p386 = Processor.selectOneBy(name='386')119 (1L, datetime.timedelta(0, 60))
120 >>> amd64 = Processor.selectOneBy(name='amd64')120
121121There are no 'amd64' build queue entries.
122'virtualized' defaults to False, so if not passed, it will return the122
123size and estimated duration of the non-virtualized (trusted) build queue.123 >>> queue_sizes['nonvirt'].keys()
124124 [u'386']
125 >>> builderset.getBuildQueueSizeForProcessor(p386)
126 (1, datetime.timedelta(0, 60))
127
128The 'amd64' build queue is empty.
129
130 >>> builderset.getBuildQueueSizeForProcessor(amd64)
131 (0, None)
132125
133The virtualized build queue for 386 is also empty.126The virtualized build queue for 386 is also empty.
134127
135 >>> builderset.getBuildQueueSizeForProcessor(p386, virtualized=True)128 >>> queue_sizes['virt'].keys()
136 (0, None)129 []
137130
138The queue size is not affect by builds target to disabled131The queue size is not affect by builds target to disabled
139archives. Builds for disabled archive are not dispatched as well, this132archives. Builds for disabled archive are not dispatched as well, this
@@ -145,23 +138,28 @@
145 >>> from lp.registry.interfaces.distribution import IDistributionSet138 >>> from lp.registry.interfaces.distribution import IDistributionSet
146 >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')139 >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
147 >>> ubuntu.main_archive.disable()140 >>> ubuntu.main_archive.disable()
141 >>> import transaction
142 >>> transaction.commit()
148 >>> login(ANONYMOUS)143 >>> login(ANONYMOUS)
149144
150That done, the non-virtualized queue for i386 becomes empty.145That done, the non-virtualized queue for i386 becomes empty.
151146
152 >>> builderset.getBuildQueueSizeForProcessor(p386)147 >>> queue_sizes = builderset.getBuildQueueSizes()
153 (0, None)148 >>> queue_sizes['nonvirt'].keys()
149 []
154150
155Let's re-enable the ubuntu primary archive.151Let's re-enable the ubuntu primary archive.
156152
157 >>> login('foo.bar@canonical.com')153 >>> login('foo.bar@canonical.com')
158 >>> ubuntu.main_archive.enable()154 >>> ubuntu.main_archive.enable()
155 >>> transaction.commit()
159 >>> login(ANONYMOUS)156 >>> login(ANONYMOUS)
160157
161The build for the ubuntu primary archive shows up again.158The build for the ubuntu primary archive shows up again.
162159
163 >>> builderset.getBuildQueueSizeForProcessor(p386)160 >>> queue_sizes = builderset.getBuildQueueSizes()
164 (1, datetime.timedelta(0, 60))161 >>> queue_sizes['nonvirt']['386']
162 (1L, datetime.timedelta(0, 60))
165163
166164
167Resuming buildd slaves165Resuming buildd slaves
@@ -306,6 +304,6 @@
306instead simply cleaned, ready for the next build.304instead simply cleaned, ready for the next build.
307305
308 >>> rescue_slave_if_lost(WaitingSlave(build_id='bad'))306 >>> rescue_slave_if_lost(WaitingSlave(build_id='bad'))
309 Cleaning slave 307 Cleaning slave
310 WARNING:root:Builder 'mock' rescued from 'bad': 'Bad value'308 WARNING:root:Builder 'mock' rescued from 'bad': 'Bad value'
311309
312310
=== 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 @@
327 def getBuildersByArch(arch):327 def getBuildersByArch(arch):
328 """Return all configured builders for a given DistroArchSeries."""328 """Return all configured builders for a given DistroArchSeries."""
329329
330 def getBuildQueueSizeForProcessor(processor, virtualized=False):330 def getBuildQueueSizes():
331 """Return the number of pending builds for a given processor.331 """Return the number of pending builds for each processor.
332332
333 :param processor: IProcessor;333 :return: a dict of tuples with the queue size and duration for
334 :param virtualized: boolean, controls which queue to check,334 each processor and virtualisation. For example:
335 'virtualized' means PPA.335 {
336336 'virt': {
337 :return: a tuple containing the size of the queue, as an integer,337 '386': (1, datetime.timedelta(0, 60)),
338 'amd64': (2, datetime.timedelta(0, 30)),
339 },
340 'nonvirt':...
341 }
342
343 The tuple contains the size of the queue, as an integer,
338 and the sum of the jobs 'estimated_duration' in queue,344 and the sum of the jobs 'estimated_duration' in queue,
339 as a timedelta or None for empty queues.345 as a timedelta or None for empty queues.
340 """346 """
341347
=== 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 @@
2424
25from sqlobject import (25from sqlobject import (
26 BoolCol, ForeignKey, IntCol, SQLObjectNotFound, StringCol)26 BoolCol, ForeignKey, IntCol, SQLObjectNotFound, StringCol)
27from storm.expr import Count, Sum
27from storm.store import Store28from storm.store import Store
28from zope.component import getUtility29from zope.component import getUtility
29from zope.interface import implements30from zope.interface import implements
@@ -36,7 +37,7 @@
36from canonical.launchpad.webapp import urlappend37from canonical.launchpad.webapp import urlappend
37from canonical.launchpad.webapp.interfaces import NotFoundError38from canonical.launchpad.webapp.interfaces import NotFoundError
38from canonical.launchpad.webapp.interfaces import (39from canonical.launchpad.webapp.interfaces import (
39 IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)40 IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR, SLAVE_FLAVOR)
40from canonical.lazr.utils import safe_hasattr41from canonical.lazr.utils import safe_hasattr
41from canonical.librarian.utils import copy_and_close42from canonical.librarian.utils import copy_and_close
42from lp.buildmaster.interfaces.buildbase import BuildStatus43from lp.buildmaster.interfaces.buildbase import BuildStatus
@@ -693,7 +694,7 @@
693 % arch.processorfamily.id,694 % arch.processorfamily.id,
694 clauseTables=("Processor",))695 clauseTables=("Processor",))
695696
696 def getBuildQueueSizeForProcessor(self, processor, virtualized=False):697 def getBuildQueueSizes(self):
697 """See `IBuilderSet`."""698 """See `IBuilderSet`."""
698 # Avoiding circular imports.699 # Avoiding circular imports.
699 from lp.soyuz.model.archive import Archive700 from lp.soyuz.model.archive import Archive
@@ -702,33 +703,33 @@
702 DistroArchSeries)703 DistroArchSeries)
703 from lp.soyuz.model.processor import Processor704 from lp.soyuz.model.processor import Processor
704705
705 store = Store.of(processor)706 find_spec = (
706 origin = (707 Count(),
707 Archive,708 Sum(BuildQueue.estimated_duration),
708 BinaryPackageBuild,
709 PackageBuild,
710 BuildFarmJob,
711 BuildPackageJob,
712 BuildQueue,
713 DistroArchSeries,
714 Processor,709 Processor,
710 Archive.require_virtualized,
715 )711 )
716 queue = store.using(*origin).find(712 store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR)
717 BuildQueue,713 results = store.find(
714 find_spec,
718 BuildPackageJob.job == BuildQueue.jobID,715 BuildPackageJob.job == BuildQueue.jobID,
719 BuildPackageJob.build == BinaryPackageBuild.id,716 BuildPackageJob.build == BinaryPackageBuild.id,
720 BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
721 BinaryPackageBuild.package_build == PackageBuild.id,717 BinaryPackageBuild.package_build == PackageBuild.id,
718 PackageBuild.build_farm_job == BuildFarmJob.id,
722 PackageBuild.archive == Archive.id,719 PackageBuild.archive == Archive.id,
723 PackageBuild.build_farm_job == BuildFarmJob.id,720 BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
724 DistroArchSeries.processorfamilyID == Processor.familyID,721 DistroArchSeries.processorfamilyID == Processor.familyID,
725 BuildFarmJob.status == BuildStatus.NEEDSBUILD,722 BuildFarmJob.status == BuildStatus.NEEDSBUILD,
726 Archive._enabled == True,723 Archive._enabled == True).group_by(
727 Processor.id == processor.id,724 Processor, Archive.require_virtualized)
728 Archive.require_virtualized == virtualized,725
729 )726 result_dict = {'virt': {}, 'nonvirt': {}}
730727 for size, duration, processor, virtualized in results:
731 return (queue.count(), queue.sum(BuildQueue.estimated_duration))728 virt_str = 'virt' if virtualized else 'nonvirt'
729 result_dict[virt_str][processor.name] = (
730 size, duration)
731
732 return result_dict
732733
733 def pollBuilders(self, logger, txn):734 def pollBuilders(self, logger, txn):
734 """See IBuilderSet."""735 """See IBuilderSet."""
735736
=== 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 @@
149 def number_of_building_builders(self):149 def number_of_building_builders(self):
150 return len([b for b in self.builders if b.currentjob is not None])150 return len([b for b in self.builders if b.currentjob is not None])
151151
152 @cachedproperty
153 def build_queue_sizes(self):
154 """Return the build queue sizes for all processors."""
155 builderset = getUtility(IBuilderSet)
156 return builderset.getBuildQueueSizes()
157
152 @property158 @property
153 def ppa_builders(self):159 def ppa_builders(self):
154 """Return a BuilderCategory object for PPA builders."""160 """Return a BuilderCategory object for PPA builders."""
155 builder_category = BuilderCategory(161 builder_category = BuilderCategory(
156 'PPA build status', virtualized=True)162 'PPA build status', virtualized=True)
157 builder_category.groupBuilders(self.builders)163 builder_category.groupBuilders(self.builders, self.build_queue_sizes)
158 return builder_category164 return builder_category
159165
160 @property166 @property
@@ -162,7 +168,7 @@
162 """Return a BuilderCategory object for PPA builders."""168 """Return a BuilderCategory object for PPA builders."""
163 builder_category = BuilderCategory(169 builder_category = BuilderCategory(
164 'Official distributions build status', virtualized=False)170 'Official distributions build status', virtualized=False)
165 builder_category.groupBuilders(self.builders)171 builder_category.groupBuilders(self.builders, self.build_queue_sizes)
166 return builder_category172 return builder_category
167173
168174
@@ -199,7 +205,7 @@
199 return sorted(self._builder_groups,205 return sorted(self._builder_groups,
200 key=operator.attrgetter('processor_name'))206 key=operator.attrgetter('processor_name'))
201207
202 def groupBuilders(self, all_builders):208 def groupBuilders(self, all_builders, build_queue_sizes):
203 """Group the given builders as a collection of Buildergroups.209 """Group the given builders as a collection of Buildergroups.
204210
205 A BuilderGroup will be initialized for each processor.211 A BuilderGroup will be initialized for each processor.
@@ -214,10 +220,10 @@
214 else:220 else:
215 grouped_builders[builder.processor] = [builder]221 grouped_builders[builder.processor] = [builder]
216222
217 builderset = getUtility(IBuilderSet)
218 for processor, builders in grouped_builders.iteritems():223 for processor, builders in grouped_builders.iteritems():
219 queue_size, duration = builderset.getBuildQueueSizeForProcessor(224 virt_str = 'virt' if self.virtualized else 'nonvirt'
220 processor, virtualized=self.virtualized)225 queue_size, duration = build_queue_sizes[virt_str].get(
226 processor.name, (0, None))
221 builder_group = BuilderGroup(227 builder_group = BuilderGroup(
222 processor.name, queue_size, duration,228 processor.name, queue_size, duration,
223 sorted(builders, key=operator.attrgetter('title')))229 sorted(builders, key=operator.attrgetter('title')))
224230
=== 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 @@
390 >>> any_failed_build.retry()390 >>> any_failed_build.retry()
391 >>> removeSecurityProxy(391 >>> removeSecurityProxy(
392 ... any_failed_build.buildqueue_record).estimated_duration = one_minute392 ... any_failed_build.buildqueue_record).estimated_duration = one_minute
393 >>> transaction.commit()
393 >>> login(ANONYMOUS)394 >>> login(ANONYMOUS)
394395
395Now the pending build is included in the right category and group.396Now the pending build is included in the right category and group.
396397
398 >>> builderset_view = getMultiAdapter(
399 ... (builderset, empty_request), name="+index")
397 >>> builder_category = builderset_view.ppa_builders400 >>> builder_category = builderset_view.ppa_builders
398 >>> print_category(builder_category)401 >>> print_category(builder_category)
399 386 2 2 0:01:00402 386 2 2 0:01:00