Launchpad itself

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

  1. flip-the-switch-516545
  2. Merge into devel
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

flip-the-switch-516545-inc.diff
=== modified file 'lib/lp/soyuz/doc/build.txt'
--- lib/lp/soyuz/doc/build.txt 2010-02-10 10:11:46 +0000
+++ lib/lp/soyuz/doc/build.txt 2010-02-10 11:20:18 +0000
@@ -18,9 +18,9 @@
18 >>> binaries = test_publisher.getPubBinaries(18 >>> binaries = test_publisher.getPubBinaries(
19 ... binaryname='firefox', pub_source=source)19 ... binaryname='firefox', pub_source=source)
20 >>> [firefox_build] = source.getBuilds()20 >>> [firefox_build] = source.getBuilds()
21 >>> the_job = removeSecurityProxy(firefox_build.buildqueue_record.job)21 >>> job = firefox_build.buildqueue_record.job
22 >>> the_job.start()22 >>> removeSecurityProxy(job).start()
23 >>> the_job.complete()23 >>> removeSecurityProxy(job).complete()
24 >>> login(ANONYMOUS)24 >>> login(ANONYMOUS)
2525
26A build has a title which describes the context source version and in26A 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
=== modified file 'lib/lp/soyuz/doc/build-estimated-dispatch-time.txt'
--- lib/lp/soyuz/doc/build-estimated-dispatch-time.txt 2010-01-20 01:23:38 +0000
+++ lib/lp/soyuz/doc/build-estimated-dispatch-time.txt 2010-02-10 11:33:14 +0000
@@ -91,17 +91,19 @@
91in the future.91in the future.
9292
93 >>> now = datetime.utcnow()93 >>> now = datetime.utcnow()
94 >>> estimate = alsa_build.getEstimatedBuildStartTime()94 >>> def job_start_estimate(build):
95 ... return build.buildqueue_record.getEstimatedJobStartTime()
96 >>> estimate = job_start_estimate(alsa_build)
95 >>> estimate > now97 >>> estimate > now
96 True98 True
9799
98The estimated build start time may only be requested for jobs that are100The estimated build start time may only be requested for jobs that are
99pending.101pending.
100102
101 >>> cur_build.getEstimatedBuildStartTime()103 >>> job_start_estimate(cur_build)
102 Traceback (most recent call last):104 Traceback (most recent call last):
103 ...105 ...
104 AssertionError: The start time is only estimated for pending builds.106 AssertionError: The start time is only estimated for pending jobs.
105107
106Now let's add two PPA packages to the mix in order to show how builds108Now let's add two PPA packages to the mix in order to show how builds
107associated with disabled archives get ignored when it comes to the calculation109associated with disabled archives get ignored when it comes to the calculation
@@ -147,17 +149,17 @@
147build (66) its estimated dispatch time is essentially "now".149build (66) its estimated dispatch time is essentially "now".
148150
149 >>> now = datetime.utcnow()151 >>> now = datetime.utcnow()
150 >>> estimate = iceweasel_build.getEstimatedBuildStartTime()152 >>> estimate = job_start_estimate(iceweasel_build)
151 >>> estimate > now153 >>> estimate > now
152 True154 True
153 >>> estimate - now155 >>> estimate - now
154 datetime.timedelta(0, 0, ...)156 datetime.timedelta(0, 5, ...)
155157
156The 'pmount' build comes next in the queue and its estimated dispatch158The 'pmount' build comes next in the queue and its estimated dispatch
157time is the estimated build time of the 'iceweasel' package i.e. 2880159time is the estimated build time of the 'iceweasel' package i.e. 2880
158seconds (48 minutes * 60).160seconds (48 minutes * 60).
159161
160 >>> estimate = pmount_build.getEstimatedBuildStartTime()162 >>> estimate = job_start_estimate(pmount_build)
161 >>> estimate > now163 >>> estimate > now
162 True164 True
163 >>> estimate - now165 >>> estimate - now
@@ -170,8 +172,8 @@
170172
171 >>> mark.archive.disable()173 >>> mark.archive.disable()
172 >>> syncUpdate(mark.archive)174 >>> syncUpdate(mark.archive)
173 >>> estimate = pmount_build.getEstimatedBuildStartTime()175 >>> estimate = job_start_estimate(pmount_build)
174 >>> estimate > now176 >>> estimate > now
175 True177 True
176 >>> estimate - now178 >>> estimate - now
177 datetime.timedelta(0, 0, ...)179 datetime.timedelta(0, 5, ...)
178180
=== 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 11:33:14 +0000
@@ -8,6 +8,7 @@
8launchpad build daemon.8launchpad build daemon.
99
10 # Create a 'mozilla-firefox' build in ubuntutest/breezy-autotest/i386.10 # Create a 'mozilla-firefox' build in ubuntutest/breezy-autotest/i386.
11 >>> from zope.security.proxy import removeSecurityProxy
11 >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher12 >>> from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
12 >>> login('foo.bar@canonical.com')13 >>> login('foo.bar@canonical.com')
13 >>> test_publisher = SoyuzTestPublisher()14 >>> test_publisher = SoyuzTestPublisher()
@@ -17,6 +18,9 @@
17 >>> binaries = test_publisher.getPubBinaries(18 >>> binaries = test_publisher.getPubBinaries(
18 ... binaryname='firefox', pub_source=source)19 ... binaryname='firefox', pub_source=source)
19 >>> [firefox_build] = source.getBuilds()20 >>> [firefox_build] = source.getBuilds()
21 >>> job = firefox_build.buildqueue_record.job
22 >>> removeSecurityProxy(job).start()
23 >>> removeSecurityProxy(job).complete()
20 >>> login(ANONYMOUS)24 >>> login(ANONYMOUS)
2125
22A build has a title which describes the context source version and in26A build has a title which describes the context source version and in
@@ -89,10 +93,10 @@
89The 'firefox_build' is already finished and requesting the estimated build93The 'firefox_build' is already finished and requesting the estimated build
90start time makes no sense. Hence an exception is raised.94start time makes no sense. Hence an exception is raised.
9195
92 >>> firefox_build.getEstimatedBuildStartTime()96 >>> firefox_build.buildqueue_record.getEstimatedJobStartTime()
93 Traceback (most recent call last):97 Traceback (most recent call last):
94 ...98 ...
95 AssertionError: The start time is only estimated for pending builds.99 AssertionError: The start time is only estimated for pending jobs.
96100
97On a build job in state `NEEDSBUILD` we can ask for its estimated101On a build job in state `NEEDSBUILD` we can ask for its estimated
98build start time.102build start time.
@@ -103,7 +107,7 @@
103 >>> [pending_build] = source.createMissingBuilds()107 >>> [pending_build] = source.createMissingBuilds()
104 >>> login(ANONYMOUS)108 >>> login(ANONYMOUS)
105109
106 >>> pending_build.getEstimatedBuildStartTime()110 >>> pending_build.buildqueue_record.getEstimatedJobStartTime()
107 datetime.datetime(...)111 datetime.datetime(...)
108112
109The currently published component is provided via the 'current_component'113The currently published component is provided via the 'current_component'
@@ -277,7 +281,6 @@
277This will allow us to observe the fact that a build retry does not281This will allow us to observe the fact that a build retry does not
278change the start time if it was set already.282change the start time if it was set already.
279283
280 >>> from zope.security.proxy import removeSecurityProxy
281 >>> from datetime import datetime, timedelta284 >>> from datetime import datetime, timedelta
282 >>> import pytz285 >>> import pytz
283 >>> UTC = pytz.timezone('UTC')286 >>> UTC = pytz.timezone('UTC')
284287
=== modified file 'lib/lp/soyuz/interfaces/build.py'
--- lib/lp/soyuz/interfaces/build.py 2010-01-22 04:11:36 +0000
+++ lib/lp/soyuz/interfaces/build.py 2010-02-10 11:33:14 +0000
@@ -234,15 +234,6 @@
234 The binarypackagerelease will be attached to this specific build.234 The binarypackagerelease will be attached to this specific build.
235 """235 """
236236
237 def getEstimatedBuildStartTime():
238 """Get the estimated build start time for a pending build job.
239
240 :return: a timestamp upon success or None on failure. None
241 indicates that an estimated start time is not available.
242 :raise: AssertionError when the build job is not in the
243 `BuildStatus.NEEDSBUILD` state.
244 """
245
246 def getFileByName(filename):237 def getFileByName(filename):
247 """Return the corresponding `ILibraryFileAlias` in this context.238 """Return the corresponding `ILibraryFileAlias` in this context.
248239
249240
=== modified file 'lib/lp/soyuz/model/build.py'
--- lib/lp/soyuz/model/build.py 2010-02-03 16:21:57 +0000
+++ lib/lp/soyuz/model/build.py 2010-02-10 11:33:14 +0000
@@ -26,8 +26,7 @@
26from canonical.database.constants import UTC_NOW26from canonical.database.constants import UTC_NOW
27from canonical.database.datetimecol import UtcDateTimeCol27from canonical.database.datetimecol import UtcDateTimeCol
28from canonical.database.enumcol import EnumCol28from canonical.database.enumcol import EnumCol
29from canonical.database.sqlbase import (29from canonical.database.sqlbase import quote_like, SQLBase, sqlvalues
30 cursor, quote_like, SQLBase, sqlvalues)
31from canonical.launchpad.components.decoratedresultset import (30from canonical.launchpad.components.decoratedresultset import (
32 DecoratedResultSet)31 DecoratedResultSet)
33from canonical.launchpad.database.librarian import (32from canonical.launchpad.database.librarian import (
@@ -53,7 +52,6 @@
53from lp.soyuz.interfaces.build import (52from lp.soyuz.interfaces.build import (
54 BuildStatus, BuildSetStatus, CannotBeRescored, IBuild, IBuildSet)53 BuildStatus, BuildSetStatus, CannotBeRescored, IBuild, IBuildSet)
55from lp.buildmaster.interfaces.buildbase import IBuildBase54from lp.buildmaster.interfaces.buildbase import IBuildBase
56from lp.buildmaster.interfaces.builder import IBuilderSet
57from lp.soyuz.interfaces.publishing import active_publishing_status55from lp.soyuz.interfaces.publishing import active_publishing_status
58from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease56from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
59from lp.buildmaster.model.builder import Builder57from lp.buildmaster.model.builder import Builder
@@ -328,159 +326,6 @@
328 store.add(specific_job)326 store.add(specific_job)
329 return specific_job327 return specific_job
330328
331 def getEstimatedBuildStartTime(self):
332 """See `IBuild`.
333
334 The estimated dispatch time for the build job at hand is
335 calculated from the following ingredients:
336 * the start time for the head job (job at the
337 head of the respective build queue)
338 * the estimated build durations of all jobs that
339 precede the job at hand in the build queue
340 (divided by the number of machines in the respective
341 build pool)
342 If either of the above cannot be determined the estimated
343 dispatch is not known in which case the EPOCH time stamp
344 is returned.
345 """
346 # This method may only be invoked for pending jobs.
347 if self.buildstate != BuildStatus.NEEDSBUILD:
348 raise AssertionError(
349 "The start time is only estimated for pending builds.")
350
351 # A None value indicates that the estimated dispatch time is not
352 # available.
353 result = None
354
355 cur = cursor()
356 # For a given build job in position N in the build queue the
357 # query below sums up the estimated build durations for the
358 # jobs [1 .. N-1] i.e. for the jobs that are ahead of job N.
359 sum_query = """
360 SELECT
361 EXTRACT(EPOCH FROM SUM(BuildQueue.estimated_duration))
362 FROM
363 Archive
364 JOIN Build ON
365 Build.archive = Archive.id
366 JOIN BuildPackageJob ON
367 Build.id = BuildPackageJob.build
368 JOIN BuildQueue ON
369 BuildPackageJob.job = BuildQueue.job
370 WHERE
371 Build.buildstate = 0 AND
372 Build.processor = %s AND
373 Archive.require_virtualized = %s AND
374 Archive.enabled = TRUE AND
375 ((BuildQueue.lastscore > %s) OR
376 ((BuildQueue.lastscore = %s) AND
377 (Build.id < %s)))
378 """ % sqlvalues(self.processor, self.is_virtualized,
379 self.buildqueue_record.lastscore,
380 self.buildqueue_record.lastscore, self)
381
382 cur.execute(sum_query)
383 # Get the sum of the estimated build time for jobs that are
384 # ahead of us in the queue.
385 [sum_of_delays] = cur.fetchone()
386
387 # Get build dispatch time for job at the head of the queue.
388 headjob_delay = self._getHeadjobDelay()
389
390 # Get the number of machines that are available in the build
391 # pool for this build job.
392 pool_size = getUtility(IBuilderSet).getBuildersForQueue(
393 self.processor, self.is_virtualized).count()
394
395 # The estimated dispatch time can only be calculated for
396 # non-zero-sized build pools.
397 if pool_size > 0:
398 # This is the estimated build job start time in seconds
399 # from now.
400 start_time = 0
401
402 if sum_of_delays is None:
403 # This job is the head job.
404 start_time = headjob_delay
405 else:
406 # There are jobs ahead of us. Divide the delay total by
407 # the number of machines available in the build pool.
408 # Please note: we need the pool size to be a floating
409 # pointer number for the purpose of the division below.
410 pool_size = float(pool_size)
411 start_time = headjob_delay + int(sum_of_delays/pool_size)
412
413 result = (
414 datetime.datetime.utcnow() +
415 datetime.timedelta(seconds=start_time))
416
417 return result
418
419 def _getHeadjobDelay(self):
420 """Get estimated dispatch time for job at the head of the queue."""
421 cur = cursor()
422 # The query below yields the remaining build times (in seconds
423 # since EPOCH) for the jobs that are currently building on the
424 # machine pool of interest.
425 delay_query = """
426 SELECT
427 CAST (EXTRACT(EPOCH FROM
428 (BuildQueue.estimated_duration -
429 (NOW() - Job.date_started))) AS INTEGER)
430 AS remainder
431 FROM
432 Archive
433 JOIN Build ON
434 Build.archive = Archive.id
435 JOIN BuildPackageJob ON
436 Build.id = BuildPackageJob.build
437 JOIN BuildQueue ON
438 BuildQueue.job = BuildPackageJob.job
439 JOIN Builder ON
440 Builder.id = BuildQueue.builder
441 JOIN Job ON
442 Job.id = BuildPackageJob.job
443 WHERE
444 Archive.require_virtualized = %s AND
445 Archive.enabled = TRUE AND
446 Build.buildstate = %s AND
447 Builder.processor = %s
448 ORDER BY
449 remainder;
450 """ % sqlvalues(self.is_virtualized, BuildStatus.BUILDING,
451 self.processor)
452
453 cur.execute(delay_query)
454 # Get the remaining build times for the jobs currently
455 # building on the respective machine pool (current build
456 # set).
457 remainders = cur.fetchall()
458 build_delays = set([int(row[0]) for row in remainders if row[0]])
459
460 # This is the head job delay in seconds. Initialize it here.
461 if len(build_delays):
462 headjob_delay = max(build_delays)
463 else:
464 headjob_delay = 0
465
466 # Did all currently building jobs overdraw their estimated
467 # time budget?
468 if headjob_delay < 0:
469 # Yes, this is the case. Reset the head job delay to two
470 # minutes.
471 headjob_delay = 120
472
473 for delay in reversed(sorted(build_delays)):
474 if delay < 0:
475 # This job is currently building and taking longer
476 # than estimated i.e. we don't have a clue when it
477 # will be finished. Make a wild guess (2 minutes?).
478 delay = 120
479 if delay < headjob_delay:
480 headjob_delay = delay
481
482 return headjob_delay
483
484 def _parseDependencyToken(self, token):329 def _parseDependencyToken(self, token):
485 """Parse the given token.330 """Parse the given token.
486331
487332
=== modified file 'lib/lp/soyuz/templates/build-index.pt'
--- lib/lp/soyuz/templates/build-index.pt 2009-11-11 16:02:21 +0000
+++ lib/lp/soyuz/templates/build-index.pt 2010-02-10 11:33:14 +0000
@@ -147,7 +147,7 @@
147 tal:content="context/dependencies">x, y, z</em>147 tal:content="context/dependencies">x, y, z</em>
148 </li>148 </li>
149 <tal:pending condition="context/buildstate/enumvalue:NEEDSBUILD">149 <tal:pending condition="context/buildstate/enumvalue:NEEDSBUILD">
150 <li tal:define="eta context/getEstimatedBuildStartTime;">150 <li tal:define="eta context/buildqueue_record/getEstimatedJobStartTime;">
151 Start <tal:eta151 Start <tal:eta
152 replace="eta/fmt:approximatedate">in 3 hours</tal:eta>152 replace="eta/fmt:approximatedate">in 3 hours</tal:eta>
153 (<span tal:replace="view/buildqueue/lastscore"/>)153 (<span tal:replace="view/buildqueue/lastscore"/>)