Launchpad itself

Merge lp:~al-maisan/launchpad/pending-jobs-499861 into lp:launchpad

pending-jobs-499861
Merge into devel

Proposed by Muharem Hrnjadovic on 2009-12-23

Status:

Merged

Approved by:

Jeroen T. Vermeulen on 2009-12-24

Approved revision:

not available

Merged at revision:

not available

Proposed branch:

lp:~al-maisan/launchpad/pending-jobs-499861

Merge into:

lp:launchpad

Diff against target:

567 lines (+507/-4)

4 files modified

lib/lp/buildmaster/interfaces/buildfarmjob.py (+66/-1)
lib/lp/buildmaster/model/buildfarmjob.py (+10/-0)
lib/lp/soyuz/model/buildpackagejob.py (+47/-3)
lib/lp/soyuz/tests/test_buildpackagejob.py (+384/-0)

To merge this branch:

bzr merge lp:~al-maisan/launchpad/pending-jobs-499861

Medium

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Jeroen T. Vermeulen (community)	code		Approve on 2009-12-24
Jonathan Lange (community)		2009-12-23	Approve on 2009-12-24
Review via email: mp+16550@code.launchpad.net

Revision history for this message

Muharem Hrnjadovic (al-maisan) wrote on 2009-12-23:

Hi there!

this branch

    - introduces a method (IBuildFarmJob.pendingJobsQuery()) that all
      Soyuz build farm jobs need to implement in order to enable a
      general (i.e. across all job types) job dispatch time estimation.
    - implements the pendingJobsQuery() method for the `BuildPackageJob`
      class i.e. for binary builds.

The logic was broken out from a larger branch that implements the
general job dispatch time estimation logic so it land earlier allowing
jtv and mwhudson to implement the pendingJobsQuery() method inside their
respective translation and recipe build job classes.

Tests to run:

bin/test -vv -t TestBuildPackageJob

No pertinent "make lint" errors or warnings.

Revision history for this message

Jonathan Lange (jml) wrote on 2009-12-24:

Download full text (26.2 KiB)

On Thu, Dec 24, 2009 at 3:45 AM, Muharem Hrnjadovic
<email address hidden> wrote:
> Muharem Hrnjadovic has proposed merging lp:~al-maisan/launchpad/pending-jobs-499861 into lp:launchpad/devel.
>
> Requested reviews:
> Canonical Launchpad Engineering (launchpad)
> Related bugs:
> #499861 Define interface allowing build farm job classes to select pending jobs
> https://bugs.launchpad.net/bugs/499861
>
>
> Hi there!
>
> this branch
>
> - introduces a method (IBuildFarmJob.pendingJobsQuery()) that all
> Soyuz build farm jobs need to implement in order to enable a
> general (i.e. across all job types) job dispatch time estimation.

Although this sounds like a good idea to me, I'm confused by the
terminology. Why do we ask a job (singular) to return a list of
pending jobs (plural)? It seems like this method would be better
placed on an interface representing a type of job.

...
> === modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
> --- lib/lp/buildmaster/interfaces/buildfarmjob.py 2009-12-01 08:43:43 +0000
> +++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2009-12-23 16:45:30 +0000
> @@ -12,8 +12,12 @@
> 'BuildFarmJobType',
> ]
>
> -from zope.interface import Interface
> +from zope.interface import Interface, Attribute
> +
> +from canonical.launchpad import _
> from lazr.enum import DBEnumeratedType, DBItem
> +from lazr.restful.fields import Reference
> +from lp.soyuz.interfaces.processor import IProcessor
>
>
> class BuildFarmJobType(DBEnumeratedType):
> @@ -69,3 +73,64 @@
> def jobAborted():
> """'Job aborted' life cycle event, handle as appropriate."""
>
> + def pendingJobsQuery(minscore, processor, virtualized):

Given that this is a method, it's name should be in the imperative,
e.g. getPendingJobsQuery.

> + """String SELECT query yielding pending jobs with given minimum score.
> +
> + This will be used for the purpose of job dispatch time estimation
> + for a build job of interest (JOI).
> + In order to estimate the dispatch time for the JOI we need to
> + calculate the sum of the estimated durations of the *pending* jobs
> + ahead of JOI.
> +
> + Depending on the build farm job type the JOI may or may not be tied
> + to a particular processor type.
> + Binary builds for example are always built for a specific processor
> + whereas "create a source package from recipe" type jobs do not care
> + about processor types or virtualization.
> +
> + When implementing this method for processor independent build farm job
> + types (e.g. recipe build) you may safely ignore the `processor` and
> + `virtualized` parameters.
> +
> + The SELECT query to be returned needs to select the following data
> +
> + 1 - BuildQueue.job
> + 2 - BuildQueue.lastscore
> + 3 - BuildQueue.estimated_duration
> + 4 - Processor.id [optional]
> + 5 - virtualized [optional]
> +
> + Please do *not* order the result set since it will be UNIONed and
> + ordered only then.
> +

Thanks, this is a useful docstring.

> + Job...

On Thu, Dec 24, 2009 at 3:45 AM, Muharem Hrnjadovic
<muharem@canonical.com> wrote:
> Muharem Hrnjadovic has proposed merging lp:~al-maisan/launchpad/pending-jobs-499861 into lp:launchpad/devel.
>
>    Requested reviews:
>    Canonical Launchpad Engineering (launchpad)
> Related bugs:
>  #499861 Define interface allowing build farm job classes to select pending jobs
>  https://bugs.launchpad.net/bugs/499861
>
>
> Hi there!
>
> this branch
>
>    - introduces a method (IBuildFarmJob.pendingJobsQuery()) that all
>      Soyuz build farm jobs need to implement in order to enable a
>      general (i.e. across all job types) job dispatch time estimation.

...
> === modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
> --- lib/lp/buildmaster/interfaces/buildfarmjob.py       2009-12-01 08:43:43 +0000
> +++ lib/lp/buildmaster/interfaces/buildfarmjob.py       2009-12-23 16:45:30 +0000
> @@ -12,8 +12,12 @@
>     'BuildFarmJobType',
>     ]
>
> -from zope.interface import Interface
> +from zope.interface import Interface, Attribute
> +
> +from canonical.launchpad import _
>  from lazr.enum import DBEnumeratedType, DBItem
> +from lazr.restful.fields import Reference
> +from lp.soyuz.interfaces.processor import IProcessor
>
>
>  class BuildFarmJobType(DBEnumeratedType):
> @@ -69,3 +73,64 @@
>     def jobAborted():
>         """'Job aborted' life cycle event, handle as appropriate."""
>
> +    def pendingJobsQuery(minscore, processor, virtualized):

Given that this is a method, it's name should be in the imperative,
e.g. getPendingJobsQuery.

> +        """String SELECT query yielding pending jobs with given minimum score.
> +
> +        This will be used for the purpose of job dispatch time estimation
> +        for a build job of interest (JOI).
> +        In order to estimate the dispatch time for the JOI we need to
> +        calculate the sum of the estimated durations of the *pending* jobs
> +        ahead of JOI.
> +
> +        Depending on the build farm job type the JOI may or may not be tied
> +        to a particular processor type.
> +        Binary builds for example are always built for a specific processor
> +        whereas "create a source package from recipe" type jobs do not care
> +        about processor types or virtualization.
> +
> +        When implementing this method for processor independent build farm job
> +        types (e.g. recipe build) you may safely ignore the `processor` and
> +        `virtualized` parameters.
> +
> +        The SELECT query to be returned needs to select the following data
> +
> +            1 - BuildQueue.job
> +            2 - BuildQueue.lastscore
> +            3 - BuildQueue.estimated_duration
> +            4 - Processor.id    [optional]
> +            5 - virtualized     [optional]
> +
> +        Please do *not* order the result set since it will be UNIONed and
> +        ordered only then.
> +

Thanks, this is a useful docstring.

> +        Job types that are processor independent or do not care about
> +        virtualization should return NULL for the optional data in the result
> +        set.
> +
> +        :param minscore: the pending jobs selected by the returned
> +            query should have score >= minscore

This should be min_score or minimum_score.

> +        :param processor: the job of interest (JOI) is tied to this
> +            processor, this information can be used to further narrow
> +            down the pending jobs that will result from the returned
> +            query. Please note: processor independent job types may
> +            safely ignore this parameter.

Does this mean, "the type of processor that the jobs are expected to
run on"? If so, could you please say that?

> +        :param virtualized: the job of interest (JOI) can only run
> +            on builders with this virtualization setting.

If I understand correctly, when this is passed in, it means that we're
telling the job that we intend to run it with a particular
virtualization setting. The way it's phrased in the docstring as-is
seems a bit backwards, but I can't think of how to phrase it better.
Can you?

> +            Again, this information can be used to narrow down the
> +            pending jobs that will result from the returned query and
> +            processor independent job types may safely ignore it.
> +        :return: a string SELECT clause that can be used to find
> +            the pending jobs of the appropriate type.

Here's my big question: why a string?

Why not return a Storm ResultSet or a Select object?

> +
> +    processor = Reference(
> +        IProcessor, title=_("Processor"),
> +        description=_(
> +            "The Processor required by this build farm job. "
> +            "For processor independent job types please return None."))
> +

"processor-independent" should have a hyphen.

> +    virtualized = Attribute(
> +        _(
> +            "The virtualization setting required by this build farm job. "
> +            "For job types that do not care about virtualization please "
> +            "return None."))
>

You don't mention either of these new attributes in your cover letter
-- what's their purpose?

