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
1=== modified file 'lib/lp/buildmaster/doc/builder.txt'
2--- lib/lp/buildmaster/doc/builder.txt 2010-04-12 05:52:01 +0000
3+++ lib/lp/buildmaster/doc/builder.txt 2010-06-05 06:54:27 +0000
4@@ -111,29 +111,22 @@
5 bob
6 frog
7
8-'getBuildQueueSizeForProcessor' returns the number of pending builds
9-for a given Processor. The callsites can also control which build-farm
10-they are interested via the 'virtualized' argument.
11-
12- >>> from lp.soyuz.model.processor import Processor
13- >>> p386 = Processor.selectOneBy(name='386')
14- >>> amd64 = Processor.selectOneBy(name='amd64')
15-
16-'virtualized' defaults to False, so if not passed, it will return the
17-size and estimated duration of the non-virtualized (trusted) build queue.
18-
19- >>> builderset.getBuildQueueSizeForProcessor(p386)
20- (1, datetime.timedelta(0, 60))
21-
22-The 'amd64' build queue is empty.
23-
24- >>> builderset.getBuildQueueSizeForProcessor(amd64)
25- (0, None)
26+'getBuildQueueSizes' returns the number of pending builds for each
27+Processor/virtualization.
28+
29+ >>> queue_sizes = builderset.getBuildQueueSizes()
30+ >>> queue_sizes['nonvirt']['386']
31+ (1L, datetime.timedelta(0, 60))
32+
33+There are no 'amd64' build queue entries.
34+
35+ >>> queue_sizes['nonvirt'].keys()
36+ [u'386']
37
38 The virtualized build queue for 386 is also empty.
39
40- >>> builderset.getBuildQueueSizeForProcessor(p386, virtualized=True)
41- (0, None)
42+ >>> queue_sizes['virt'].keys()
43+ []
44
45 The queue size is not affect by builds target to disabled
46 archives. Builds for disabled archive are not dispatched as well, this
47@@ -145,23 +138,28 @@
48 >>> from lp.registry.interfaces.distribution import IDistributionSet
49 >>> ubuntu = getUtility(IDistributionSet).getByName('ubuntu')
50 >>> ubuntu.main_archive.disable()
51+ >>> import transaction
52+ >>> transaction.commit()
53 >>> login(ANONYMOUS)
54
55 That done, the non-virtualized queue for i386 becomes empty.
56
57- >>> builderset.getBuildQueueSizeForProcessor(p386)
58- (0, None)
59+ >>> queue_sizes = builderset.getBuildQueueSizes()
60+ >>> queue_sizes['nonvirt'].keys()
61+ []
62
63 Let's re-enable the ubuntu primary archive.
64
65 >>> login('foo.bar@canonical.com')
66 >>> ubuntu.main_archive.enable()
67+ >>> transaction.commit()
68 >>> login(ANONYMOUS)
69
70 The build for the ubuntu primary archive shows up again.
71
72- >>> builderset.getBuildQueueSizeForProcessor(p386)
73- (1, datetime.timedelta(0, 60))
74+ >>> queue_sizes = builderset.getBuildQueueSizes()
75+ >>> queue_sizes['nonvirt']['386']
76+ (1L, datetime.timedelta(0, 60))
77
78
79 Resuming buildd slaves
80@@ -306,6 +304,6 @@
81 instead simply cleaned, ready for the next build.
82
83 >>> rescue_slave_if_lost(WaitingSlave(build_id='bad'))
84- Cleaning slave
85+ Cleaning slave
86 WARNING:root:Builder 'mock' rescued from 'bad': 'Bad value'
87
88
89=== modified file 'lib/lp/buildmaster/interfaces/builder.py'
90--- lib/lp/buildmaster/interfaces/builder.py 2010-04-12 05:52:01 +0000
91+++ lib/lp/buildmaster/interfaces/builder.py 2010-06-05 06:54:27 +0000
92@@ -327,14 +327,20 @@
93 def getBuildersByArch(arch):
94 """Return all configured builders for a given DistroArchSeries."""
95
96- def getBuildQueueSizeForProcessor(processor, virtualized=False):
97- """Return the number of pending builds for a given processor.
98-
99- :param processor: IProcessor;
100- :param virtualized: boolean, controls which queue to check,
101- 'virtualized' means PPA.
102-
103- :return: a tuple containing the size of the queue, as an integer,
104+ def getBuildQueueSizes():
105+ """Return the number of pending builds for each processor.
106+
107+ :return: a dict of tuples with the queue size and duration for
108+ each processor and virtualisation. For example:
109+ {
110+ 'virt': {
111+ '386': (1, datetime.timedelta(0, 60)),
112+ 'amd64': (2, datetime.timedelta(0, 30)),
113+ },
114+ 'nonvirt':...
115+ }
116+
117+ The tuple contains the size of the queue, as an integer,
118 and the sum of the jobs 'estimated_duration' in queue,
119 as a timedelta or None for empty queues.
120 """
121
122=== modified file 'lib/lp/buildmaster/model/builder.py'
123--- lib/lp/buildmaster/model/builder.py 2010-05-18 08:54:36 +0000
124+++ lib/lp/buildmaster/model/builder.py 2010-06-05 06:54:27 +0000
125@@ -24,6 +24,7 @@
126
127 from sqlobject import (
128 BoolCol, ForeignKey, IntCol, SQLObjectNotFound, StringCol)
129+from storm.expr import Count, Sum
130 from storm.store import Store
131 from zope.component import getUtility
132 from zope.interface import implements
133@@ -36,7 +37,7 @@
134 from canonical.launchpad.webapp import urlappend
135 from canonical.launchpad.webapp.interfaces import NotFoundError
136 from canonical.launchpad.webapp.interfaces import (
137- IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
138+ IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR, SLAVE_FLAVOR)
139 from canonical.lazr.utils import safe_hasattr
140 from canonical.librarian.utils import copy_and_close
141 from lp.buildmaster.interfaces.buildbase import BuildStatus
142@@ -693,7 +694,7 @@
143 % arch.processorfamily.id,
144 clauseTables=("Processor",))
145
146- def getBuildQueueSizeForProcessor(self, processor, virtualized=False):
147+ def getBuildQueueSizes(self):
148 """See `IBuilderSet`."""
149 # Avoiding circular imports.
150 from lp.soyuz.model.archive import Archive
151@@ -702,33 +703,33 @@
152 DistroArchSeries)
153 from lp.soyuz.model.processor import Processor
154
155- store = Store.of(processor)
156- origin = (
157- Archive,
158- BinaryPackageBuild,
159- PackageBuild,
160- BuildFarmJob,
161- BuildPackageJob,
162- BuildQueue,
163- DistroArchSeries,
164+ find_spec = (
165+ Count(),
166+ Sum(BuildQueue.estimated_duration),
167 Processor,
168+ Archive.require_virtualized,
169 )
170- queue = store.using(*origin).find(
171- BuildQueue,
172+ store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR)
173+ results = store.find(
174+ find_spec,
175 BuildPackageJob.job == BuildQueue.jobID,
176 BuildPackageJob.build == BinaryPackageBuild.id,
177- BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
178 BinaryPackageBuild.package_build == PackageBuild.id,
179+ PackageBuild.build_farm_job == BuildFarmJob.id,
180 PackageBuild.archive == Archive.id,
181- PackageBuild.build_farm_job == BuildFarmJob.id,
182+ BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
183 DistroArchSeries.processorfamilyID == Processor.familyID,
184 BuildFarmJob.status == BuildStatus.NEEDSBUILD,
185- Archive._enabled == True,
186- Processor.id == processor.id,
187- Archive.require_virtualized == virtualized,
188- )
189-
190- return (queue.count(), queue.sum(BuildQueue.estimated_duration))
191+ Archive._enabled == True).group_by(
192+ Processor, Archive.require_virtualized)
193+
194+ result_dict = {'virt': {}, 'nonvirt': {}}
195+ for size, duration, processor, virtualized in results:
196+ virt_str = 'virt' if virtualized else 'nonvirt'
197+ result_dict[virt_str][processor.name] = (
198+ size, duration)
199+
200+ return result_dict
201
202 def pollBuilders(self, logger, txn):
203 """See IBuilderSet."""
204
205=== modified file 'lib/lp/soyuz/browser/builder.py'
206--- lib/lp/soyuz/browser/builder.py 2010-05-26 09:12:34 +0000
207+++ lib/lp/soyuz/browser/builder.py 2010-06-05 06:54:27 +0000
208@@ -149,12 +149,18 @@
209 def number_of_building_builders(self):
210 return len([b for b in self.builders if b.currentjob is not None])
211
212+ @cachedproperty
213+ def build_queue_sizes(self):
214+ """Return the build queue sizes for all processors."""
215+ builderset = getUtility(IBuilderSet)
216+ return builderset.getBuildQueueSizes()
217+
218 @property
219 def ppa_builders(self):
220 """Return a BuilderCategory object for PPA builders."""
221 builder_category = BuilderCategory(
222 'PPA build status', virtualized=True)
223- builder_category.groupBuilders(self.builders)
224+ builder_category.groupBuilders(self.builders, self.build_queue_sizes)
225 return builder_category
226
227 @property
228@@ -162,7 +168,7 @@
229 """Return a BuilderCategory object for PPA builders."""
230 builder_category = BuilderCategory(
231 'Official distributions build status', virtualized=False)
232- builder_category.groupBuilders(self.builders)
233+ builder_category.groupBuilders(self.builders, self.build_queue_sizes)
234 return builder_category
235
236
237@@ -199,7 +205,7 @@
238 return sorted(self._builder_groups,
239 key=operator.attrgetter('processor_name'))
240
241- def groupBuilders(self, all_builders):
242+ def groupBuilders(self, all_builders, build_queue_sizes):
243 """Group the given builders as a collection of Buildergroups.
244
245 A BuilderGroup will be initialized for each processor.
246@@ -214,10 +220,10 @@
247 else:
248 grouped_builders[builder.processor] = [builder]
249
250- builderset = getUtility(IBuilderSet)
251 for processor, builders in grouped_builders.iteritems():
252- queue_size, duration = builderset.getBuildQueueSizeForProcessor(
253- processor, virtualized=self.virtualized)
254+ virt_str = 'virt' if self.virtualized else 'nonvirt'
255+ queue_size, duration = build_queue_sizes[virt_str].get(
256+ processor.name, (0, None))
257 builder_group = BuilderGroup(
258 processor.name, queue_size, duration,
259 sorted(builders, key=operator.attrgetter('title')))
260
261=== modified file 'lib/lp/soyuz/browser/tests/builder-views.txt'
262--- lib/lp/soyuz/browser/tests/builder-views.txt 2010-04-09 15:46:09 +0000
263+++ lib/lp/soyuz/browser/tests/builder-views.txt 2010-06-05 06:54:27 +0000
264@@ -390,10 +390,13 @@
265 >>> any_failed_build.retry()
266 >>> removeSecurityProxy(
267 ... any_failed_build.buildqueue_record).estimated_duration = one_minute
268+ >>> transaction.commit()
269 >>> login(ANONYMOUS)
270
271 Now the pending build is included in the right category and group.
272
273+ >>> builderset_view = getMultiAdapter(
274+ ... (builderset, empty_request), name="+index")
275 >>> builder_category = builderset_view.ppa_builders
276 >>> print_category(builder_category)
277 386 2 2 0:01:00