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

[Muharem Hrnjadovic]

[Muharem Hrnjadovic (al-maisan)]

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.

[Gavin Panella (allenap)]

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

[Muharem Hrnjadovic (al-maisan)]

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

=== 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 @@
 in the future.
 
     >>> now = datetime.utcnow()
-    >>> estimate = alsa_build.getEstimatedBuildStartTime()
+    >>> def job_start_estimate(build):
+    ...     return build.buildqueue_record.getEstimatedJobStartTime()
+    >>> estimate = job_start_estimate(alsa_build)
     >>> estimate > now
     True
 
 The estimated build start time may only be requested for jobs that are
 pending.
 
-    >>> cur_build.getEstimatedBuildStartTime()
+    >>> job_start_estimate(cur_build)
     Traceback (most recent call last):
     ...
-    AssertionError: The start time is only estimated for pending builds.
+    AssertionError: The start time is only estimated for pending jobs.
 
 Now let's add two PPA packages to the mix in order to show how builds
 associated with disabled archives get ignored when it comes to the calculation
@@ -147,17 +149,17 @@
 build (66) its estimated dispatch time is essentially "now".
 
     >>> now = datetime.utcnow()
-    >>> estimate = iceweasel_build.getEstimatedBuildStartTime()
+    >>> estimate = job_start_estimate(iceweasel_build)
     >>> estimate > now
     True
     >>> estimate - now
-    datetime.timedelta(0, 0, ...)
+    datetime.timedelta(0, 5, ...)
 
 The 'pmount' build comes next in the queue and its estimated dispatch
 time is the estimated build time of the 'iceweasel' package i.e. 2880
 seconds (48 minutes * 60).
 
-    >>> estimate = pmount_build.getEstimatedBuildStartTime()
+    >>> estimate = job_start_estimate(pmount_build)
     >>> estimate > now
     True
     >>> estimate - now
@@ -170,8 +172,8 @@
 
     >>> mark.archive.disable()
     >>> syncUpdate(mark.archive)
-    >>> estimate = pmount_build.getEstimatedBuildStartTime()
+    >>> estimate = job_start_estimate(pmount_build)
     >>> estimate > now
     True
     >>> estimate - now
-    datetime.timedelta(0, 0, ...)
+    datetime.timedelta(0, 5, ...)

=== 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 @@
 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('[e-mail]')
     >>> test_publisher = SoyuzTestPublisher()
@@ -17,6 +18,9 @@
     >>> binaries = test_publisher.getPubBinaries(
     ...     binaryname='firefox', pub_source=source)
     >>> [firefox_build] = source.getBuilds()
+    >>> job = firefox_build.buildqueue_record.job
+    >>> removeSecurityProxy(job).start()
+    >>> removeSecurityProxy(job).complete()
     >>> login(ANONYMOUS)
 
 A build has a title which describes the context source version and in
@@ -89,10 +93,10 @@
 The 'firefox_build' is already finished and requesting the estimated build
 start time makes no sense. Hence an exception is raised.
 
-    >>> firefox_build.getEstimatedBuildStartTime()
+    >>> firefox_build.buildqueue_record.getEstimatedJobStartTime()
     Traceback (most recent call last):
     ...
-    AssertionError: The start time is only estimated for pending builds.
+    AssertionError: The start time is only estimated for pending jobs.
 
 On a build job in state `NEEDSBUILD` we can ask for its estimated
 build start time.
@@ -103,7 +107,7 @@
     >>> [pending_build] = source.createMissingBuilds()
     >>> login(ANONYMOUS)
 
-    >>> pending_build.getEstimatedBuildStartTime()
+    >>> pending_build.buildqueue_record.getEstimatedJobStartTime()
     datetime.datetime(...)
 
 The currently published component is provided via the 'current_component'
@@ -277,7 +281,6 @@
 This will allow us to observe the fact that a build retry does not
 change the start time if it was set already.
 