...
> === modified file 'lib/lp/soyuz/model/buildpackagejob.py'
> --- lib/lp/soyuz/model/buildpackagejob.py       2009-11-13 20:39:36 +0000
> +++ lib/lp/soyuz/model/buildpackagejob.py       2009-12-23 16:45:30 +0000
> @@ -13,9 +13,13 @@
>  from zope.interface import implements
>
>  from canonical.database.constants import UTC_NOW
> -from canonical.launchpad.interfaces import SourcePackageUrgency
> -from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
> +from canonical.database.sqlbase import sqlvalues
> +
> +from lp.buildmaster.interfaces.buildfarmjob import (
> +    BuildFarmJobType, IBuildFarmJob)
> +from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
>  from lp.registry.interfaces.pocket import PackagePublishingPocket
> +from lp.services.job.interfaces.job import JobStatus
>  from lp.soyuz.interfaces.archive import ArchivePurpose
>  from lp.soyuz.interfaces.build import BuildStatus
>  from lp.soyuz.interfaces.buildpackagejob import IBuildPackageJob
> @@ -166,3 +170,41 @@
>         # fix it.
>         self.build.buildstate = BuildStatus.BUILDING
>
> +    @staticmethod
> +    def pendingJobsQuery(minscore, processor, virtualized):
> +        """See `IBuildFarmJob`."""
> +        return """
> +            SELECT
> +                BuildQueue.job,
> +                BuildQueue.lastscore,
> +                BuildQueue.estimated_duration,
> +                Build.processor AS processor,
> +                Archive.require_virtualized AS virtualized
> +            FROM
> +                BuildQueue, Build, BuildPackageJob, Archive, Job
> +            WHERE
> +                BuildQueue.job_type = %s
> +                AND BuildPackageJob.job = BuildQueue.job
> +                AND BuildPackageJob.job = Job.id
> +                AND Job.status = %s
> +                AND BuildPackageJob.build = Build.id
> +                AND Build.buildstate = %s
> +                AND Build.archive = Archive.id
> +                AND Archive.enabled = TRUE
> +                AND BuildQueue.lastscore >= %s
> +                AND Build.processor = %s
> +                AND Archive.require_virtualized = %s
> +        """ % sqlvalues(
> +            BuildFarmJobType.PACKAGEBUILD, JobStatus.WAITING,
> +            BuildStatus.NEEDSBUILD, minscore, processor, virtualized)
> +
> +    @property
> +    def processor(self):
> +        """See `IBuildFarmJob`."""
> +        return self.build.processor
> +
> +    @property
> +    def virtualized(self):
> +        """See `IBuildFarmJob`."""
> +        return self.build.is_virtualized
> +
>

