Merge lp:~wgrant/launchpad/faster-and-more-general-getBuildQueueSizes into lp:launchpad

Proposed by William Grant
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 11102
Proposed branch: lp:~wgrant/launchpad/faster-and-more-general-getBuildQueueSizes
Merge into: lp:launchpad
Diff against target: 121 lines (+30/-28)
3 files modified
lib/lp/buildmaster/doc/builder.txt (+18/-0)
lib/lp/buildmaster/model/builder.py (+9/-27)
lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt (+3/-1)
To merge this branch: bzr merge lp:~wgrant/launchpad/faster-and-more-general-getBuildQueueSizes
Reviewer Review Type Date Requested Status
Abel Deuring (community) code Approve
Review via email: mp+28476@code.launchpad.net

Commit message

Generalise BuilderSet.getBuildQueueSizes.

Description of the change

This branch generalises BuilderSet.getBuildQueueSizes to consider all build farm jobs -- not just BinaryPackageBuilds. This also makes the query a whole lot simpler, and reduces lp.buildmaster's dependency on lp.soyuz.

As mentioned in bug #592573, this misses jobs which are architecture-agnostic (processor IS NULL). But we don't have any such jobs at the moment, probably won't for the foreseeable future, and there's no UI to display them either.

There is no lint.

To post a comment you must log in.
Revision history for this message
Michael Nelson (michael.nelson) wrote :

{{{
09:20 < wgrant> Nice simple buildfarm-related branch at https://code.edge.launchpad.net/~wgrant/launchpad/faster-and-more-general-getBuildQueueSizes/+merge/28476, if anyone's interested...
09:21 < noodles775> Nice... more red than green :)
09:22 < noodles775> And much simpler query. Is it worth adding tests that ensure the other build types are considered (or possible yet? or worth waiting to land this when it is possible?)
09:23 < wgrant> It's not possible yet.
09:24 < wgrant> Hm.
09:24 < wgrant> Actually, it might be.
09:24 < wgrant> Since this is just BuildQueue, not BuildFarmJob.
09:25 < noodles775> Right - which means it's implied anyway (and doesn't need to be tested... as the BFJ table isn't even referenced)?
09:26 < wgrant> If it works for one it will work for the others. It would still be nice to test, though.
09:26 < wgrant> Let's see how easy the tests are to change.
09:27 < noodles775> And would ensure the implementation doesn't change in the future to exclude them... great, thanks wgrant.
}}}

Revision history for this message
Abel Deuring (adeuring) :
review: Approve (code)
Revision history for this message
William Grant (wgrant) wrote :

