Merge lp:~al-maisan/launchpad/flip-the-switch-516545 into lp:launchpad

Proposed by Muharem Hrnjadovic
Status: Merged
Approved by: Gavin Panella
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~al-maisan/launchpad/flip-the-switch-516545
Merge into: lp:launchpad
Diff against target: 325 lines (+19/-178)
5 files modified
lib/lp/soyuz/doc/build-estimated-dispatch-time.txt (+10/-8)
lib/lp/soyuz/doc/build.txt (+7/-4)
lib/lp/soyuz/interfaces/build.py (+0/-9)
lib/lp/soyuz/model/build.py (+1/-156)
lib/lp/soyuz/templates/build-index.pt (+1/-1)
To merge this branch: bzr merge lp:~al-maisan/launchpad/flip-the-switch-516545
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+18992@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

Hello there!

The generalization of the build farm (so it can run jobs other than binary
builds) made a generalization of the job dispatch time estimation logic
necessary as well.

The latter was implemented in the course of the last 6 weeks and the
corresponding branches have all landed already.

The branch at hand "flips the switch" and turns on the new generalized job
dispatch time estimation logic.

Tests to run:

    bin/test -vv -t build

No pertinent "make lint" errors or warnings.

Revision history for this message
Gavin Panella (allenap) wrote :

Hi Muharem,

All looks good. One tiny comment in below.

Gavin.

> === modified file 'lib/lp/soyuz/doc/build.txt'
> --- lib/lp/soyuz/doc/build.txt 2010-01-21 08:53:21 +0000
> +++ lib/lp/soyuz/doc/build.txt 2010-02-10 10:33:18 +0000
> @@ -8,6 +8,7 @@
> launchpad build daemon.
>
> # Create a 'mozilla-firefox' build in ubuntutest/breezy-autotest/i386.
> + >>> from zope.security.proxy import removeSecurityProxy
> >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
> >>> login('<email address hidden>')
> >>> test_publisher = SoyuzTestPublisher()
> @@ -17,6 +18,9 @@
> >>> binaries = test_publisher.getPubBinaries(
> ... binaryname='firefox', pub_source=source)
> >>> [firefox_build] = source.getBuilds()
> + >>> the_job = removeSecurityProxy(firefox_build.buildqueue_record.job)
> + >>> the_job.start()
> + >>> the_job.complete()

I think it's bad to keep a reference to an unwrapped object that is
normally subject to access control. It's too easy to inadvertently
reuse this reference later in the test. Would you be happy to do
something like:

    job = firefox_build.buildqueue_record.job
    removeSecurityProxy(job).start()
    removeSecurityProxy(job).complete()

instead?

review: Approve
Revision history for this message
Muharem Hrnjadovic (al-maisan) wrote :

On 02/10/2010 11:51 AM, Gavin Panella wrote:
> Review: Approve
> Hi Muharem,
>
> All looks good. One tiny comment in below.
Hello Gavin,

your point below is well made. Thanks!

>> === modified file 'lib/lp/soyuz/doc/build.txt'
>> --- lib/lp/soyuz/doc/build.txt 2010-01-21 08:53:21 +0000
>> +++ lib/lp/soyuz/doc/build.txt 2010-02-10 10:33:18 +0000
>> @@ -8,6 +8,7 @@
>> launchpad build daemon.
>>
>> # Create a 'mozilla-firefox' build in ubuntutest/breezy-autotest/i386.
>> + >>> from zope.security.proxy import removeSecurityProxy
>> >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
>> >>> login('<email address hidden>')
>> >>> test_publisher = SoyuzTestPublisher()
>> @@ -17,6 +18,9 @@
>> >>> binaries = test_publisher.getPubBinaries(
>> ... binaryname='firefox', pub_source=source)
>> >>> [firefox_build] = source.getBuilds()
>> + >>> the_job = removeSecurityProxy(firefox_build.buildqueue_record.job)
>> + >>> the_job.start()
>> + >>> the_job.complete()
>
> I think it's bad to keep a reference to an unwrapped object that is
> normally subject to access control. It's too easy to inadvertently
> reuse this reference later in the test. Would you be happy to do
> something like:
>
> job = firefox_build.buildqueue_record.job
> removeSecurityProxy(job).start()
> removeSecurityProxy(job).complete()
>
> instead?
That's much better indeed. Done. Thanks again!

Best regards

--
Muharem Hrnjadovic <email address hidden>
Public key id : B2BBFCFC
Key fingerprint : A5A3 CC67 2B87 D641 103F 5602 219F 6B60 B2BB FCFC