> === added file 'lib/lp/soyuz/tests/test_buildpackagejob.py'
> --- lib/lp/soyuz/tests/test_buildpackagejob.py  1970-01-01 00:00:00 +0000
> +++ lib/lp/soyuz/tests/test_buildpackagejob.py  2009-12-23 16:45:30 +0000
> @@ -0,0 +1,377 @@
> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Test BuildQueue features."""
> +
> +import unittest
> +
> +from datetime import datetime, timedelta
> +from pytz import utc
> +
> +from zope.component import getUtility
> +
> +from canonical.launchpad.webapp.interfaces import (
> +    IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
> +from canonical.testing import LaunchpadZopelessLayer
> +
> +from lp.soyuz.interfaces.archive import ArchivePurpose
> +from lp.soyuz.interfaces.build import BuildStatus, IBuildSet
> +from lp.soyuz.interfaces.builder import IBuilderSet
> +from lp.soyuz.model.processor import ProcessorFamilySet
> +from lp.soyuz.interfaces.publishing import PackagePublishingStatus
> +from lp.soyuz.model.build import Build
> +from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
> +from lp.testing import TestCaseWithFactory
> +
> +
> +def find_job(test, name, processor='386'):

Docstring please.

> +    result = None
> +    for build in test.builds:
> +        if (build.sourcepackagerelease.name == name
> +            and build.processor.name == processor):
> +            result = build

Were it me, I'd return (build, build.buildqueue_record) here, forget
the break and the 'result' variable -- it's pretty much the standard
way of spelling "linear search" in Python.

> +            break
> +    if result is not None:
> +        return (result, result.buildqueue_record)
> +    else:
> +        return (None, None)
> +
> +def builder_key(build):
> +    return (build.processor.id,build.is_virtualized)
> +

Space after the comma please. Also, two blank lines between top-level
function definitions.

Also, you might consider defining this method within 'nth_builder'.

> +def nth_builder(test, build, n):

Docstring please.

> +    builder = None
> +    builders = test.builders.get(builder_key(build), [])
> +    try:
> +        builder = builders[n-1]
> +    except IndexError:
> +        pass
> +    return builder
> +

Second blank line please.

> +def assign_to_builder(test, job_name, builder_number, processor='386'):

Docstring please.

> +    build, bq = find_job(test, job_name, processor)
> +    builder = nth_builder(test, build, builder_number)
> +    bq.markAsBuilding(builder)
> +
> +
> +class TestBuildJobBase(TestCaseWithFactory):
> +    """Setup the test publisher and some builders."""
> +
> +    layer = LaunchpadZopelessLayer
> +
> +    def setUp(self):
> +        super(TestBuildJobBase, self).setUp()
> +        self.publisher = SoyuzTestPublisher()
> +        self.publisher.prepareBreezyAutotest()
> +
> +        self.i8 = self.factory.makeBuilder(name='i386-n-8', virtualized=False)
> +        self.i9 = self.factory.makeBuilder(name='i386-n-9', virtualized=False)
> +
> +        processor_fam = ProcessorFamilySet().getByName('hppa')
> +        proc = processor_fam.processors[0]
> +        self.h6 = self.factory.makeBuilder(
> +            name='hppa-n-6', processor=proc, virtualized=False)
> +        self.h7 = self.factory.makeBuilder(
> +            name='hppa-n-7', processor=proc, virtualized=False)
> +
> +        self.builders = dict()
> +        # x86 native
> +        self.builders[(1,False)] = [self.i8, self.i9]
> +
> +        # hppa native
> +        self.builders[(3,True)] = [self.h6, self.h7]
> +
> +        # Ensure all builders are operational.
> +        for builders in self.builders.values():
> +            for builder in builders:
> +                builder.builderok = True
> +                builder.manual = False
> +
> +        # Disable the sample data builders.
> +        getUtility(IBuilderSet)['bob'].builderok = False
> +        getUtility(IBuilderSet)['frog'].builderok = False
> +
> +
> +class TestBuildPackageJob(TestBuildJobBase):
> +    """Test dispatch time estimates for binary builds (i.e. single build
> +    farm job type) targetting a single processor architecture and the primary
> +    archive.
> +    """
> +    def setUp(self):
> +        """Set up some native x86 builds for the test archive."""
> +        super(TestBuildPackageJob, self).setUp()
> +        # The builds will be set up as follows:
> +        #
> +        # j: 3        gedit p: hppa v:False e:0:01:00 *** s: 1001
> +        # j: 4        gedit p:  386 v:False e:0:02:00 *** s: 1002
> +        # j: 5      firefox p: hppa v:False e:0:03:00 *** s: 1003
> +        # j: 6      firefox p:  386 v:False e:0:04:00 *** s: 1004
> +        # j: 7     cobblers p: hppa v:False e:0:05:00 *** s: 1005
> +        # j: 8     cobblers p:  386 v:False e:0:06:00 *** s: 1006
> +        # j: 9 thunderpants p: hppa v:False e:0:07:00 *** s: 1007
> +        # j:10 thunderpants p:  386 v:False e:0:08:00 *** s: 1008
> +        # j:11          apg p: hppa v:False e:0:09:00 *** s: 1009
> +        # j:12          apg p:  386 v:False e:0:10:00 *** s: 1010
> +        # j:13          vim p: hppa v:False e:0:11:00 *** s: 1011
> +        # j:14          vim p:  386 v:False e:0:12:00 *** s: 1012
> +        # j:15          gcc p: hppa v:False e:0:13:00 *** s: 1013
> +        # j:16          gcc p:  386 v:False e:0:14:00 *** s: 1014
> +        # j:17        bison p: hppa v:False e:0:15:00 *** s: 1015
> +        # j:18        bison p:  386 v:False e:0:16:00 *** s: 1016
> +        # j:19         flex p: hppa v:False e:0:17:00 *** s: 1017
> +        # j:20         flex p:  386 v:False e:0:18:00 *** s: 1018
> +        # j:21     postgres p: hppa v:False e:0:19:00 *** s: 1019
> +        # j:22     postgres p:  386 v:False e:0:20:00 *** s: 1020
> +        #
> +        # j=job, p=processor, v=virtualized, e=estimated_duration, s=score
> +
> +        # First mark all builds in the sample data as already built.
> +        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
> +        sample_data = store.find(Build)
> +        for build in sample_data:
> +            build.buildstate = BuildStatus.FULLYBUILT
> +        store.flush()
> +
> +        # We test builds that target a primary archive.
> +        self.non_ppa = self.factory.makeArchive(
> +            name="primary", purpose=ArchivePurpose.PRIMARY)
> +        self.non_ppa.require_virtualized = False
> +
> +        self.builds = []
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
> +                archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="firefox", status=PackagePublishingStatus.PUBLISHED,
> +                archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="cobblers", status=PackagePublishingStatus.PUBLISHED,
> +                archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="thunderpants",
> +                status=PackagePublishingStatus.PUBLISHED, archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="apg", status=PackagePublishingStatus.PUBLISHED,
> +                archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="vim", status=PackagePublishingStatus.PUBLISHED,
> +                archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="gcc", status=PackagePublishingStatus.PUBLISHED,
> +                archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="bison", status=PackagePublishingStatus.PUBLISHED,
> +                archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="flex", status=PackagePublishingStatus.PUBLISHED,
> +                archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        self.builds.extend(
> +            self.publisher.getPubSource(
> +                sourcename="postgres",
> +                status=PackagePublishingStatus.PUBLISHED,
> +                archive=self.non_ppa,
> +                architecturehintlist='any').createMissingBuilds())
> +        # Set up the builds for test.

Given that we're in a test module about builds, that's only to be
expected. Maybe it's worth swapping this comment for one that explains
why we're setting the scores & durations in this way.

> +        score = 1000
> +        duration = 0
> +        for build in self.builds:
> +            score += 1
> +            duration += 60
> +            bq = build.buildqueue_record
> +            bq.lastscore = score
> +            bq.estimated_duration = timedelta(seconds=duration)
> +
> +    def test_x86_pending_queries(self):
> +        # Make sure the query returned by pendingJobsQuery() selects the
> +        # proper jobs.
> +        #

"Make sure" isn't needed in test comments and "proper" doesn't
communicate anything -- it's rare for a test to guarantee that we
return improper values.

More generally, surely there have to be very, very similar tests
already in Launchpad to test the build queue behaviour. Or am I
missing something?

And, as a side note, did you consider simply testing the returned
query itself rather than its results?

> +        # The x86 builds are as follows:
> +        #
> +        # j: 4        gedit p:  386 v:False e:0:02:00 *** s: 1002
> +        # j: 6      firefox p:  386 v:False e:0:04:00 *** s: 1004
> +        # j: 8     cobblers p:  386 v:False e:0:06:00 *** s: 1006
> +        # j:10 thunderpants p:  386 v:False e:0:08:00 *** s: 1008
> +        # j:12          apg p:  386 v:False e:0:10:00 *** s: 1010
> +        # j:14          vim p:  386 v:False e:0:12:00 *** s: 1012
> +        # j:16          gcc p:  386 v:False e:0:14:00 *** s: 1014
> +        # j:18        bison p:  386 v:False e:0:16:00 *** s: 1016
> +        # j:20         flex p:  386 v:False e:0:18:00 *** s: 1018
> +        # j:22     postgres p:  386 v:False e:0:20:00 *** s: 1020
> +        #
> +        # Please note: the higher scored jobs are lower in the list.
> +        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
> +
> +        build, bq = find_job(self, 'apg')
> +        bpj = bq.specific_job
> +        query = bpj.pendingJobsQuery(1010, *builder_key(build))
> +        result_set = store.execute(query).get_all()
> +        # The pending x86 jobs with score 1010 or higher are as follows.
> +        # Please note that we do not require the results to be in any
> +        # particular order.
> +
> +        # Processor == 1 -> Intel 386
> +        # SELECT id,name,title FROM processor
> +        #  id | name  |     title
> +        # ----+-------+----------------
> +        #   1 | 386   | Intel 386
> +        #   2 | amd64 | AMD 64bit
> +        #   3 | hppa  | HPPA Processor
> +
> +        expected_results = [
> +        #   job  score estimated_duration   processor   virtualized
> +            (12, 1010, timedelta(0, 600),       1,      False),
> +            (14, 1012, timedelta(0, 720),       1,      False),
> +            (16, 1014, timedelta(0, 840),       1,      False),
> +            (18, 1016, timedelta(0, 960),       1,      False),
> +            (20, 1018, timedelta(0, 1080),      1,      False),
> +            (22, 1020, timedelta(0, 1200),      1,      False)]
> +        self.assertEqual(sorted(result_set), sorted(expected_results))
> +        # How about builds with lower scores? Please note also that no
> +        # hppa builds are listed.
> +        query = bpj.pendingJobsQuery(0, *builder_key(build))
> +        result_set = store.execute(query).get_all()
> +        expected_results = [
> +        #   job  score estimated_duration   processor   virtualized
> +            (22, 1020, timedelta(0, 1200),      1,      False),
> +            (20, 1018, timedelta(0, 1080),      1,      False),
> +            (18, 1016, timedelta(0, 960),       1,      False),
> +            (16, 1014, timedelta(0, 840),       1,      False),
> +            (14, 1012, timedelta(0, 720),       1,      False),
> +            (12, 1010, timedelta(0, 600),       1,      False),
> +            (10, 1008, timedelta(0, 480),       1,      False),
> +            (8,  1006, timedelta(0, 360),       1,      False),
> +            (6,  1004, timedelta(0, 240),       1,      False),
> +            (4,  1002, timedelta(0, 120),       1,      False)]
> +        self.assertEqual(sorted(result_set), sorted(expected_results))
> +        # How about builds with higher scores?
> +        query = bpj.pendingJobsQuery(2500, *builder_key(build))
> +        result_set = store.execute(query).get_all()
> +        expected_results = []
> +        self.assertEqual(sorted(result_set), sorted(expected_results))
> +
> +        # We will start the 'flex' job now and see whether it still turns
> +        # up in our pending job list.
> +        assign_to_builder(self, 'flex', 1)
> +        query = bpj.pendingJobsQuery(1016, *builder_key(build))
> +        result_set = store.execute(query).get_all()
> +        expected_results = [
> +        #   job  score estimated_duration   processor   virtualized
> +            (22, 1020, timedelta(0, 1200),      1,      False),
> +            (18, 1016, timedelta(0, 960),       1,      False)]
> +        self.assertEqual(sorted(result_set), sorted(expected_results))
> +        # As we can see it was absent as expected.
> +
> +    def test_hppa_pending_queries(self):
> +        # Make sure the query returned by pendingJobsQuery() selects the
> +        # proper jobs.
> +        #
> +        # The hppa builds are as follows:
> +        #
> +        # j: 3        gedit p: hppa v:False e:0:01:00 *** s: 1001
> +        # j: 5      firefox p: hppa v:False e:0:03:00 *** s: 1003
> +        # j: 7     cobblers p: hppa v:False e:0:05:00 *** s: 1005
> +        # j: 9 thunderpants p: hppa v:False e:0:07:00 *** s: 1007
> +        # j:11          apg p: hppa v:False e:0:09:00 *** s: 1009
> +        # j:13          vim p: hppa v:False e:0:11:00 *** s: 1011
> +        # j:15          gcc p: hppa v:False e:0:13:00 *** s: 1013
> +        # j:17        bison p: hppa v:False e:0:15:00 *** s: 1015
> +        # j:19         flex p: hppa v:False e:0:17:00 *** s: 1017
> +        # j:21     postgres p: hppa v:False e:0:19:00 *** s: 1019
> +        #
> +        # Please note: the higher scored jobs are lower in the list.
> +        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
> +
> +        build, bq = find_job(self, 'vim', 'hppa')
> +        bpj = bq.specific_job
> +        query = bpj.pendingJobsQuery(1011, *builder_key(build))
> +        result_set = store.execute(query).get_all()
> +        # The pending hppa jobs with score 1011 or higher are as follows.
> +        # Please note that we do not require the results to be in any
> +        # particular order.
> +
> +        # Processor == 3 -> HPPA
> +        # SELECT id,name,title FROM processor
> +        #  id | name  |     title
> +        # ----+-------+----------------
> +        #   1 | 386   | Intel 386
> +        #   2 | amd64 | AMD 64bit
> +        #   3 | hppa  | HPPA Processor
> +
> +        expected_results = [
> +        #   job  score estimated_duration   processor   virtualized
> +            (13, 1011, timedelta(0, 660),       3,      False),
> +            (15, 1013, timedelta(0, 780),       3,      False),
> +            (17, 1015, timedelta(0, 900),       3,      False),
> +            (19, 1017, timedelta(0, 1020),      3,      False),
> +            (21, 1019, timedelta(0, 1140),      3,      False)]
> +        self.assertEqual(sorted(result_set), sorted(expected_results))
> +        # How about builds with lower scores? Please note also that no
> +        # hppa builds are listed.
> +        query = bpj.pendingJobsQuery(0, *builder_key(build))
> +        result_set = store.execute(query).get_all()
> +        expected_results = [
> +        #   job  score estimated_duration   processor   virtualized
> +            (3,  1001, timedelta(0, 60),        3,      False),
> +            (5,  1003, timedelta(0, 180),       3,      False),
> +            (7,  1005, timedelta(0, 300),       3,      False),
> +            (9,  1007, timedelta(0, 420),       3,      False),
> +            (11, 1009, timedelta(0, 540),       3,      False),
> +            (13, 1011, timedelta(0, 660),       3,      False),
> +            (15, 1013, timedelta(0, 780),       3,      False),
> +            (17, 1015, timedelta(0, 900),       3,      False),
> +            (19, 1017, timedelta(0, 1020),      3,      False),
> +            (21, 1019, timedelta(0, 1140),      3,      False)]
> +        self.assertEqual(sorted(result_set), sorted(expected_results))
> +        # How about builds with higher scores?
> +        query = bpj.pendingJobsQuery(2500, *builder_key(build))
> +        result_set = store.execute(query).get_all()
> +        expected_results = []
> +        self.assertEqual(sorted(result_set), sorted(expected_results))
> +
> +        # We will start the 'flex' job now and see whether it still turns
> +        # up in our pending job list.
> +        assign_to_builder(self, 'flex', 1, 'hppa')
> +        query = bpj.pendingJobsQuery(1014, *builder_key(build))
> +        result_set = store.execute(query).get_all()
> +        expected_results = [
> +        #   job  score estimated_duration   processor   virtualized
> +            (17, 1015, timedelta(0, 900),       3,      False),
> +            (21, 1019, timedelta(0, 1140),      3,      False)]
> +        self.assertEqual(sorted(result_set), sorted(expected_results))
> +        # As we can see it was absent as expected.
> +

Thanks! It's really exciting to see this general infrastructure taking shape.

jml

Revision history for this message

Jonathan Lange (jml) wrote on 2009-12-24:

As per my email, there are still some things to fix up.

review: Needs Fixing

Revision history for this message

Muharem Hrnjadovic (al-maisan) wrote on 2009-12-24:

Download full text (28.8 KiB)

Jonathan Lange wrote:
> On Thu, Dec 24, 2009 at 3:45 AM, Muharem Hrnjadovic
> <email address hidden> wrote:
>> Muharem Hrnjadovic has proposed merging lp:~al-maisan/launchpad/pending-jobs-499861 into lp:launchpad/devel.
>>
>> Requested reviews:
>> Canonical Launchpad Engineering (launchpad)
>> Related bugs:
>> #499861 Define interface allowing build farm job classes to select pending jobs
>> https://bugs.launchpad.net/bugs/499861
>>
>>
>> Hi there!
>>
>> this branch
>>
>> - introduces a method (IBuildFarmJob.pendingJobsQuery()) that all
>> Soyuz build farm jobs need to implement in order to enable a
>> general (i.e. across all job types) job dispatch time estimation.
>
> Although this sounds like a good idea to me, I'm confused by the
> terminology. Why do we ask a job (singular) to return a list of
> pending jobs (plural)? It seems like this method would be better
> placed on an interface representing a type of job.

Hmm .. good point .. can you point to an example?

The original idea was that these methods be implemented as static
methods. I can stress that point in the doc string if desired.

> ...
>> === modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
>> --- lib/lp/buildmaster/interfaces/buildfarmjob.py 2009-12-01 08:43:43 +0000
>> +++ lib/lp/buildmaster/interfaces/buildfarmjob.py 2009-12-23 16:45:30 +0000
>> @@ -12,8 +12,12 @@
>> 'BuildFarmJobType',
>> ]
>>
>> -from zope.interface import Interface
>> +from zope.interface import Interface, Attribute
>> +
>> +from canonical.launchpad import _
>> from lazr.enum import DBEnumeratedType, DBItem
>> +from lazr.restful.fields import Reference
>> +from lp.soyuz.interfaces.processor import IProcessor
>>
>>
>> class BuildFarmJobType(DBEnumeratedType):
>> @@ -69,3 +73,64 @@
>> def jobAborted():
>> """'Job aborted' life cycle event, handle as appropriate."""
>>
>> + def pendingJobsQuery(minscore, processor, virtualized):
>
> Given that this is a method, it's name should be in the imperative,
> e.g. getPendingJobsQuery.
done.

Jonathan Lange wrote:
> On Thu, Dec 24, 2009 at 3:45 AM, Muharem Hrnjadovic
> <muharem@canonical.com> wrote:
>> Muharem Hrnjadovic has proposed merging lp:~al-maisan/launchpad/pending-jobs-499861 into lp:launchpad/devel.
>>
>>    Requested reviews:
>>    Canonical Launchpad Engineering (launchpad)
>> Related bugs:
>>  #499861 Define interface allowing build farm job classes to select pending jobs
>>  https://bugs.launchpad.net/bugs/499861
>>
>>
>> Hi there!
>>
>> this branch
>>
>>    - introduces a method (IBuildFarmJob.pendingJobsQuery()) that all
>>      Soyuz build farm jobs need to implement in order to enable a
>>      general (i.e. across all job types) job dispatch time estimation.
> 
> Although this sounds like a good idea to me, I'm confused by the
> terminology. Why do we ask a job (singular) to return a list of
> pending jobs (plural)? It seems like this method would be better
> placed on an interface representing a type of job.

Hmm .. good point .. can you point to an example?

The original idea was that these methods be implemented as static
methods. I can stress that point in the doc string if desired.

> ...
>> === modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
>> --- lib/lp/buildmaster/interfaces/buildfarmjob.py       2009-12-01 08:43:43 +0000
>> +++ lib/lp/buildmaster/interfaces/buildfarmjob.py       2009-12-23 16:45:30 +0000
>> @@ -12,8 +12,12 @@
>>     'BuildFarmJobType',
>>     ]
>>
>> -from zope.interface import Interface
>> +from zope.interface import Interface, Attribute
>> +
>> +from canonical.launchpad import _
>>  from lazr.enum import DBEnumeratedType, DBItem
>> +from lazr.restful.fields import Reference
>> +from lp.soyuz.interfaces.processor import IProcessor
>>
>>
>>  class BuildFarmJobType(DBEnumeratedType):
>> @@ -69,3 +73,64 @@
>>     def jobAborted():
>>         """'Job aborted' life cycle event, handle as appropriate."""
>>
>> +    def pendingJobsQuery(minscore, processor, virtualized):
> 
> Given that this is a method, it's name should be in the imperative,
> e.g. getPendingJobsQuery.
done.

>> +        """String SELECT query yielding pending jobs with given minimum score.
>> +
>> +        This will be used for the purpose of job dispatch time estimation
>> +        for a build job of interest (JOI).
>> +        In order to estimate the dispatch time for the JOI we need to
>> +        calculate the sum of the estimated durations of the *pending* jobs
>> +        ahead of JOI.
>> +
>> +        Depending on the build farm job type the JOI may or may not be tied
>> +        to a particular processor type.
>> +        Binary builds for example are always built for a specific processor
>> +        whereas "create a source package from recipe" type jobs do not care
>> +        about processor types or virtualization.
>> +
>> +        When implementing this method for processor independent build farm job
>> +        types (e.g. recipe build) you may safely ignore the `processor` and
>> +        `virtualized` parameters.
>> +
>> +        The SELECT query to be returned needs to select the following data
>> +
>> +            1 - BuildQueue.job
>> +            2 - BuildQueue.lastscore
>> +            3 - BuildQueue.estimated_duration
>> +            4 - Processor.id    [optional]
>> +            5 - virtualized     [optional]
>> +
>> +        Please do *not* order the result set since it will be UNIONed and
>> +        ordered only then.
>> +
> 
> Thanks, this is a useful docstring.
:)