There was a test failure. It is now fixed.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/doc/builder.txt'
2--- lib/lp/buildmaster/doc/builder.txt 2010-06-04 14:50:46 +0000
3+++ lib/lp/buildmaster/doc/builder.txt 2010-06-25 23:15:43 +0000
4@@ -161,6 +161,24 @@
5 >>> queue_sizes['nonvirt']['386']
6 (1L, datetime.timedelta(0, 60))
7
8+All job types are included. If we create a recipe build job, it will
9+show up in the calculated queue size.
10+
11+ >>> recipe_bq = factory.makeSourcePackageRecipeBuildJob()
12+ >>> # XXX wgrant 20100625 bug=598397: The factory erroneously creates
13+ >>> # architecture-independent jobs.
14+ >>> print recipe_bq.processor
15+ None
16+
17+ >>> from lp.soyuz.interfaces.processor import IProcessorFamilySet
18+ >>> i386_family = getUtility(IProcessorFamilySet).getByName('x86')
19+ >>> recipe_bq.processor = i386_family.processors[0]
20+
21+ >>> transaction.commit()
22+ >>> queue_sizes = builderset.getBuildQueueSizes()
23+ >>> print queue_sizes['virt']['386']
24+ (1L, datetime.timedelta(0, 64))
25+
26
27 Resuming buildd slaves
28 ======================
29
30=== modified file 'lib/lp/buildmaster/model/builder.py'
31--- lib/lp/buildmaster/model/builder.py 2010-06-16 12:07:31 +0000
32+++ lib/lp/buildmaster/model/builder.py 2010-06-25 23:15:43 +0000
33@@ -40,7 +40,6 @@
34 IStoreSelector, MAIN_STORE, DEFAULT_FLAVOR, SLAVE_FLAVOR)
35 from canonical.lazr.utils import safe_hasattr
36 from canonical.librarian.utils import copy_and_close
37-from lp.buildmaster.interfaces.buildbase import BuildStatus
38 from lp.buildmaster.interfaces.builder import (
39 BuildDaemonError, BuildSlaveFailure, CannotBuild, CannotFetchFile,
40 CannotResumeHost, CorruptBuildCookie, IBuilder, IBuilderSet,
41@@ -49,20 +48,19 @@
42 from lp.buildmaster.interfaces.buildfarmjobbehavior import (
43 BuildBehaviorMismatch)
44 from lp.buildmaster.interfaces.buildqueue import IBuildQueueSet
45-from lp.buildmaster.model.buildfarmjob import BuildFarmJob
46 from lp.buildmaster.model.buildfarmjobbehavior import IdleBuildBehavior
47 from lp.buildmaster.model.buildqueue import BuildQueue, specific_job_classes
48-from lp.buildmaster.model.packagebuild import PackageBuild
49 from canonical.database.sqlbase import SQLBase, sqlvalues
50 from lp.registry.interfaces.person import validate_public_person
51 from lp.services.job.interfaces.job import JobStatus
52+from lp.services.job.model.job import Job
53 # XXX Michael Nelson 2010-01-13 bug=491330
54 # These dependencies on soyuz will be removed when getBuildRecords()
55 # is moved.
56 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
57 from lp.soyuz.interfaces.buildrecords import (
58 IHasBuildRecords, IncompatibleArguments)
59-from lp.soyuz.model.buildpackagejob import BuildPackageJob
60+from lp.soyuz.model.processor import Processor
61
62
63 class TimeoutHTTPConnection(httplib.HTTPConnection):
64@@ -701,32 +699,16 @@
65
66 def getBuildQueueSizes(self):
67 """See `IBuilderSet`."""
68- # Avoiding circular imports.
69- from lp.soyuz.model.archive import Archive
70- from lp.soyuz.model.binarypackagebuild import BinaryPackageBuild
71- from lp.soyuz.model.distroarchseries import (
72- DistroArchSeries)
73- from lp.soyuz.model.processor import Processor
74-
75- find_spec = (
76+ store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR)
77+ results = store.find((
78 Count(),
79 Sum(BuildQueue.estimated_duration),
80 Processor,
81- Archive.require_virtualized,
82- )
83- store = getUtility(IStoreSelector).get(MAIN_STORE, SLAVE_FLAVOR)
84- results = store.find(
85- find_spec,
86- BuildPackageJob.job == BuildQueue.jobID,
87- BuildPackageJob.build == BinaryPackageBuild.id,
88- BinaryPackageBuild.package_build == PackageBuild.id,
89- PackageBuild.build_farm_job == BuildFarmJob.id,
90- PackageBuild.archive == Archive.id,
91- BinaryPackageBuild.distro_arch_series == DistroArchSeries.id,
92- DistroArchSeries.processorfamilyID == Processor.familyID,
93- BuildFarmJob.status == BuildStatus.NEEDSBUILD,
94- Archive._enabled == True).group_by(
95- Processor, Archive.require_virtualized)
96+ BuildQueue.virtualized),
97+ Processor.id == BuildQueue.processorID,
98+ Job.id == BuildQueue.jobID,
99+ Job._status == JobStatus.WAITING).group_by(
100+ Processor, BuildQueue.virtualized)
101
102 result_dict = {'virt': {}, 'nonvirt': {}}
103 for size, duration, processor, virtualized in results:
104
105=== modified file 'lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt'
106--- lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt 2010-03-06 04:57:40 +0000
107+++ lib/lp/soyuz/stories/soyuz/xx-buildfarm-index.txt 2010-06-25 23:15:43 +0000
108@@ -80,10 +80,12 @@
109 386 0 1 job (a minute)
110
111 If the archive for the build does not require virtual builders, then
112-the pending job will appear in the 'official' queue:
113+the pending job will appear in the 'official' queue. Since the build
114+record already exists, we must manually set it to non-virtualized too.
115
116 >>> login('foo.bar@canonical.com')
117 >>> cprov.archive.require_virtualized = False
118+ >>> removeSecurityProxy(bq).virtualized = False
119 >>> logout()
120
121 >>> anon_browser.reload()