-    >>> from zope.security.proxy import removeSecurityProxy
     >>> from datetime import datetime, timedelta
     >>> import pytz
     >>> UTC = pytz.timezone('UTC')

=== 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 @@
         The binarypackagerelease will be attached to this specific build.
         """
 
-    def getEstimatedBuildStartTime():
-        """Get the estimated build start time for a pending build job.
-
-        :return: a timestamp upon success or None on failure. None
-            indicates that an estimated start time is not available.
-        :raise: AssertionError when the build job is not in the
-            `BuildStatus.NEEDSBUILD` state.
-        """
-
     def getFileByName(filename):
         """Return the corresponding `ILibraryFileAlias` in this context.
 

=== 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 @@
 from canonical.database.constants import UTC_NOW
 from canonical.database.datetimecol import UtcDateTimeCol
 from canonical.database.enumcol import EnumCol
-from canonical.database.sqlbase import (
-    cursor, quote_like, SQLBase, sqlvalues)
+from canonical.database.sqlbase import quote_like, SQLBase, sqlvalues
 from canonical.launchpad.components.decoratedresultset import (
     DecoratedResultSet)
 from canonical.launchpad.database.librarian import (
@@ -53,7 +52,6 @@
 from lp.soyuz.interfaces.build import (
     BuildStatus, BuildSetStatus, CannotBeRescored, IBuild, IBuildSet)
 from lp.buildmaster.interfaces.buildbase import IBuildBase
-from lp.buildmaster.interfaces.builder import IBuilderSet
 from lp.soyuz.interfaces.publishing import active_publishing_status
 from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
 from lp.buildmaster.model.builder import Builder
@@ -328,159 +326,6 @@
         store.add(specific_job)
         return specific_job
 
-    def getEstimatedBuildStartTime(self):
-        """See `IBuild`.
-
-        The estimated dispatch time for the build job at hand is
-        calculated from the following ingredients:
-            * the start time for the head job (job at the
-              head of the respective build queue)
-            * the estimated build durations of all jobs that
-              precede the job at hand in the build queue
-              (divided by the number of machines in the respective
-              build pool)
-        If either of the above cannot be determined the estimated
-        dispatch is not known in which case the EPOCH time stamp
-        is returned.
-        """
-        # This method may only be invoked for pending jobs.
-        if self.buildstate != BuildStatus.NEEDSBUILD:
-            raise AssertionError(
-                "The start time is only estimated for pending builds.")
-
-        # A None value indicates that the estimated dispatch time is not
-        # available.
-        result = None
-
-        cur = cursor()
-        # For a given build job in position N in the build queue the
-        # query below sums up the estimated build durations for the
-        # jobs [1 .. N-1] i.e. for the jobs that are ahead of job N.
-        sum_query = """
-            SELECT
-                EXTRACT(EPOCH FROM SUM(BuildQueue.estimated_duration))
-            FROM
-                Archive
-                JOIN Build ON
-                    Build.archive = Archive.id
-                JOIN BuildPackageJob ON
-                    Build.id = BuildPackageJob.build
-                JOIN BuildQueue ON
-                    BuildPackageJob.job = BuildQueue.job
-            WHERE
-                Build.buildstate = 0 AND
-                Build.processor = %s AND
-                Archive.require_virtualized = %s AND
-                Archive.enabled = TRUE AND
-                ((BuildQueue.lastscore > %s) OR
-                 ((BuildQueue.lastscore = %s) AND
-                  (Build.id < %s)))
-             """ % sqlvalues(self.processor, self.is_virtualized,
-                      self.buildqueue_record.lastscore,
-                      self.buildqueue_record.lastscore, self)
-
-        cur.execute(sum_query)
-        # Get the sum of the estimated build time for jobs that are
-        # ahead of us in the queue.
-        [sum_of_delays] = cur.fetchone()
-
-        # Get build dispatch time for job at the head of the queue.
-        headjob_delay = self._getHeadjobDelay()
-
-        # Get the number of machines that are available in the build
-        # pool for this build job.
-        pool_size = getUtility(IBuilderSet).getBuildersForQueue(
-            self.processor, self.is_virtualized).count()
-
-        # The estimated dispatch time can only be calculated for
-        # non-zero-sized build pools.
-        if pool_size > 0:
-            # This is the estimated build job start time in seconds
-            # from now.
-            start_time = 0
-
-            if sum_of_delays is None:
-                # This job is the head job.
-                start_time = headjob_delay
-            else:
-                # There are jobs ahead of us. Divide the delay total by
-                # the number of machines available in the build pool.
-                # Please note: we need the pool size to be a floating
-                # pointer number for the purpose of the division below.
-                pool_size = float(pool_size)
-                start_time = headjob_delay + int(sum_of_delays/pool_size)
-
-            result = (
-                datetime.datetime.utcnow() +
-                datetime.timedelta(seconds=start_time))
-
-        return result
-
-    def _getHeadjobDelay(self):
-        """Get estimated dispatch time for job at the head of the queue."""
-        cur = cursor()
-        # The query below yields the remaining build times (in seconds
-        # since EPOCH) for the jobs that are currently building on the
-        # machine pool of interest.
-        delay_query = """
-            SELECT
-                CAST (EXTRACT(EPOCH FROM
-                        (BuildQueue.estimated_duration -
-                        (NOW() - Job.date_started))) AS INTEGER)
-                    AS remainder
-            FROM
-                Archive
-                JOIN Build ON
-                    Build.archive = Archive.id
-                JOIN BuildPackageJob ON
-                    Build.id = BuildPackageJob.build
-                JOIN BuildQueue ON
-                    BuildQueue.job = BuildPackageJob.job
-                JOIN Builder ON
-                    Builder.id = BuildQueue.builder
-                JOIN Job ON
-                    Job.id = BuildPackageJob.job
-            WHERE
-                Archive.require_virtualized = %s AND
-                Archive.enabled = TRUE AND
-                Build.buildstate = %s AND
-                Builder.processor = %s
-            ORDER BY
-                remainder;
-            """ % sqlvalues(self.is_virtualized, BuildStatus.BUILDING,
-                    self.processor)
-
-        cur.execute(delay_query)
-        # Get the remaining build times for the jobs currently
-        # building on the respective machine pool (current build
-        # set).
-        remainders = cur.fetchall()
-        build_delays = set([int(row[0]) for row in remainders if row[0]])
-
-        # This is the head job delay in seconds. Initialize it here.
-        if len(build_delays):
-            headjob_delay = max(build_delays)
-        else:
-            headjob_delay = 0
-
-        # Did all currently building jobs overdraw their estimated
-        # time budget?
-        if headjob_delay < 0:
-            # Yes, this is the case. Reset the head job delay to two
-            # minutes.
-            headjob_delay = 120
-
-        for delay in reversed(sorted(build_delays)):
-            if delay < 0:
-                # This job is currently building and taking longer
-                # than estimated i.e. we don't have a clue when it
-                # will be finished. Make a wild guess (2 minutes?).
-                delay = 120
-            if delay < headjob_delay:
-                headjob_delay = delay
-
-        return headjob_delay
-
     def _parseDependencyToken(self, token):
         """Parse the given token.
 

=== 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 @@
          tal:content="context/dependencies">x, y, z</em>
       </li>
       <tal:pending condition="context/buildstate/enumvalue:NEEDSBUILD">
-        <li tal:define="eta context/getEstimatedBuildStartTime;">
+        <li tal:define="eta context/buildqueue_record/getEstimatedJobStartTime;">
           Start <tal:eta
             replace="eta/fmt:approximatedate">in 3 hours</tal:eta>
             (<span tal:replace="view/buildqueue/lastscore"/>)