>> +        Job types that are processor independent or do not care about
>> +        virtualization should return NULL for the optional data in the result
>> +        set.
>> +
>> +        :param minscore: the pending jobs selected by the returned
>> +            query should have score >= minscore
> 
> This should be min_score or minimum_score.
replaced with 'min_score'.

>> +        :param processor: the job of interest (JOI) is tied to this
>> +            processor, this information can be used to further narrow
>> +            down the pending jobs that will result from the returned
>> +            query. Please note: processor independent job types may
>> +            safely ignore this parameter.
> 
> Does this mean, "the type of processor that the jobs are expected to
> run on"? If so, could you please say that?
Yeah, good point. Rephrased the description.

>> +        :param virtualized: the job of interest (JOI) can only run
>> +            on builders with this virtualization setting.
> 
> If I understand correctly, when this is passed in, it means that we're
> telling the job that we intend to run it with a particular
> virtualization setting. The way it's phrased in the docstring as-is
> seems a bit backwards, but I can't think of how to phrase it better.
> Can you?
I did try :) see how you like the new doc string.

>> +            Again, this information can be used to narrow down the
>> +            pending jobs that will result from the returned query and
>> +            processor independent job types may safely ignore it.
>> +        :return: a string SELECT clause that can be used to find
>> +            the pending jobs of the appropriate type.
> 
> Here's my big question: why a string?
> 
> Why not return a Storm ResultSet or a Select object?
Good question, I tried both a Storm ResultSet and a Select object (took
me a day at least).

Ultimately it did not work because

- once other job types are added to the build farm I will need to
    UNION their queries and then *order* the UNIONed result set
  - the job types that do not care about processor or virtualization
    will return None/NULL "values" for these columns in their result
    sets
  - neither the storm `Union` facility nor the ResultSet.union() method
    would support ordering of result sets that have *literal* values.

I did also ask on the #storm IRC channel but did not get any answers
that seemed to fit my problem.

>> +
>> +    processor = Reference(
>> +        IProcessor, title=_("Processor"),
>> +        description=_(
>> +            "The Processor required by this build farm job. "
>> +            "For processor independent job types please return None."))
>> +
> 
> "processor-independent" should have a hyphen.
Fixed.

>> +    virtualized = Attribute(
>> +        _(
>> +            "The virtualization setting required by this build farm job. "
>> +            "For job types that do not care about virtualization please "
>> +            "return None."))
>>
> 
> You don't mention either of these new attributes in your cover letter
> -- what's their purpose?
When interrogated each build farm job shall be able to tell what its
processor/virtualization requirements are.

"Build a source package from recipe" type jobs would e.g. return None
for both thus indicating that they (do not care about processor or
virtualization and) may run on any builder.

