Merge lp:~cjwatson/launchpad/remove-ppa-builder-limits into lp:launchpad

Proposed by Colin Watson
Status: Merged
Approved by: William Grant
Approved revision: no longer in the source branch.
Merged at revision: 16758
Proposed branch: lp:~cjwatson/launchpad/remove-ppa-builder-limits
Merge into: lp:launchpad
Diff against target: 120 lines (+9/-61)
2 files modified
lib/lp/buildmaster/tests/test_builder.py (+7/-17)
lib/lp/soyuz/model/buildpackagejob.py (+2/-44)
To merge this branch: bzr merge lp:~cjwatson/launchpad/remove-ppa-builder-limits
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+183834@code.launchpad.net

Commit message

Remove 80%-of-builders limitation on builds for any given public PPA and architecture.

Description of the change

== Summary ==

Bug 666308 reports unnecessary limitations on certain types of PPAs, as well as a request for more flexibility.

== Proposed fix ==

This is an RFC, but my position is that the limitation on how many builders a given PPA may use at any one time does more harm than good at this point. Now that we have reasonably reliable build cancellation, in the rare cases of abuse or needing something to jump the queue we can always cancel something and retry it later. Other than that, all that this limitation really achieves at this point is to require us to occasionally play silly games with builders' architectures to circumvent it.

We could add more complexity to make this controllable per-PPA or per-owner, as the bug suggests, but I don't think it's worth it.

== Tests ==

bin/test -vvct buildmaster -t soyuz

== Demo and Q/A ==

I don't think we can currently QA this on dogfood since it only has a single working PPA builder at the moment; but since this is pretty much just deleting code I'm not too worried.

To post a comment you must log in.
Revision history for this message
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/buildmaster/tests/test_builder.py'
2--- lib/lp/buildmaster/tests/test_builder.py 2013-09-02 07:30:53 +0000
3+++ lib/lp/buildmaster/tests/test_builder.py 2013-09-04 09:02:03 +0000
4@@ -252,20 +252,11 @@
5
6 class TestFindBuildCandidatePPA(TestFindBuildCandidatePPABase):
7
8- def test_findBuildCandidate_first_build_started(self):
9- # A PPA cannot start a build if it would use 80% or more of the
10- # builders.
11- next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
12- build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job)
13- self.failIfEqual('joesppa', build.archive.name)
14-
15- def test_findBuildCandidate_first_build_finished(self):
16- # When joe's first ppa build finishes, his fourth i386 build
17- # will be the next build candidate.
18- self.joe_builds[0].updateStatus(BuildStatus.FAILEDTOBUILD)
19- next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
20- build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job)
21- self.failUnlessEqual('joesppa', build.archive.name)
22+ def test_findBuildCandidate(self):
23+ # joe's fourth i386 build will be the next build candidate.
24+ next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
25+ build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job)
26+ self.assertEqual('joesppa', build.archive.name)
27
28 def test_findBuildCandidate_with_disabled_archive(self):
29 # Disabled archives should not be considered for dispatching
30@@ -283,11 +274,10 @@
31 ppa_joe_private = True
32
33 def test_findBuildCandidate_for_private_ppa(self):
34- # If a ppa is private it will be able to have parallel builds
35- # for the one architecture.
36+ # joe's fourth i386 build will be the next build candidate.
37 next_job = removeSecurityProxy(self.builder4)._findBuildCandidate()
38 build = getUtility(IBinaryPackageBuildSet).getByQueueEntry(next_job)
39- self.failUnlessEqual('joesppa', build.archive.name)
40+ self.assertEqual('joesppa', build.archive.name)
41
42 # If the source for the build is still pending, it won't be
43 # dispatched because the builder has to fetch the source files
44
45=== modified file 'lib/lp/soyuz/model/buildpackagejob.py'
46--- lib/lp/soyuz/model/buildpackagejob.py 2013-09-02 06:34:15 +0000
47+++ lib/lp/soyuz/model/buildpackagejob.py 2013-09-04 09:02:03 +0000
48@@ -16,17 +16,13 @@
49 from zope.interface import implements
50
51 from lp.buildmaster.enums import BuildStatus
52-from lp.buildmaster.interfaces.builder import IBuilderSet
53 from lp.buildmaster.model.buildfarmjob import BuildFarmJobOld
54 from lp.registry.interfaces.pocket import PackagePublishingPocket
55 from lp.registry.interfaces.series import SeriesStatus
56 from lp.services.database.bulk import load_related
57 from lp.services.database.interfaces import IStore
58 from lp.services.database.sqlbase import sqlvalues
59-from lp.soyuz.enums import (
60- ArchivePurpose,
61- PackagePublishingStatus,
62- )
63+from lp.soyuz.enums import PackagePublishingStatus
64 from lp.soyuz.interfaces.binarypackagebuild import IBinaryPackageBuildSet
65 from lp.soyuz.interfaces.buildpackagejob import (
66 COPY_ARCHIVE_SCORE_PENALTY,
67@@ -129,7 +125,7 @@
68 PackagePublishingStatus.SUPERSEDED,
69 PackagePublishingStatus.DELETED,
70 )
71- sub_query = """
72+ return """
73 SELECT TRUE FROM Archive, BinaryPackageBuild, BuildPackageJob,
74 DistroArchSeries
75 WHERE
76@@ -154,44 +150,6 @@
77 BinaryPackageBuild.status = %s
78 """ % sqlvalues(private_statuses, BuildStatus.NEEDSBUILD)
79
80- # Ensure that if BUILDING builds exist for the same
81- # public ppa archive and architecture and another would not
82- # leave at least 20% of them free, then we don't consider
83- # another as a candidate.
84- #
85- # This clause selects the count of currently building builds on
86- # the arch in question, then adds one to that total before
87- # deriving a percentage of the total available builders on that
88- # arch. It then makes sure that percentage is under 80.
89- #
90- # The extra clause is only used if the number of available
91- # builders is greater than one, or nothing would get dispatched
92- # at all.
93- num_arch_builders = getUtility(IBuilderSet).getBuildersForQueue(
94- processor, virtualized).count()
95- if num_arch_builders > 1:
96- sub_query += """
97- AND Archive.id NOT IN (
98- SELECT Archive.id
99- FROM Archive, BinaryPackageBuild, DistroArchSeries
100- WHERE
101- BinaryPackageBuild.distro_arch_series = DistroArchSeries.id
102- AND DistroArchSeries.processorfamily = %s
103- AND BinaryPackageBuild.status = %s
104- AND BinaryPackageBuild.archive = Archive.id
105- AND Archive.purpose = %s
106- AND Archive.private IS FALSE
107- GROUP BY Archive.id
108- HAVING (
109- (count(*)+1) * 100.0 / %s
110- ) >= 80
111- )
112- """ % sqlvalues(
113- processor.family, BuildStatus.BUILDING,
114- ArchivePurpose.PPA, num_arch_builders)
115-
116- return sub_query
117-
118 @staticmethod
119 def postprocessCandidate(job, logger):
120 """See `IBuildFarmJob`."""