From 2125831d5cfbae353bbb504710cfafef6efce56c Mon Sep 17 00:00:00 2001 From: warner Date: Wed, 8 Aug 2007 04:40:12 +0100 Subject: [PATCH] add max_builds= to BuildSlave, thanks to Dustin Mitchell. Closes #48. --- ChangeLog | 18 ++++++++++++++ buildbot/buildslave.py | 20 ++++++++++++++-- buildbot/process/builder.py | 7 ++++-- buildbot/scripts/sample.cfg | 4 +++- buildbot/test/test_run.py | 57 +++++++++++++++++++++++++++++++++++++++++++++ docs/buildbot.texinfo | 10 +++++++- 6 files changed, 110 insertions(+), 6 deletions(-) diff --git a/ChangeLog b/ChangeLog index b7ae623..2378d1c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,23 @@ 2007-08-07 Brian Warner + * buildbot/buildslave.py (BuildSlave.__init__): add max_builds=, + which imposes a per-slave limit on how many builds are allowed to + run simultaneously. This has a the same scope than the SlaveLock, + but is different because max_builds= gives the buildmaster the + freedom to assign the build to a different slave, whereas the + SlaveLock doesn't get tested until after the build is irrevocably + assigned to a slave. Therefore using max_builds= will improve + utilization in the presence of multiple buildslaves that are + attached to the same Builder. This completes the incorporation of + Dustin Mitchell's patches, and closes ticket #48. Thanks Dustin! + * buildbot/process/builder.py (SlaveBuilder.buildFinished): when + any Builder finishes, potentially trigger *all* Builders, since + max_builds= may have stalled someone else while waiting for the + slave. + * buildbot/scripts/sample.cfg: mention max_builds= + * buildbot/test/test_run.py (ConcurrencyLimit): test it + * docs/buildbot.texinfo (Buildslave Specifiers): document it + * buildbot/test/test_run.py (CanStartBuild._do_test2): tests which inherit from RunMixin do not need to call master.stopService() themselves, since RunMixin.tearDown does that. The double call diff --git a/buildbot/buildslave.py b/buildbot/buildslave.py index 8dc733b..4e5d0c6 100644 --- a/buildbot/buildslave.py +++ b/buildbot/buildslave.py @@ -16,7 +16,16 @@ class BuildSlave(NewCredPerspective): running builds. I am instantiated by the configuration file, and can be subclassed to add extra functionality.""" - def __init__(self, name, password): + def __init__(self, name, password, max_builds=None): + """ + @param name: botname this machine will supply when it connects + @param password: password this machine will supply when + it connects + @param max_builds: maximum number of simultaneous builds that will + be run concurrently on this buildslave (the + default is None for no limit) + """ + self.slavename = name self.password = password self.botmaster = None # no buildmaster yet @@ -24,6 +33,7 @@ class BuildSlave(NewCredPerspective): self.slave = None # a RemoteReference to the Bot, when connected self.slave_commands = None self.slavebuilders = [] + self.max_builds = max_builds def update(self, new): """ @@ -33,7 +43,9 @@ class BuildSlave(NewCredPerspective): """ # the reconfiguration logic should guarantee this: assert self.slavename == new.slavename - self.password = new.password + assert self.password == new.password + assert self.__class__ == new.__class__ + self.max_builds = new.max_builds def __repr__(self): builders = self.botmaster.getBuildersForSlave(self.slavename) @@ -237,5 +249,9 @@ class BuildSlave(NewCredPerspective): can start a build. This function can be used to limit overall concurrency on the buildslave. """ + if self.max_builds: + active_builders = [sb for sb in self.slavebuilders if sb.isBusy()] + if len(active_builders) >= self.max_builds: + return False return True diff --git a/buildbot/process/builder.py b/buildbot/process/builder.py index d2e362a..561aa1b 100644 --- a/buildbot/process/builder.py +++ b/buildbot/process/builder.py @@ -50,7 +50,7 @@ class SlaveBuilder(pb.Referenceable): def isAvailable(self): # if this SlaveBuilder is busy, then it's definitely not available - if self.state != IDLE: + if self.isBusy(): return False # otherwise, check in with the BuildSlave @@ -60,6 +60,9 @@ class SlaveBuilder(pb.Referenceable): # no slave? not very available. return False + def isBusy(self): + return self.state != IDLE + def attached(self, slave, remote, commands): """ @type slave: L{buildbot.buildslave.BuildSlave} @@ -112,7 +115,7 @@ class SlaveBuilder(pb.Referenceable): def buildFinished(self): self.state = IDLE - reactor.callLater(0, self.builder.maybeStartBuild) + reactor.callLater(0, self.builder.botmaster.maybeStartAllBuilds) def ping(self, timeout, status=None): """Ping the slave to make sure it is still there. Returns a Deferred diff --git a/buildbot/scripts/sample.cfg b/buildbot/scripts/sample.cfg index 4a295c9..311fda0 100644 --- a/buildbot/scripts/sample.cfg +++ b/buildbot/scripts/sample.cfg @@ -22,13 +22,15 @@ c = BuildmasterConfig = {} from buildbot.buildslave import BuildSlave c['slaves'] = [BuildSlave("bot1name", "bot1passwd")] +# to limit to two concurrent builds on a slave, use +# c['slaves'] = [BuildSlave("bot1name", "bot1passwd", max_builds=2)] + # 'slavePortnum' defines the TCP port to listen on. This must match the value # configured into the buildslaves (with their --master option) c['slavePortnum'] = 9989 - ####### CHANGESOURCES # the 'change_source' setting tells the buildmaster how it should find out diff --git a/buildbot/test/test_run.py b/buildbot/test/test_run.py index f8d2029..5af935e 100644 --- a/buildbot/test/test_run.py +++ b/buildbot/test/test_run.py @@ -56,6 +56,19 @@ class MyBuildSlave(BuildSlave): c['slaves'] = [ MyBuildSlave('bot1', 'sekrit') ] """ +config_concurrency = config_base + """ +from buildbot.buildslave import BuildSlave +c['slaves'] = [ BuildSlave('bot1', 'sekrit', max_builds=1) ] + +from buildbot.scheduler import Scheduler +c['schedulers'] = [Scheduler('dummy', None, 0.1, ['dummy', 'dummy2'])] + +c['builders'].append({'name': 'dummy', 'slavename': 'bot1', + 'builddir': 'dummy', 'factory': f2}) +c['builders'].append({'name': 'dummy2', 'slavename': 'bot1', + 'builddir': 'dummy2', 'factory': f2}) +""" + config_2 = config_base + """ c['builders'] = [{'name': 'dummy', 'slavename': 'bot1', 'builddir': 'dummy1', 'factory': f2}, @@ -158,6 +171,50 @@ class CanStartBuild(RunMixin, unittest.TestCase): else: self.failUnlessEqual(bs.state, IDLE) + +class ConcurrencyLimit(RunMixin, unittest.TestCase): + + def testConcurrencyLimit(self): + d = self.master.loadConfig(config_concurrency) + d.addCallback(lambda res: self.master.startService()) + d.addCallback(lambda res: self.connectSlave()) + + def _send(res): + # send a change. This will trigger both builders at the same + # time, but since they share a slave, the max_builds=1 setting + # will insure that only one of the two builds gets to run. + cm = self.master.change_svc + c = changes.Change("bob", ["Makefile", "foo/bar.c"], + "changed stuff") + cm.addChange(c) + d.addCallback(_send) + + def _delay(res): + d1 = defer.Deferred() + reactor.callLater(1, d1.callback, None) + # this test depends upon this 1s delay landing us in the middle + # of one of the builds. + return d1 + d.addCallback(_delay) + + def _check(res): + builders = [ self.master.botmaster.builders[bn] + for bn in ('dummy', 'dummy2') ] + for builder in builders: + self.failUnless(len(builder.slaves) == 1) + + from buildbot.process.builder import IDLE, BUILDING + building_bs = [ builder + for builder in builders + if builder.slaves[0].state == BUILDING ] + # assert that only one build is running right now. If the + # max_builds= weren't in effect, this would be 2. + self.failUnlessEqual(len(building_bs), 1) + d.addCallback(_check) + + return d + + class Ping(RunMixin, unittest.TestCase): def testPing(self): self.master.loadConfig(config_2) diff --git a/docs/buildbot.texinfo b/docs/buildbot.texinfo index 4e10ae6..aee901f 100644 --- a/docs/buildbot.texinfo +++ b/docs/buildbot.texinfo @@ -2260,12 +2260,20 @@ Buildslaves with an unrecognized slavename or a non-matching password will be rejected when they attempt to connect, and a message describing the problem will be put in the log file (see @ref{Logfiles}). +The @code{BuildSlave} constructor can take an optional +@code{max_builds} parameter to limit the number of builds that it will +execute simultaneously: + +@example +from buildbot.buildslave import BuildSlave +c['slaves'] = [BuildSlave("bot-linux", "linuxpassword", max_builds=2)] +@end example + Historical note: in buildbot-0.7.5 and earlier, the @code{c['bots']} key was used instead, and it took a list of (name, password) tuples. This key is accepted for backwards compatibility, but is deprecated as of 0.7.6 and will go away in some future release. - @node Defining Builders, Defining Status Targets, Buildslave Specifiers, Configuration @section Defining Builders -- 2.11.4.GIT