> ...
>> === modified file 'lib/lp/soyuz/model/buildpackagejob.py'
>> --- lib/lp/soyuz/model/buildpackagejob.py       2009-11-13 20:39:36 +0000
>> +++ lib/lp/soyuz/model/buildpackagejob.py       2009-12-23 16:45:30 +0000
>> @@ -13,9 +13,13 @@
>>  from zope.interface import implements
>>
>>  from canonical.database.constants import UTC_NOW
>> -from canonical.launchpad.interfaces import SourcePackageUrgency
>> -from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
>> +from canonical.database.sqlbase import sqlvalues
>> +
>> +from lp.buildmaster.interfaces.buildfarmjob import (
>> +    BuildFarmJobType, IBuildFarmJob)
>> +from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
>>  from lp.registry.interfaces.pocket import PackagePublishingPocket
>> +from lp.services.job.interfaces.job import JobStatus
>>  from lp.soyuz.interfaces.archive import ArchivePurpose
>>  from lp.soyuz.interfaces.build import BuildStatus
>>  from lp.soyuz.interfaces.buildpackagejob import IBuildPackageJob
>> @@ -166,3 +170,41 @@
>>         # fix it.
>>         self.build.buildstate = BuildStatus.BUILDING
>>
>> +    @staticmethod
>> +    def pendingJobsQuery(minscore, processor, virtualized):
>> +        """See `IBuildFarmJob`."""
>> +        return """
>> +            SELECT
>> +                BuildQueue.job,
>> +                BuildQueue.lastscore,
>> +                BuildQueue.estimated_duration,
>> +                Build.processor AS processor,
>> +                Archive.require_virtualized AS virtualized
>> +            FROM
>> +                BuildQueue, Build, BuildPackageJob, Archive, Job
>> +            WHERE
>> +                BuildQueue.job_type = %s
>> +                AND BuildPackageJob.job = BuildQueue.job
>> +                AND BuildPackageJob.job = Job.id
>> +                AND Job.status = %s
>> +                AND BuildPackageJob.build = Build.id
>> +                AND Build.buildstate = %s
>> +                AND Build.archive = Archive.id
>> +                AND Archive.enabled = TRUE
>> +                AND BuildQueue.lastscore >= %s
>> +                AND Build.processor = %s
>> +                AND Archive.require_virtualized = %s
>> +        """ % sqlvalues(
>> +            BuildFarmJobType.PACKAGEBUILD, JobStatus.WAITING,
>> +            BuildStatus.NEEDSBUILD, minscore, processor, virtualized)
>> +
>> +    @property
>> +    def processor(self):
>> +        """See `IBuildFarmJob`."""
>> +        return self.build.processor
>> +
>> +    @property
>> +    def virtualized(self):
>> +        """See `IBuildFarmJob`."""
>> +        return self.build.is_virtualized
>> +
>>
> 
> 
> 
>> === added file 'lib/lp/soyuz/tests/test_buildpackagejob.py'
>> --- lib/lp/soyuz/tests/test_buildpackagejob.py  1970-01-01 00:00:00 +0000
>> +++ lib/lp/soyuz/tests/test_buildpackagejob.py  2009-12-23 16:45:30 +0000
>> @@ -0,0 +1,377 @@
>> +# Copyright 2009 Canonical Ltd.  This software is licensed under the
>> +# GNU Affero General Public License version 3 (see the file LICENSE).
>> +
>> +"""Test BuildQueue features."""
>> +
>> +import unittest
>> +
>> +from datetime import datetime, timedelta
>> +from pytz import utc
>> +
>> +from zope.component import getUtility
>> +
>> +from canonical.launchpad.webapp.interfaces import (
>> +    IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
>> +from canonical.testing import LaunchpadZopelessLayer
>> +
>> +from lp.soyuz.interfaces.archive import ArchivePurpose
>> +from lp.soyuz.interfaces.build import BuildStatus, IBuildSet
>> +from lp.soyuz.interfaces.builder import IBuilderSet
>> +from lp.soyuz.model.processor import ProcessorFamilySet
>> +from lp.soyuz.interfaces.publishing import PackagePublishingStatus
>> +from lp.soyuz.model.build import Build
>> +from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
>> +from lp.testing import TestCaseWithFactory
>> +
>> +
>> +def find_job(test, name, processor='386'):
> 
> Docstring please.
Will add one.

>> +    result = None
>> +    for build in test.builds:
>> +        if (build.sourcepackagerelease.name == name
>> +            and build.processor.name == processor):
>> +            result = build
> 
> Were it me, I'd return (build, build.buildqueue_record) here, forget
> the break and the 'result' variable -- it's pretty much the standard
> way of spelling "linear search" in Python.
OK .. will revise the code accordingly.

>> +            break
>> +    if result is not None:
>> +        return (result, result.buildqueue_record)
>> +    else:
>> +        return (None, None)
>> +
>> +def builder_key(build):
>> +    return (build.processor.id,build.is_virtualized)
>> +
> 
> Space after the comma please. Also, two blank lines between top-level
> function definitions.
Will fix that.

> Also, you might consider defining this method within 'nth_builder'.
Good idea. Will do.

>> +def nth_builder(test, build, n):
> 
> Docstring please.
Will fix that.

>> +    builder = None
>> +    builders = test.builders.get(builder_key(build), [])
>> +    try:
>> +        builder = builders[n-1]
>> +    except IndexError:
>> +        pass
>> +    return builder
>> +
> 
> Second blank line please.
Will fix that.

>> +def assign_to_builder(test, job_name, builder_number, processor='386'):
> 
> Docstring please.
Will fix that.

>> +    build, bq = find_job(test, job_name, processor)
>> +    builder = nth_builder(test, build, builder_number)
>> +    bq.markAsBuilding(builder)
>> +
>> +
>> +class TestBuildJobBase(TestCaseWithFactory):
>> +    """Setup the test publisher and some builders."""
>> +
>> +    layer = LaunchpadZopelessLayer
>> +
>> +    def setUp(self):
>> +        super(TestBuildJobBase, self).setUp()
>> +        self.publisher = SoyuzTestPublisher()
>> +        self.publisher.prepareBreezyAutotest()
>> +
>> +        self.i8 = self.factory.makeBuilder(name='i386-n-8', virtualized=False)
>> +        self.i9 = self.factory.makeBuilder(name='i386-n-9', virtualized=False)
>> +
>> +        processor_fam = ProcessorFamilySet().getByName('hppa')
>> +        proc = processor_fam.processors[0]
>> +        self.h6 = self.factory.makeBuilder(
>> +            name='hppa-n-6', processor=proc, virtualized=False)
>> +        self.h7 = self.factory.makeBuilder(
>> +            name='hppa-n-7', processor=proc, virtualized=False)
>> +
>> +        self.builders = dict()
>> +        # x86 native
>> +        self.builders[(1,False)] = [self.i8, self.i9]
>> +
>> +        # hppa native
>> +        self.builders[(3,True)] = [self.h6, self.h7]
>> +
>> +        # Ensure all builders are operational.
>> +        for builders in self.builders.values():
>> +            for builder in builders:
>> +                builder.builderok = True
>> +                builder.manual = False
>> +
>> +        # Disable the sample data builders.
>> +        getUtility(IBuilderSet)['bob'].builderok = False
>> +        getUtility(IBuilderSet)['frog'].builderok = False
>> +
>> +
>> +class TestBuildPackageJob(TestBuildJobBase):
>> +    """Test dispatch time estimates for binary builds (i.e. single build
>> +    farm job type) targetting a single processor architecture and the primary
>> +    archive.
>> +    """
>> +    def setUp(self):
>> +        """Set up some native x86 builds for the test archive."""
>> +        super(TestBuildPackageJob, self).setUp()
>> +        # The builds will be set up as follows:
>> +        #
>> +        # j: 3        gedit p: hppa v:False e:0:01:00 *** s: 1001
>> +        # j: 4        gedit p:  386 v:False e:0:02:00 *** s: 1002
>> +        # j: 5      firefox p: hppa v:False e:0:03:00 *** s: 1003
>> +        # j: 6      firefox p:  386 v:False e:0:04:00 *** s: 1004
>> +        # j: 7     cobblers p: hppa v:False e:0:05:00 *** s: 1005
>> +        # j: 8     cobblers p:  386 v:False e:0:06:00 *** s: 1006
>> +        # j: 9 thunderpants p: hppa v:False e:0:07:00 *** s: 1007
>> +        # j:10 thunderpants p:  386 v:False e:0:08:00 *** s: 1008
>> +        # j:11          apg p: hppa v:False e:0:09:00 *** s: 1009
>> +        # j:12          apg p:  386 v:False e:0:10:00 *** s: 1010
>> +        # j:13          vim p: hppa v:False e:0:11:00 *** s: 1011
>> +        # j:14          vim p:  386 v:False e:0:12:00 *** s: 1012
>> +        # j:15          gcc p: hppa v:False e:0:13:00 *** s: 1013
>> +        # j:16          gcc p:  386 v:False e:0:14:00 *** s: 1014
>> +        # j:17        bison p: hppa v:False e:0:15:00 *** s: 1015
>> +        # j:18        bison p:  386 v:False e:0:16:00 *** s: 1016
>> +        # j:19         flex p: hppa v:False e:0:17:00 *** s: 1017
>> +        # j:20         flex p:  386 v:False e:0:18:00 *** s: 1018
>> +        # j:21     postgres p: hppa v:False e:0:19:00 *** s: 1019
>> +        # j:22     postgres p:  386 v:False e:0:20:00 *** s: 1020
>> +        #
>> +        # j=job, p=processor, v=virtualized, e=estimated_duration, s=score
>> +
>> +        # First mark all builds in the sample data as already built.
>> +        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
>> +        sample_data = store.find(Build)
>> +        for build in sample_data:
>> +            build.buildstate = BuildStatus.FULLYBUILT
>> +        store.flush()
>> +
>> +        # We test builds that target a primary archive.
>> +        self.non_ppa = self.factory.makeArchive(
>> +            name="primary", purpose=ArchivePurpose.PRIMARY)
>> +        self.non_ppa.require_virtualized = False
>> +
>> +        self.builds = []
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
>> +                archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="firefox", status=PackagePublishingStatus.PUBLISHED,
>> +                archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="cobblers", status=PackagePublishingStatus.PUBLISHED,
>> +                archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="thunderpants",
>> +                status=PackagePublishingStatus.PUBLISHED, archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="apg", status=PackagePublishingStatus.PUBLISHED,
>> +                archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="vim", status=PackagePublishingStatus.PUBLISHED,
>> +                archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="gcc", status=PackagePublishingStatus.PUBLISHED,
>> +                archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="bison", status=PackagePublishingStatus.PUBLISHED,
>> +                archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="flex", status=PackagePublishingStatus.PUBLISHED,
>> +                archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        self.builds.extend(
>> +            self.publisher.getPubSource(
>> +                sourcename="postgres",
>> +                status=PackagePublishingStatus.PUBLISHED,
>> +                archive=self.non_ppa,
>> +                architecturehintlist='any').createMissingBuilds())
>> +        # Set up the builds for test.
> 
> Given that we're in a test module about builds, that's only to be
> expected. Maybe it's worth swapping this comment for one that explains
> why we're setting the scores & durations in this way.
Basically to have diverse data that exercises the returned queries.
Will fix that.

>> +        score = 1000
>> +        duration = 0
>> +        for build in self.builds:
>> +            score += 1
>> +            duration += 60
>> +            bq = build.buildqueue_record
>> +            bq.lastscore = score
>> +            bq.estimated_duration = timedelta(seconds=duration)
>> +
>> +    def test_x86_pending_queries(self):
>> +        # Make sure the query returned by pendingJobsQuery() selects the
>> +        # proper jobs.
>> +        #
> 
> "Make sure" isn't needed in test comments and "proper" doesn't
> communicate anything -- it's rare for a test to guarantee that we
> return improper values.
Good point.
Will fix that.

> More generally, surely there have to be very, very similar tests
> already in Launchpad to test the build queue behaviour. Or am I
> missing something?
These existing build dispatch time estimation tests barely scratch the
surface unfortunately.

> And, as a side note, did you consider simply testing the returned
> query itself rather than its results?
Yeah .. but seeing that the queries behave correctly with reasonably
reperesentative test data is much better IMHO.

>> +        # The x86 builds are as follows:
>> +        #
>> +        # j: 4        gedit p:  386 v:False e:0:02:00 *** s: 1002
>> +        # j: 6      firefox p:  386 v:False e:0:04:00 *** s: 1004
>> +        # j: 8     cobblers p:  386 v:False e:0:06:00 *** s: 1006
>> +        # j:10 thunderpants p:  386 v:False e:0:08:00 *** s: 1008
>> +        # j:12          apg p:  386 v:False e:0:10:00 *** s: 1010
>> +        # j:14          vim p:  386 v:False e:0:12:00 *** s: 1012
>> +        # j:16          gcc p:  386 v:False e:0:14:00 *** s: 1014
>> +        # j:18        bison p:  386 v:False e:0:16:00 *** s: 1016
>> +        # j:20         flex p:  386 v:False e:0:18:00 *** s: 1018
>> +        # j:22     postgres p:  386 v:False e:0:20:00 *** s: 1020
>> +        #
>> +        # Please note: the higher scored jobs are lower in the list.
>> +        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
>> +
>> +        build, bq = find_job(self, 'apg')
>> +        bpj = bq.specific_job
>> +        query = bpj.pendingJobsQuery(1010, *builder_key(build))
>> +        result_set = store.execute(query).get_all()
>> +        # The pending x86 jobs with score 1010 or higher are as follows.
>> +        # Please note that we do not require the results to be in any
>> +        # particular order.
>> +
>> +        # Processor == 1 -> Intel 386
>> +        # SELECT id,name,title FROM processor
>> +        #  id | name  |     title
>> +        # ----+-------+----------------
>> +        #   1 | 386   | Intel 386
>> +        #   2 | amd64 | AMD 64bit
>> +        #   3 | hppa  | HPPA Processor
>> +
>> +        expected_results = [
>> +        #   job  score estimated_duration   processor   virtualized
>> +            (12, 1010, timedelta(0, 600),       1,      False),
>> +            (14, 1012, timedelta(0, 720),       1,      False),
>> +            (16, 1014, timedelta(0, 840),       1,      False),
>> +            (18, 1016, timedelta(0, 960),       1,      False),
>> +            (20, 1018, timedelta(0, 1080),      1,      False),
>> +            (22, 1020, timedelta(0, 1200),      1,      False)]
>> +        self.assertEqual(sorted(result_set), sorted(expected_results))
>> +        # How about builds with lower scores? Please note also that no
>> +        # hppa builds are listed.
>> +        query = bpj.pendingJobsQuery(0, *builder_key(build))
>> +        result_set = store.execute(query).get_all()
>> +        expected_results = [
>> +        #   job  score estimated_duration   processor   virtualized
>> +            (22, 1020, timedelta(0, 1200),      1,      False),
>> +            (20, 1018, timedelta(0, 1080),      1,      False),
>> +            (18, 1016, timedelta(0, 960),       1,      False),
>> +            (16, 1014, timedelta(0, 840),       1,      False),
>> +            (14, 1012, timedelta(0, 720),       1,      False),
>> +            (12, 1010, timedelta(0, 600),       1,      False),
>> +            (10, 1008, timedelta(0, 480),       1,      False),
>> +            (8,  1006, timedelta(0, 360),       1,      False),
>> +            (6,  1004, timedelta(0, 240),       1,      False),
>> +            (4,  1002, timedelta(0, 120),       1,      False)]
>> +        self.assertEqual(sorted(result_set), sorted(expected_results))
>> +        # How about builds with higher scores?
>> +        query = bpj.pendingJobsQuery(2500, *builder_key(build))
>> +        result_set = store.execute(query).get_all()
>> +        expected_results = []
>> +        self.assertEqual(sorted(result_set), sorted(expected_results))
>> +
>> +        # We will start the 'flex' job now and see whether it still turns
>> +        # up in our pending job list.
>> +        assign_to_builder(self, 'flex', 1)
>> +        query = bpj.pendingJobsQuery(1016, *builder_key(build))
>> +        result_set = store.execute(query).get_all()
>> +        expected_results = [
>> +        #   job  score estimated_duration   processor   virtualized
>> +            (22, 1020, timedelta(0, 1200),      1,      False),
>> +            (18, 1016, timedelta(0, 960),       1,      False)]
>> +        self.assertEqual(sorted(result_set), sorted(expected_results))
>> +        # As we can see it was absent as expected.
>> +
>> +    def test_hppa_pending_queries(self):
>> +        # Make sure the query returned by pendingJobsQuery() selects the
>> +        # proper jobs.
>> +        #
>> +        # The hppa builds are as follows:
>> +        #
>> +        # j: 3        gedit p: hppa v:False e:0:01:00 *** s: 1001
>> +        # j: 5      firefox p: hppa v:False e:0:03:00 *** s: 1003
>> +        # j: 7     cobblers p: hppa v:False e:0:05:00 *** s: 1005
>> +        # j: 9 thunderpants p: hppa v:False e:0:07:00 *** s: 1007
>> +        # j:11          apg p: hppa v:False e:0:09:00 *** s: 1009
>> +        # j:13          vim p: hppa v:False e:0:11:00 *** s: 1011
>> +        # j:15          gcc p: hppa v:False e:0:13:00 *** s: 1013
>> +        # j:17        bison p: hppa v:False e:0:15:00 *** s: 1015
>> +        # j:19         flex p: hppa v:False e:0:17:00 *** s: 1017
>> +        # j:21     postgres p: hppa v:False e:0:19:00 *** s: 1019
>> +        #
>> +        # Please note: the higher scored jobs are lower in the list.
>> +        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
>> +
>> +        build, bq = find_job(self, 'vim', 'hppa')
>> +        bpj = bq.specific_job
>> +        query = bpj.pendingJobsQuery(1011, *builder_key(build))
>> +        result_set = store.execute(query).get_all()
>> +        # The pending hppa jobs with score 1011 or higher are as follows.
>> +        # Please note that we do not require the results to be in any
>> +        # particular order.
>> +
>> +        # Processor == 3 -> HPPA
>> +        # SELECT id,name,title FROM processor
>> +        #  id | name  |     title
>> +        # ----+-------+----------------
>> +        #   1 | 386   | Intel 386
>> +        #   2 | amd64 | AMD 64bit
>> +        #   3 | hppa  | HPPA Processor
>> +
>> +        expected_results = [
>> +        #   job  score estimated_duration   processor   virtualized
>> +            (13, 1011, timedelta(0, 660),       3,      False),
>> +            (15, 1013, timedelta(0, 780),       3,      False),
>> +            (17, 1015, timedelta(0, 900),       3,      False),
>> +            (19, 1017, timedelta(0, 1020),      3,      False),
>> +            (21, 1019, timedelta(0, 1140),      3,      False)]
>> +        self.assertEqual(sorted(result_set), sorted(expected_results))
>> +        # How about builds with lower scores? Please note also that no
>> +        # hppa builds are listed.
>> +        query = bpj.pendingJobsQuery(0, *builder_key(build))
>> +        result_set = store.execute(query).get_all()
>> +        expected_results = [
>> +        #   job  score estimated_duration   processor   virtualized
>> +            (3,  1001, timedelta(0, 60),        3,      False),
>> +            (5,  1003, timedelta(0, 180),       3,      False),
>> +            (7,  1005, timedelta(0, 300),       3,      False),
>> +            (9,  1007, timedelta(0, 420),       3,      False),
>> +            (11, 1009, timedelta(0, 540),       3,      False),
>> +            (13, 1011, timedelta(0, 660),       3,      False),
>> +            (15, 1013, timedelta(0, 780),       3,      False),
>> +            (17, 1015, timedelta(0, 900),       3,      False),
>> +            (19, 1017, timedelta(0, 1020),      3,      False),
>> +            (21, 1019, timedelta(0, 1140),      3,      False)]
>> +        self.assertEqual(sorted(result_set), sorted(expected_results))
>> +        # How about builds with higher scores?
>> +        query = bpj.pendingJobsQuery(2500, *builder_key(build))
>> +        result_set = store.execute(query).get_all()
>> +        expected_results = []
>> +        self.assertEqual(sorted(result_set), sorted(expected_results))
>> +
>> +        # We will start the 'flex' job now and see whether it still turns
>> +        # up in our pending job list.
>> +        assign_to_builder(self, 'flex', 1, 'hppa')
>> +        query = bpj.pendingJobsQuery(1014, *builder_key(build))
>> +        result_set = store.execute(query).get_all()
>> +        expected_results = [
>> +        #   job  score estimated_duration   processor   virtualized
>> +            (17, 1015, timedelta(0, 900),       3,      False),
>> +            (21, 1019, timedelta(0, 1140),      3,      False)]
>> +        self.assertEqual(sorted(result_set), sorted(expected_results))
>> +        # As we can see it was absent as expected.
>> +
> 
> Thanks! It's really exciting to see this general infrastructure taking shape.
Thanks for the last-minute review!

Best regards

-- 
Muharem Hrnjadovic <muharem@ubuntu.com>
Public key id   : B2BBFCFC
Key fingerprint : A5A3 CC67 2B87 D641 103F  5602 219F 6B60 B2BB FCFC

Revision history for this message

Jonathan Lange (jml) wrote on 2009-12-24:

Download full text (4.3 KiB)

On Thu, Dec 24, 2009 at 8:16 PM, Muharem Hrnjadovic <email address hidden> wrote:
> Jonathan Lange wrote:
>> On Thu, Dec 24, 2009 at 3:45 AM, Muharem Hrnjadovic
>> <email address hidden> wrote:
>>> Muharem Hrnjadovic has proposed merging lp:~al-maisan/launchpad/pending-jobs-499861 into lp:launchpad/devel.
>>>
>>> Requested reviews:
>>> Canonical Launchpad Engineering (launchpad)
>>> Related bugs:
>>> #499861 Define interface allowing build farm job classes to select pending jobs
>>> https://bugs.launchpad.net/bugs/499861
>>>
>>>
>>> Hi there!
>>>
>>> this branch
>>>
>>> - introduces a method (IBuildFarmJob.pendingJobsQuery()) that all
>>> Soyuz build farm jobs need to implement in order to enable a
>>> general (i.e. across all job types) job dispatch time estimation.
>>
>> Although this sounds like a good idea to me, I'm confused by the
>> terminology. Why do we ask a job (singular) to return a list of
>> pending jobs (plural)? It seems like this method would be better
>> placed on an interface representing a type of job.
>
> Hmm .. good point .. can you point to an example?
>
> The original idea was that these methods be implemented as static
> methods. I can stress that point in the doc string if desired.
>

I don't know examples off the top of my head, but I reckon if you grep
for classProvides, you'll find some fairly quickly. The key point is
that static methods are a different interface.

>>> + :param virtualized: the job of interest (JOI) can only run
>>> + on builders with this virtualization setting.
>>
>> If I understand correctly, when this is passed in, it means that we're
>> telling the job that we intend to run it with a particular
>> virtualization setting. The way it's phrased in the docstring as-is
>> seems a bit backwards, but I can't think of how to phrase it better.
>> Can you?
> I did try :) see how you like the new doc string.
>