1=== modified file 'lib/lp/soyuz/doc/build.txt'
2--- lib/lp/soyuz/doc/build.txt 2010-02-10 10:11:46 +0000
3+++ lib/lp/soyuz/doc/build.txt 2010-02-10 11:20:18 +0000
4@@ -18,9 +18,9 @@
5 >>> binaries = test_publisher.getPubBinaries(
6 ... binaryname='firefox', pub_source=source)
7 >>> [firefox_build] = source.getBuilds()
8- >>> the_job = removeSecurityProxy(firefox_build.buildqueue_record.job)
9- >>> the_job.start()
10- >>> the_job.complete()
11+ >>> job = firefox_build.buildqueue_record.job
12+ >>> removeSecurityProxy(job).start()
13+ >>> removeSecurityProxy(job).complete()
14 >>> login(ANONYMOUS)
15
16 A build has a title which describes the context source version and in

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/soyuz/doc/build-estimated-dispatch-time.txt'
2--- lib/lp/soyuz/doc/build-estimated-dispatch-time.txt 2010-01-20 01:23:38 +0000
3+++ lib/lp/soyuz/doc/build-estimated-dispatch-time.txt 2010-02-10 11:33:14 +0000
4@@ -91,17 +91,19 @@
5 in the future.
6
7 >>> now = datetime.utcnow()
8- >>> estimate = alsa_build.getEstimatedBuildStartTime()
9+ >>> def job_start_estimate(build):
10+ ... return build.buildqueue_record.getEstimatedJobStartTime()
11+ >>> estimate = job_start_estimate(alsa_build)
12 >>> estimate > now
13 True
14
15 The estimated build start time may only be requested for jobs that are
16 pending.
17
18- >>> cur_build.getEstimatedBuildStartTime()
19+ >>> job_start_estimate(cur_build)
20 Traceback (most recent call last):
21 ...
22- AssertionError: The start time is only estimated for pending builds.
23+ AssertionError: The start time is only estimated for pending jobs.
24
25 Now let's add two PPA packages to the mix in order to show how builds
26 associated with disabled archives get ignored when it comes to the calculation
27@@ -147,17 +149,17 @@
28 build (66) its estimated dispatch time is essentially "now".
29
30 >>> now = datetime.utcnow()
31- >>> estimate = iceweasel_build.getEstimatedBuildStartTime()
32+ >>> estimate = job_start_estimate(iceweasel_build)
33 >>> estimate > now
34 True
35 >>> estimate - now
36- datetime.timedelta(0, 0, ...)
37+ datetime.timedelta(0, 5, ...)
38
39 The 'pmount' build comes next in the queue and its estimated dispatch
40 time is the estimated build time of the 'iceweasel' package i.e. 2880
41 seconds (48 minutes * 60).
42
43- >>> estimate = pmount_build.getEstimatedBuildStartTime()
44+ >>> estimate = job_start_estimate(pmount_build)
45 >>> estimate > now
46 True
47 >>> estimate - now
48@@ -170,8 +172,8 @@
49
50 >>> mark.archive.disable()
51 >>> syncUpdate(mark.archive)
52- >>> estimate = pmount_build.getEstimatedBuildStartTime()
53+ >>> estimate = job_start_estimate(pmount_build)
54 >>> estimate > now
55 True
56 >>> estimate - now
57- datetime.timedelta(0, 0, ...)
58+ datetime.timedelta(0, 5, ...)
59
60=== modified file 'lib/lp/soyuz/doc/build.txt'
61--- lib/lp/soyuz/doc/build.txt 2010-01-21 08:53:21 +0000
62+++ lib/lp/soyuz/doc/build.txt 2010-02-10 11:33:14 +0000
63@@ -8,6 +8,7 @@
64 launchpad build daemon.
65
66 # Create a 'mozilla-firefox' build in ubuntutest/breezy-autotest/i386.
67+ >>> from zope.security.proxy import removeSecurityProxy
68 >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
69 >>> login('foo.bar@canonical.com')
70 >>> test_publisher = SoyuzTestPublisher()
71@@ -17,6 +18,9 @@
72 >>> binaries = test_publisher.getPubBinaries(
73 ... binaryname='firefox', pub_source=source)
74 >>> [firefox_build] = source.getBuilds()
75+ >>> job = firefox_build.buildqueue_record.job
76+ >>> removeSecurityProxy(job).start()
77+ >>> removeSecurityProxy(job).complete()
78 >>> login(ANONYMOUS)
79
80 A build has a title which describes the context source version and in
81@@ -89,10 +93,10 @@
82 The 'firefox_build' is already finished and requesting the estimated build
83 start time makes no sense. Hence an exception is raised.
84
85- >>> firefox_build.getEstimatedBuildStartTime()
86+ >>> firefox_build.buildqueue_record.getEstimatedJobStartTime()
87 Traceback (most recent call last):
88 ...
89- AssertionError: The start time is only estimated for pending builds.
90+ AssertionError: The start time is only estimated for pending jobs.
91
92 On a build job in state `NEEDSBUILD` we can ask for its estimated
93 build start time.
94@@ -103,7 +107,7 @@
95 >>> [pending_build] = source.createMissingBuilds()
96 >>> login(ANONYMOUS)
97
98- >>> pending_build.getEstimatedBuildStartTime()
99+ >>> pending_build.buildqueue_record.getEstimatedJobStartTime()
100 datetime.datetime(...)
101
102 The currently published component is provided via the 'current_component'
103@@ -277,7 +281,6 @@
104 This will allow us to observe the fact that a build retry does not
105 change the start time if it was set already.
106
107- >>> from zope.security.proxy import removeSecurityProxy
108 >>> from datetime import datetime, timedelta
109 >>> import pytz
110 >>> UTC = pytz.timezone('UTC')
111
112=== modified file 'lib/lp/soyuz/interfaces/build.py'
113--- lib/lp/soyuz/interfaces/build.py 2010-01-22 04:11:36 +0000
114+++ lib/lp/soyuz/interfaces/build.py 2010-02-10 11:33:14 +0000
115@@ -234,15 +234,6 @@
116 The binarypackagerelease will be attached to this specific build.
117 """
118
119- def getEstimatedBuildStartTime():
120- """Get the estimated build start time for a pending build job.
121-
122- :return: a timestamp upon success or None on failure. None
123- indicates that an estimated start time is not available.
124- :raise: AssertionError when the build job is not in the
125- `BuildStatus.NEEDSBUILD` state.
126- """
127-
128 def getFileByName(filename):
129 """Return the corresponding `ILibraryFileAlias` in this context.
130
131
132=== modified file 'lib/lp/soyuz/model/build.py'
133--- lib/lp/soyuz/model/build.py 2010-02-03 16:21:57 +0000
134+++ lib/lp/soyuz/model/build.py 2010-02-10 11:33:14 +0000
135@@ -26,8 +26,7 @@
136 from canonical.database.constants import UTC_NOW
137 from canonical.database.datetimecol import UtcDateTimeCol
138 from canonical.database.enumcol import EnumCol
139-from canonical.database.sqlbase import (
140- cursor, quote_like, SQLBase, sqlvalues)
141+from canonical.database.sqlbase import quote_like, SQLBase, sqlvalues
142 from canonical.launchpad.components.decoratedresultset import (
143 DecoratedResultSet)
144 from canonical.launchpad.database.librarian import (
145@@ -53,7 +52,6 @@
146 from lp.soyuz.interfaces.build import (
147 BuildStatus, BuildSetStatus, CannotBeRescored, IBuild, IBuildSet)
148 from lp.buildmaster.interfaces.buildbase import IBuildBase
149-from lp.buildmaster.interfaces.builder import IBuilderSet
150 from lp.soyuz.interfaces.publishing import active_publishing_status
151 from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
152 from lp.buildmaster.model.builder import Builder
153@@ -328,159 +326,6 @@
154 store.add(specific_job)
155 return specific_job
156
157- def getEstimatedBuildStartTime(self):
158- """See `IBuild`.
159-
160- The estimated dispatch time for the build job at hand is
161- calculated from the following ingredients:
162- * the start time for the head job (job at the
163- head of the respective build queue)
164- * the estimated build durations of all jobs that
165- precede the job at hand in the build queue
166- (divided by the number of machines in the respective
167- build pool)
168- If either of the above cannot be determined the estimated
169- dispatch is not known in which case the EPOCH time stamp
170- is returned.
171- """
172- # This method may only be invoked for pending jobs.
173- if self.buildstate != BuildStatus.NEEDSBUILD:
174- raise AssertionError(
175- "The start time is only estimated for pending builds.")
176-
177- # A None value indicates that the estimated dispatch time is not
178- # available.
179- result = None
180-
181- cur = cursor()
182- # For a given build job in position N in the build queue the
183- # query below sums up the estimated build durations for the
184- # jobs [1 .. N-1] i.e. for the jobs that are ahead of job N.
185- sum_query = """
186- SELECT
187- EXTRACT(EPOCH FROM SUM(BuildQueue.estimated_duration))
188- FROM
189- Archive
190- JOIN Build ON
191- Build.archive = Archive.id
192- JOIN BuildPackageJob ON
193- Build.id = BuildPackageJob.build
194- JOIN BuildQueue ON
195- BuildPackageJob.job = BuildQueue.job
196- WHERE
197- Build.buildstate = 0 AND
198- Build.processor = %s AND
199- Archive.require_virtualized = %s AND
200- Archive.enabled = TRUE AND
201- ((BuildQueue.lastscore > %s) OR
202- ((BuildQueue.lastscore = %s) AND
203- (Build.id < %s)))
204- """ % sqlvalues(self.processor, self.is_virtualized,
205- self.buildqueue_record.lastscore,
206- self.buildqueue_record.lastscore, self)
207-
208- cur.execute(sum_query)
209- # Get the sum of the estimated build time for jobs that are
210- # ahead of us in the queue.
211- [sum_of_delays] = cur.fetchone()
212-
213- # Get build dispatch time for job at the head of the queue.
214- headjob_delay = self._getHeadjobDelay()
215-
216- # Get the number of machines that are available in the build
217- # pool for this build job.
218- pool_size = getUtility(IBuilderSet).getBuildersForQueue(
219- self.processor, self.is_virtualized).count()
220-
221- # The estimated dispatch time can only be calculated for
222- # non-zero-sized build pools.
223- if pool_size > 0:
224- # This is the estimated build job start time in seconds
225- # from now.
226- start_time = 0
227-
228- if sum_of_delays is None:
229- # This job is the head job.
230- start_time = headjob_delay
231- else:
232- # There are jobs ahead of us. Divide the delay total by
233- # the number of machines available in the build pool.
234- # Please note: we need the pool size to be a floating
235- # pointer number for the purpose of the division below.
236- pool_size = float(pool_size)
237- start_time = headjob_delay + int(sum_of_delays/pool_size)
238-
239- result = (
240- datetime.datetime.utcnow() +
241- datetime.timedelta(seconds=start_time))
242-
243- return result
244-
245- def _getHeadjobDelay(self):
246- """Get estimated dispatch time for job at the head of the queue."""
247- cur = cursor()
248- # The query below yields the remaining build times (in seconds
249- # since EPOCH) for the jobs that are currently building on the
250- # machine pool of interest.
251- delay_query = """
252- SELECT
253- CAST (EXTRACT(EPOCH FROM
254- (BuildQueue.estimated_duration -
255- (NOW() - Job.date_started))) AS INTEGER)
256- AS remainder
257- FROM
258- Archive
259- JOIN Build ON
260- Build.archive = Archive.id
261- JOIN BuildPackageJob ON
262- Build.id = BuildPackageJob.build
263- JOIN BuildQueue ON
264- BuildQueue.job = BuildPackageJob.job
265- JOIN Builder ON
266- Builder.id = BuildQueue.builder
267- JOIN Job ON
268- Job.id = BuildPackageJob.job
269- WHERE
270- Archive.require_virtualized = %s AND
271- Archive.enabled = TRUE AND
272- Build.buildstate = %s AND
273- Builder.processor = %s
274- ORDER BY
275- remainder;
276- """ % sqlvalues(self.is_virtualized, BuildStatus.BUILDING,
277- self.processor)
278-
279- cur.execute(delay_query)
280- # Get the remaining build times for the jobs currently
281- # building on the respective machine pool (current build
282- # set).
283- remainders = cur.fetchall()
284- build_delays = set([int(row[0]) for row in remainders if row[0]])
285-
286- # This is the head job delay in seconds. Initialize it here.
287- if len(build_delays):
288- headjob_delay = max(build_delays)
289- else:
290- headjob_delay = 0
291-
292- # Did all currently building jobs overdraw their estimated
293- # time budget?
294- if headjob_delay < 0:
295- # Yes, this is the case. Reset the head job delay to two
296- # minutes.
297- headjob_delay = 120
298-
299- for delay in reversed(sorted(build_delays)):
300- if delay < 0:
301- # This job is currently building and taking longer
302- # than estimated i.e. we don't have a clue when it
303- # will be finished. Make a wild guess (2 minutes?).
304- delay = 120
305- if delay < headjob_delay:
306- headjob_delay = delay
307-
308- return headjob_delay
309-
310 def _parseDependencyToken(self, token):
311 """Parse the given token.
312
313
314=== modified file 'lib/lp/soyuz/templates/build-index.pt'
315--- lib/lp/soyuz/templates/build-index.pt 2009-11-11 16:02:21 +0000
316+++ lib/lp/soyuz/templates/build-index.pt 2010-02-10 11:33:14 +0000
317@@ -147,7 +147,7 @@
318 tal:content="context/dependencies">x, y, z</em>
319 </li>
320 <tal:pending condition="context/buildstate/enumvalue:NEEDSBUILD">
321- <li tal:define="eta context/getEstimatedBuildStartTime;">
322+ <li tal:define="eta context/buildqueue_record/getEstimatedJobStartTime;">
323 Start <tal:eta
324 replace="eta/fmt:approximatedate">in 3 hours</tal:eta>
325 (<span tal:replace="view/buildqueue/lastscore"/>)