It's great, thanks.

>>> + Again, this information can be used to narrow down the
>>> + pending jobs that will result from the returned query and
>>> + processor independent job types may safely ignore it.
>>> + :return: a string SELECT clause that can be used to find
>>> + the pending jobs of the appropriate type.
>>
>> Here's my big question: why a string?
>>
>> Why not return a Storm ResultSet or a Select object?
> Good question, I tried both a Storm ResultSet and a Select object (took
> me a day at least).
>
> Ultimately it did not work because
>
> - once other job types are added to the build farm I will need to
> UNION their queries and then *order* the UNIONed result set
> - the job types that do not care about processor or virtualization
> will return None/NULL "values" for these columns in their result
> sets
> - neither the storm `Union` facility nor the ResultSet.union() method
> would support ordering of result sets that have *literal* values.
>
> I did also ask on the #storm IRC channel but did not get any answers
> that seemed to fit my problem.
>

Hmm. In that case, keep what you've got.

It might be worth asking on the storm mailing list for interests sake,
or future work.

>>...

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Barki Mustapha

Celso Providelo

Christian Reis

Christy Awad

Colin Watson

Harpianto,ANDI

James Troup

John A Meinel

Kevin bush

Launchpad code reviewers

Launchpad code reviewers from Canonical

Matthew Tanner

Maximiliano Bertacchini

Muharem Hrnjadovic

Oguz Ersoz

Simon Brakhane

Ubuntu-BR DevOps

William Grant

alhawiti

api.ng

pedro cavazos

todaioan

wenjingwen

to status/vote changes:

Tzaddi

Tzaddi Belding

Launchpad itself

Merge lp:~al-maisan/launchpad/pending-jobs-499861 into lp:launchpad

Commit message

Description of the change

Preview Diff

Subscribers

 === modified file 'lib/lp/buildmaster/interfaces/buildfarmjob.py'
 --- lib/lp/buildmaster/interfaces/buildfarmjob.py	2009-12-01 08:43:43 +0000
 +++ lib/lp/buildmaster/interfaces/buildfarmjob.py	2009-12-24 15:02:13 +0000
@@ -9,11 +9,16 @@
  __all__ = [
      'IBuildFarmJob',
++    'IBuildFarmJobDispatchEstimation',
      'BuildFarmJobType',
+     ]
--from zope.interface import Interface
++from zope.interface import Interface, Attribute
++
++from canonical.launchpad import _
  from lazr.enum import DBEnumeratedType, DBItem
++from lazr.restful.fields import Reference
++from lp.soyuz.interfaces.processor import IProcessor
  class BuildFarmJobType(DBEnumeratedType):
@@ -69,3 +74,63 @@
      def jobAborted():
          """'Job aborted' life cycle event, handle as appropriate."""
++    processor = Reference(
++        IProcessor, title=_("Processor"),
++        description=_(
++            "The Processor required by this build farm job. "
++            "For processor-independent job types please return None."))
++
++    virtualized = Attribute(
++        _(
++            "The virtualization setting required by this build farm job. "
++            "For job types that do not care about virtualization please "
++            "return None."))
++
++
++class IBuildFarmJobDispatchEstimation(Interface):
++    """Operations needed for job dipatch time estimation."""
++
++    def composePendingJobsQuery(min_score, processor, virtualized):
++        """String SELECT query yielding pending jobs with given minimum score.
++
++        This will be used for the purpose of job dispatch time estimation
++        for a build job of interest (JOI).
++        In order to estimate the dispatch time for the JOI we need to
++        calculate the sum of the estimated durations of the *pending* jobs
++        ahead of JOI.
++
++        Depending on the build farm job type the JOI may or may not be tied
++        to a particular processor type.
++        Binary builds for example are always built for a specific processor
++        whereas "create a source package from recipe" type jobs do not care
++        about processor types or virtualization.
++
++        When implementing this method for processor independent build farm job
++        types (e.g. recipe build) you may safely ignore the `processor` and
++        `virtualized` parameters.
++
++        The SELECT query to be returned needs to select the following data
++
++            1 - BuildQueue.job
++            2 - BuildQueue.lastscore
++            3 - BuildQueue.estimated_duration
++            4 - Processor.id    [optional]
++            5 - virtualized     [optional]
++
++        Please do *not* order the result set since it will be UNIONed and
++        ordered only then.
++
++        Job types that are processor independent or do not care about
++        virtualization should return NULL for the optional data in the result
++        set.
++
++        :param min_score: the pending jobs selected by the returned
++            query should have score >= min_score.
++        :param processor: the type of processor that the jobs are expected
++            to run on.
++        :param virtualized: whether the jobs are expected to run on the
++            `processor` natively or inside a virtual machine.
++        :return: a string SELECT clause that can be used to find
++            the pending jobs of the appropriate type.
++        """
++
 === modified file 'lib/lp/buildmaster/model/buildfarmjob.py'
 --- lib/lp/buildmaster/model/buildfarmjob.py	2009-11-13 20:39:36 +0000
 +++ lib/lp/buildmaster/model/buildfarmjob.py	2009-12-24 15:02:13 +0000
@@ -38,3 +38,13 @@
          """See `IBuildFarmJob`."""
          pass
++    @property
++    def processor(self):
++        """See `IBuildFarmJob`."""
++        return None
++
++    @property
++    def virtualized(self):
++        """See `IBuildFarmJob`."""
++        return None
++
 === modified file 'lib/lp/soyuz/model/buildpackagejob.py'
 --- lib/lp/soyuz/model/buildpackagejob.py	2009-11-13 20:39:36 +0000
 +++ lib/lp/soyuz/model/buildpackagejob.py	2009-12-24 15:02:13 +0000
@@ -10,12 +10,16 @@
  from storm.locals import Int, Reference, Storm
--from zope.interface import implements
++from zope.interface import classProvides, implements
  from canonical.database.constants import UTC_NOW
--from canonical.launchpad.interfaces import SourcePackageUrgency
--from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
++from canonical.database.sqlbase import sqlvalues
++
++from lp.buildmaster.interfaces.buildfarmjob import (
++    BuildFarmJobType, IBuildFarmJob, IBuildFarmJobDispatchEstimation)
++from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
  from lp.registry.interfaces.pocket import PackagePublishingPocket
++from lp.services.job.interfaces.job import JobStatus
  from lp.soyuz.interfaces.archive import ArchivePurpose
  from lp.soyuz.interfaces.build import BuildStatus
  from lp.soyuz.interfaces.buildpackagejob import IBuildPackageJob
@@ -24,6 +28,8 @@
  class BuildPackageJob(Storm):
      """See `IBuildPackageJob`."""
      implements(IBuildFarmJob, IBuildPackageJob)
++    classProvides(IBuildFarmJobDispatchEstimation)
++
      __storm_table__ = 'buildpackagejob'
      id = Int(primary=True)
@@ -166,3 +172,41 @@
          # fix it.
          self.build.buildstate = BuildStatus.BUILDING
++    @staticmethod
++    def composePendingJobsQuery(min_score, processor, virtualized):
++        """See `IBuildFarmJob`."""
++        return """
++            SELECT
++                BuildQueue.job,
++                BuildQueue.lastscore,
++                BuildQueue.estimated_duration,
++                Build.processor AS processor,
++                Archive.require_virtualized AS virtualized
++            FROM
++                BuildQueue, Build, BuildPackageJob, Archive, Job
++            WHERE
++                BuildQueue.job_type = %s
++                AND BuildPackageJob.job = BuildQueue.job
++                AND BuildPackageJob.job = Job.id
++                AND Job.status = %s
++                AND BuildPackageJob.build = Build.id
++                AND Build.buildstate = %s
++                AND Build.archive = Archive.id
++                AND Archive.enabled = TRUE
++                AND BuildQueue.lastscore >= %s
++                AND Build.processor = %s
++                AND Archive.require_virtualized = %s
++        """ % sqlvalues(
++            BuildFarmJobType.PACKAGEBUILD, JobStatus.WAITING,
++            BuildStatus.NEEDSBUILD, min_score, processor, virtualized)
++
++    @property
++    def processor(self):
++        """See `IBuildFarmJob`."""
++        return self.build.processor
++
++    @property
++    def virtualized(self):
++        """See `IBuildFarmJob`."""
++        return self.build.is_virtualized
++
 === added file 'lib/lp/soyuz/tests/test_buildpackagejob.py'
 --- lib/lp/soyuz/tests/test_buildpackagejob.py	1970-01-01 00:00:00 +0000
 +++ lib/lp/soyuz/tests/test_buildpackagejob.py	2009-12-24 15:02:13 +0000
@@ -0,0 +1,384 @@
++# Copyright 2009 Canonical Ltd.  This software is licensed under the
++# GNU Affero General Public License version 3 (see the file LICENSE).
++
++"""Test BuildQueue features."""
++
++from datetime import timedelta
++
++from zope.component import getUtility
++
++from canonical.launchpad.webapp.interfaces import (
++    IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR)
++from canonical.launchpad.webapp.testing import verifyObject
++from canonical.testing import LaunchpadZopelessLayer
++
++from lp.buildmaster.interfaces.buildfarmjob import (
++    IBuildFarmJobDispatchEstimation)
++from lp.soyuz.interfaces.archive import ArchivePurpose
++from lp.soyuz.interfaces.build import BuildStatus
++from lp.soyuz.interfaces.builder import IBuilderSet
++from lp.soyuz.interfaces.publishing import PackagePublishingStatus
++from lp.soyuz.model.build import Build
++from lp.soyuz.model.buildpackagejob import BuildPackageJob
++from lp.soyuz.model.processor import ProcessorFamilySet
++from lp.soyuz.tests.test_publishing import SoyuzTestPublisher
++from lp.testing import TestCaseWithFactory
++
++def find_job(test, name, processor='386'):
++    """Find build and queue instance for the given source and processor."""
++    for build in test.builds:
++        if (build.sourcepackagerelease.name == name
++            and build.processor.name == processor):
++            return (build, build.buildqueue_record)
++    return (None, None)
++
++
++def builder_key(build):
++    """Return processor and virtualization for the given build."""
++    return (build.processor.id, build.is_virtualized)
++
++
++def assign_to_builder(test, job_name, builder_number, processor='386'):
++    """Simulate assigning a build to a builder."""
++    def nth_builder(test, build, n):
++        """Get builder #n for the given build processor and virtualization."""
++        builder = None
++        builders = test.builders.get(builder_key(build), [])
++        try:
++            builder = builders[n-1]
++        except IndexError:
++            pass
++        return builder
++
++    build, bq = find_job(test, job_name, processor)
++    builder = nth_builder(test, build, builder_number)
++    bq.markAsBuilding(builder)
++
++
++class TestBuildJobBase(TestCaseWithFactory):
++    """Setup the test publisher and some builders."""
++
++    layer = LaunchpadZopelessLayer
++
++    def setUp(self):
++        super(TestBuildJobBase, self).setUp()
++        self.publisher = SoyuzTestPublisher()
++        self.publisher.prepareBreezyAutotest()
++
++        self.i8 = self.factory.makeBuilder(name='i386-n-8', virtualized=False)
++        self.i9 = self.factory.makeBuilder(name='i386-n-9', virtualized=False)
++
++        processor_fam = ProcessorFamilySet().getByName('hppa')
++        proc = processor_fam.processors[0]
++        self.h6 = self.factory.makeBuilder(
++            name='hppa-n-6', processor=proc, virtualized=False)
++        self.h7 = self.factory.makeBuilder(
++            name='hppa-n-7', processor=proc, virtualized=False)
++
++        self.builders = dict()
++        # x86 native
++        self.builders[(1, False)] = [self.i8, self.i9]
++
++        # hppa native
++        self.builders[(3, True)] = [self.h6, self.h7]
++
++        # Ensure all builders are operational.
++        for builders in self.builders.values():
++            for builder in builders:
++                builder.builderok = True
++                builder.manual = False
++
++        # Disable the sample data builders.
++        getUtility(IBuilderSet)['bob'].builderok = False
++        getUtility(IBuilderSet)['frog'].builderok = False
++
++
++class TestBuildPackageJob(TestBuildJobBase):
++    """Test dispatch time estimates for binary builds (i.e. single build
++    farm job type) targetting a single processor architecture and the primary
++    archive.
++    """
++    def setUp(self):
++        """Set up some native x86 builds for the test archive."""
++        super(TestBuildPackageJob, self).setUp()
++        # The builds will be set up as follows:
++        #
++        # j: 3        gedit p: hppa v:False e:0:01:00 *** s: 1001
++        # j: 4        gedit p:  386 v:False e:0:02:00 *** s: 1002
++        # j: 5      firefox p: hppa v:False e:0:03:00 *** s: 1003
++        # j: 6      firefox p:  386 v:False e:0:04:00 *** s: 1004
++        # j: 7     cobblers p: hppa v:False e:0:05:00 *** s: 1005
++        # j: 8     cobblers p:  386 v:False e:0:06:00 *** s: 1006
++        # j: 9 thunderpants p: hppa v:False e:0:07:00 *** s: 1007
++        # j:10 thunderpants p:  386 v:False e:0:08:00 *** s: 1008
++        # j:11          apg p: hppa v:False e:0:09:00 *** s: 1009
++        # j:12          apg p:  386 v:False e:0:10:00 *** s: 1010
++        # j:13          vim p: hppa v:False e:0:11:00 *** s: 1011
++        # j:14          vim p:  386 v:False e:0:12:00 *** s: 1012
++        # j:15          gcc p: hppa v:False e:0:13:00 *** s: 1013
++        # j:16          gcc p:  386 v:False e:0:14:00 *** s: 1014
++        # j:17        bison p: hppa v:False e:0:15:00 *** s: 1015
++        # j:18        bison p:  386 v:False e:0:16:00 *** s: 1016
++        # j:19         flex p: hppa v:False e:0:17:00 *** s: 1017
++        # j:20         flex p:  386 v:False e:0:18:00 *** s: 1018
++        # j:21     postgres p: hppa v:False e:0:19:00 *** s: 1019
++        # j:22     postgres p:  386 v:False e:0:20:00 *** s: 1020
++        #
++        # j=job, p=processor, v=virtualized, e=estimated_duration, s=score
++
++        # First mark all builds in the sample data as already built.
++        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
++        sample_data = store.find(Build)
++        for build in sample_data:
++            build.buildstate = BuildStatus.FULLYBUILT
++        store.flush()
++
++        # We test builds that target a primary archive.
++        self.non_ppa = self.factory.makeArchive(
++            name="primary", purpose=ArchivePurpose.PRIMARY)
++        self.non_ppa.require_virtualized = False
++
++        self.builds = []
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="gedit", status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="firefox",
++                status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="cobblers",
++                status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="thunderpants",
++                status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="apg", status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="vim", status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="gcc", status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="bison", status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="flex", status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        self.builds.extend(
++            self.publisher.getPubSource(
++                sourcename="postgres",
++                status=PackagePublishingStatus.PUBLISHED,
++                archive=self.non_ppa,
++                architecturehintlist='any').createMissingBuilds())
++        # We want the builds to have a lot of variety when it comes to score
++        # and estimated duration etc. so that the queries under test get
++        # exercised properly.
++        score = 1000
++        duration = 0
++        for build in self.builds:
++            score += 1
++            duration += 60
++            bq = build.buildqueue_record
++            bq.lastscore = score
++            bq.estimated_duration = timedelta(seconds=duration)
++
++    def test_x86_pending_queries(self):
++        # Select x86 jobs with equal or better score.
++        #
++        # The x86 builds are as follows:
++        #
++        # j: 4        gedit p:  386 v:False e:0:02:00 *** s: 1002
++        # j: 6      firefox p:  386 v:False e:0:04:00 *** s: 1004
++        # j: 8     cobblers p:  386 v:False e:0:06:00 *** s: 1006
++        # j:10 thunderpants p:  386 v:False e:0:08:00 *** s: 1008
++        # j:12          apg p:  386 v:False e:0:10:00 *** s: 1010
++        # j:14          vim p:  386 v:False e:0:12:00 *** s: 1012
++        # j:16          gcc p:  386 v:False e:0:14:00 *** s: 1014
++        # j:18        bison p:  386 v:False e:0:16:00 *** s: 1016
++        # j:20         flex p:  386 v:False e:0:18:00 *** s: 1018
++        # j:22     postgres p:  386 v:False e:0:20:00 *** s: 1020
++        #
++        # Please note: the higher scored jobs are lower in the list.
++        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
++
++        build, bq = find_job(self, 'apg')
++        bpj = bq.specific_job
++        query = bpj.composePendingJobsQuery(1010, *builder_key(build))
++        result_set = store.execute(query).get_all()
++        # The pending x86 jobs with score 1010 or higher are as follows.
++        # Please note that we do not require the results to be in any
++        # particular order.
++
++        # Processor == 1 -> Intel 386
++        # SELECT id,name,title FROM processor
++        #  id | name  |     title
++        # ----+-------+----------------
++        #   1 | 386   | Intel 386
++        #   2 | amd64 | AMD 64bit
++        #   3 | hppa  | HPPA Processor
++
++        expected_results = [
++        #   job  score estimated_duration   processor   virtualized
++            (12, 1010, timedelta(0, 600),       1,      False),
++            (14, 1012, timedelta(0, 720),       1,      False),
++            (16, 1014, timedelta(0, 840),       1,      False),
++            (18, 1016, timedelta(0, 960),       1,      False),
++            (20, 1018, timedelta(0, 1080),      1,      False),
++            (22, 1020, timedelta(0, 1200),      1,      False)]
++        self.assertEqual(sorted(result_set), sorted(expected_results))
++        # How about builds with lower scores? Please note also that no
++        # hppa builds are listed.
++        query = bpj.composePendingJobsQuery(0, *builder_key(build))
++        result_set = store.execute(query).get_all()
++        expected_results = [
++        #   job  score estimated_duration   processor   virtualized
++            (22, 1020, timedelta(0, 1200),      1,      False),
++            (20, 1018, timedelta(0, 1080),      1,      False),
++            (18, 1016, timedelta(0, 960),       1,      False),
++            (16, 1014, timedelta(0, 840),       1,      False),
++            (14, 1012, timedelta(0, 720),       1,      False),
++            (12, 1010, timedelta(0, 600),       1,      False),
++            (10, 1008, timedelta(0, 480),       1,      False),
++            (8,  1006, timedelta(0, 360),       1,      False),
++            (6,  1004, timedelta(0, 240),       1,      False),
++            (4,  1002, timedelta(0, 120),       1,      False)]
++        self.assertEqual(sorted(result_set), sorted(expected_results))
++        # How about builds with higher scores?
++        query = bpj.composePendingJobsQuery(2500, *builder_key(build))
++        result_set = store.execute(query).get_all()
++        expected_results = []
++        self.assertEqual(sorted(result_set), sorted(expected_results))
++
++        # We will start the 'flex' job now and see whether it still turns
++        # up in our pending job list.
++        assign_to_builder(self, 'flex', 1)
++        query = bpj.composePendingJobsQuery(1016, *builder_key(build))
++        result_set = store.execute(query).get_all()
++        expected_results = [
++        #   job  score estimated_duration   processor   virtualized
++            (22, 1020, timedelta(0, 1200),      1,      False),
++            (18, 1016, timedelta(0, 960),       1,      False)]
++        self.assertEqual(sorted(result_set), sorted(expected_results))
++        # As we can see it was absent as expected.
++
++    def test_hppa_pending_queries(self):
++        # Select hppa jobs with equal or better score.
++        #
++        # The hppa builds are as follows:
++        #
++        # j: 3        gedit p: hppa v:False e:0:01:00 *** s: 1001
++        # j: 5      firefox p: hppa v:False e:0:03:00 *** s: 1003
++        # j: 7     cobblers p: hppa v:False e:0:05:00 *** s: 1005
++        # j: 9 thunderpants p: hppa v:False e:0:07:00 *** s: 1007
++        # j:11          apg p: hppa v:False e:0:09:00 *** s: 1009
++        # j:13          vim p: hppa v:False e:0:11:00 *** s: 1011
++        # j:15          gcc p: hppa v:False e:0:13:00 *** s: 1013
++        # j:17        bison p: hppa v:False e:0:15:00 *** s: 1015
++        # j:19         flex p: hppa v:False e:0:17:00 *** s: 1017
++        # j:21     postgres p: hppa v:False e:0:19:00 *** s: 1019
++        #
++        # Please note: the higher scored jobs are lower in the list.
++        store = getUtility(IStoreSelector).get(MAIN_STORE, DEFAULT_FLAVOR)
++
++        build, bq = find_job(self, 'vim', 'hppa')
++        bpj = bq.specific_job
++        query = bpj.composePendingJobsQuery(1011, *builder_key(build))
++        result_set = store.execute(query).get_all()
++        # The pending hppa jobs with score 1011 or higher are as follows.
++        # Please note that we do not require the results to be in any
++        # particular order.
++
++        # Processor == 3 -> HPPA
++        # SELECT id,name,title FROM processor
++        #  id | name  |     title
++        # ----+-------+----------------
++        #   1 | 386   | Intel 386
++        #   2 | amd64 | AMD 64bit
++        #   3 | hppa  | HPPA Processor
++
++        expected_results = [
++        #   job  score estimated_duration   processor   virtualized
++            (13, 1011, timedelta(0, 660),       3,      False),
++            (15, 1013, timedelta(0, 780),       3,      False),
++            (17, 1015, timedelta(0, 900),       3,      False),
++            (19, 1017, timedelta(0, 1020),      3,      False),
++            (21, 1019, timedelta(0, 1140),      3,      False)]
++        self.assertEqual(sorted(result_set), sorted(expected_results))
++        # How about builds with lower scores? Please note also that no
++        # hppa builds are listed.
++        query = bpj.composePendingJobsQuery(0, *builder_key(build))
++        result_set = store.execute(query).get_all()
++        expected_results = [
++        #   job  score estimated_duration   processor   virtualized
++            (3,  1001, timedelta(0, 60),        3,      False),
++            (5,  1003, timedelta(0, 180),       3,      False),
++            (7,  1005, timedelta(0, 300),       3,      False),
++            (9,  1007, timedelta(0, 420),       3,      False),
++            (11, 1009, timedelta(0, 540),       3,      False),
++            (13, 1011, timedelta(0, 660),       3,      False),
++            (15, 1013, timedelta(0, 780),       3,      False),
++            (17, 1015, timedelta(0, 900),       3,      False),
++            (19, 1017, timedelta(0, 1020),      3,      False),
++            (21, 1019, timedelta(0, 1140),      3,      False)]
++        self.assertEqual(sorted(result_set), sorted(expected_results))
++        # How about builds with higher scores?
++        query = bpj.composePendingJobsQuery(2500, *builder_key(build))
++        result_set = store.execute(query).get_all()
++        expected_results = []
++        self.assertEqual(sorted(result_set), sorted(expected_results))
++
++        # We will start the 'flex' job now and see whether it still turns
++        # up in our pending job list.
++        assign_to_builder(self, 'flex', 1, 'hppa')
++        query = bpj.composePendingJobsQuery(1014, *builder_key(build))
++        result_set = store.execute(query).get_all()
++        expected_results = [
++        #   job  score estimated_duration   processor   virtualized
++            (17, 1015, timedelta(0, 900),       3,      False),
++            (21, 1019, timedelta(0, 1140),      3,      False)]
++        self.assertEqual(sorted(result_set), sorted(expected_results))
++        # As we can see it was absent as expected.
++
++    def test_processor(self):
++        # Test that BuildPackageJob returns the correct processor.
++        build, bq = find_job(self, 'gcc', '386')
++        bpj = bq.specific_job
++        self.assertEqual(bpj.processor.id, 1)
++        build, bq = find_job(self, 'bison', 'hppa')
++        bpj = bq.specific_job
++        self.assertEqual(bpj.processor.id, 3)
++
++    def test_virtualized(self):
++        # Test that BuildPackageJob returns the correct virtualized flag.
++        build, bq = find_job(self, 'apg', '386')
++        bpj = bq.specific_job
++        self.assertEqual(bpj.virtualized, False)
++        build, bq = find_job(self, 'flex', 'hppa')
++        bpj = bq.specific_job
++        self.assertEqual(bpj.virtualized, False)
++
++    def test_provides_dispatch_estimation_interface(self):
++        verifyObject(IBuildFarmJobDispatchEstimation, BuildPackageJob)