Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Merged in bug 636101 from default branch:

changeset:   1782:204086db1d2c
tag:         tip
parent:      1779:48a72b08bb38
user:        Chris AtLee <catlee@mozilla.com>
date:        Thu Mar 03 16:55:33 2011 -0500
summary:     Bug 636101: Fix fast slave selection. r=dustin,nthomas

--HG--
branch : production-0.8
  • Loading branch information...
commit e161507615567d5e66aebd12ea86f7dafa3a3103 2 parents 3e40cc9 + 5b84eb6
@catlee catlee authored
Showing with 184 additions and 30 deletions.
  1. +26 −30 misc.py
  2. +158 −0 test/test_misc_nextslaves.py
View
56 misc.py
@@ -269,34 +269,50 @@ def _nextSlowSlave(builder, available_slaves):
elif fast:
return sorted(fast, _recentSort(builder))[-1]
else:
- return []
+ return None
except:
log.msg("Error choosing next slow slave for builder '%s', choosing randomly instead" % builder.name)
log.err()
return random.choice(available_slaves)
-def _nextFastSlave(builder, available_slaves, only_fast=False):
+def _nextFastSlave(builder, available_slaves, only_fast=False, reserved=False):
+ # Check if our reserved slaves count needs updating
+ global _checkedReservedSlaveFile, _reservedFileName
+ if int(time.time() - _checkedReservedSlaveFile) > 60:
+ _readReservedFile(_reservedFileName)
+ _checkedReservedSlaveFile = int(time.time())
+
try:
if only_fast:
- # Check that the builder has some fast slaves configured.
- # We do this because some machines classes don't have a fast/slow distinction, and
- # so they default to 'slow'
+ # Check that the builder has some fast slaves configured. We do
+ # this because some machines classes don't have a fast/slow
+ # distinction, and so they default to 'slow'
+ # We should look at the full set of slaves here regardless of if
+ # we're only supposed to be returning unreserved slaves so we get
+ # the full set of slaves on the builder.
fast, slow = _partitionSlaves(builder.slaves)
if not fast:
log.msg("Builder '%s' has no fast slaves configured, but only_fast is enabled; disabling only_fast" % builder.name)
only_fast = False
- fast, slow = _partitionUnreservedSlaves(available_slaves)
+ if reserved:
+ # We have access to the full set of slaves!
+ fast, slow = _partitionSlaves(available_slaves)
+ else:
+ # We only have access to unreserved slaves
+ fast, slow = _partitionUnreservedSlaves(available_slaves)
# Choose the fast slave that was most recently on this builder
# If there aren't any fast slaves, choose the slow slave that was most
# recently on this builder if only_fast is False
- if fast:
+ if not fast and only_fast:
+ return None
+ elif fast:
return sorted(fast, _recentSort(builder))[-1]
elif slow and not only_fast:
return sorted(slow, _recentSort(builder))[-1]
else:
- return []
+ return None
except:
log.msg("Error choosing next fast slave for builder '%s', choosing randomly instead" % builder.name)
log.err()
@@ -308,28 +324,8 @@ def setReservedFileName(filename):
global _reservedFileName
_reservedFileName = filename
-def _nextFastReservedSlave(builder, available_slaves, onlyFast=True):
- global _checkedReservedSlaveFile, _reservedFileName
- if int(time.time() - _checkedReservedSlaveFile) > 60:
- _readReservedFile(_reservedFileName)
- _checkedReservedSlaveFile = int(time.time())
-
- try:
- fast, slow = _partitionSlaves(available_slaves)
- # Choose the fast slave that was most recently on this builder
- # If there aren't any fast slaves, choose the slow slave that was most
- # recently on this builder if onlyFast isn't set
- if fast:
- return sorted(fast, _recentSort(builder))[-1]
- elif not onlyFast and slow:
- return sorted(slow, _recentSort(builder))[-1]
- else:
- log.msg("No fast or slow slaves found for builder '%s', choosing randomly instead" % builder.name)
- return random.choice(available_slaves)
- except:
- log.msg("Error choosing next fast slave for builder '%s', choosing randomly instead" % builder.name)
- log.err()
- return random.choice(available_slaves)
+def _nextFastReservedSlave(builder, available_slaves, only_fast=True):
+ return _nextFastSlave(builder, available_slaves, only_fast, reserved=True)
def _nextL10nSlave(n=8):
"""Return a nextSlave function that restricts itself to choosing amongst
View
158 test/test_misc_nextslaves.py
@@ -0,0 +1,158 @@
+import time
+import tempfile
+
+import mock
+
+from twisted.trial import unittest
+
+import buildbotcustom.misc
+from buildbotcustom.misc import _nextSlowIdleSlave, _nextL10nSlave,\
+ _nextFastSlave, _nextFastReservedSlave, _nextSlowSlave,\
+ setReservedFileName
+
+class TestNextSlaveFuncs(unittest.TestCase):
+ def setUp(self):
+ # Reset these each time
+ buildbotcustom.misc.fastRegexes = ['fast']
+ buildbotcustom.misc.nReservedFastSlaves = 0
+ buildbotcustom.misc.nReservedSlowSlaves = 0
+
+ # Prevent looking for reserved slaves file
+ buildbotcustom.misc._checkedReservedSlaveFile = time.time()
+
+ self.slaves = slaves = []
+ for name in ('fast1', 'fast2', 'fast3', 'slow1', 'slow2', 'slow3'):
+ slave = mock.Mock()
+ slave.slave.slavename = name
+ slaves.append(slave)
+ self.slow_slaves = [s for s in self.slaves if "slow" in s.slave.slavename]
+ self.fast_slaves = [s for s in self.slaves if "fast" in s.slave.slavename]
+
+ self.builder = builder = mock.Mock()
+ builder.builder_status.buildCache.keys.return_value = []
+ builder.slaves = self.slaves
+
+ def test_nextFastSlave_AllAvail(self):
+ """Test that _nextFastSlave and _nextFastReservedSlave return a fast
+ slave when all slaves are available."""
+ for func in _nextFastReservedSlave, _nextFastSlave:
+ slave = func(self.builder, self.slaves, only_fast=True)
+ self.assert_(slave.slave.slavename.startswith("fast"))
+
+ def test_nextFastSlave_OnlySlowAvail(self):
+ """Test that _nextFastSlave and _nextFastReservedSlave return None
+ slave when only slow slaves are available, and only_fast is True."""
+ for func in _nextFastReservedSlave, _nextFastSlave:
+ slave = func(self.builder, self.slow_slaves, only_fast=True)
+ self.assert_(slave is None)
+
+ def test_nextFastSlave_OnlySlowAvail_notOnlyFast(self):
+ """Test that _nextFastSlave and _nextFastReservedSlave return a slow
+ slave when only slow slaves are available and only_fast is False."""
+ for func in _nextFastReservedSlave, _nextFastSlave:
+ slave = func(self.builder, self.slow_slaves, only_fast=False)
+ self.assert_(slave.slave.slavename.startswith("slow"))
+
+ def test_nextFastReservedSlave_reserved(self):
+ """Test that _nextFastReservedSlave returns a fast slave if there's one
+ reserved."""
+ buildbotcustom.misc.nReservedFastSlaves = 1
+
+ # Only one fast slave available
+ available_slaves = [s for s in self.slaves if s.slave.slavename == 'fast2']
+ slave = _nextFastReservedSlave(self.builder, available_slaves)
+ self.assert_(slave.slave.slavename == "fast2")
+
+ def test_nextFastSlave_reserved(self):
+ """Test that _nextFastSlave returns None if there's one slave
+ reserved."""
+ buildbotcustom.misc.nReservedFastSlaves = 1
+
+ # Only one fast slave available
+ available_slaves = [s for s in self.slaves if s.slave.slavename == 'fast2']
+ slave = _nextFastSlave(self.builder, available_slaves)
+ self.assert_(slave is None)
+
+ def test_nextFastSlave_allslow(self):
+ """Test that _nextFastSlave works if the builder is configured with
+ just slow slaves. This handles the case for platforms that don't have a
+ fast/slow distinction."""
+ buildbotcustom.misc.nReservedFastSlaves = 1
+ self.builder.slaves = self.slow_slaves
+
+ slave = _nextFastSlave(self.builder, self.slow_slaves, only_fast=True)
+ self.assert_(slave.slavename.startswith('slow'))
+
+ def test_nextSlowSlave(self):
+ """Test that _nextSlowSlave returns a slow slave if one is available."""
+ slave = _nextSlowSlave(self.builder, self.slaves)
+ self.assert_(slave.slave.slavename.startswith("slow"))
+
+ def test_nextSlowSlave_OnlyFastAvail(self):
+ """Test that _nextSlowSlave returns a fast slave if no slow slaves are
+ available."""
+ slave = _nextSlowSlave(self.builder, self.fast_slaves)
+ self.assert_(slave.slave.slavename.startswith("fast"))
+
+ def test_nextSlowIdleSlave_avail(self):
+ """Test that _nextSlowIdleSlave returns a slow slave if enough slow
+ slaves are available."""
+ func = _nextSlowIdleSlave(1)
+ slave = func(self.builder, self.slaves)
+ self.assert_(slave.slave.slavename.startswith("slow"))
+
+ def test_nextSlowIdleSlave_unavail(self):
+ """Test that _nextSlowIdleSlave returns None if not enough slow
+ slaves are available."""
+ func = _nextSlowIdleSlave(5)
+ slave = func(self.builder, self.slaves)
+ self.assert_(slave is None)
+
+ def test_nextL10nSlave_avail(self):
+ """Test that _nextL10nSlave returns a slow slave if the first slow
+ slave is available."""
+ func = _nextL10nSlave(1)
+ slave = func(self.builder, self.slaves)
+ self.assert_(slave.slave.slavename == 'slow1')
+
+ def test_nextL10nSlave_unavail(self):
+ """Test that _nextL10nSlave returns None if the first slow slave is not
+ available."""
+ func = _nextL10nSlave(1)
+ available_slaves = [s for s in self.slaves if s.slave.slavename != 'slow1']
+ slave = func(self.builder, available_slaves)
+ self.assert_(slave is None)
+
+ def test_update_reserved(self):
+ """Test that updates to the reserved file are obeyed, and that calls to
+ the _nextFast functions pick it up."""
+ reservedFile = tempfile.NamedTemporaryFile()
+ buildbotcustom.misc._checkedReservedSlaveFile = 0
+ # Need to fake out time.time
+ with mock.patch.object(time, 'time') as time_method:
+ setReservedFileName(reservedFile.name)
+ time_method.return_value = 0
+ self.assertEquals(buildbotcustom.misc.nReservedFastSlaves, 0)
+
+ # Only one fast slave available, but none are reserved yet
+ available_slaves = [s for s in self.slaves if s.slave.slavename == 'fast2']
+ slave = _nextFastSlave(self.builder, available_slaves)
+ self.assert_(slave.slave.slavename == 'fast2')
+
+ # Reserve 1 slave
+ reservedFile.write('1')
+ reservedFile.flush()
+ time_method.return_value = 61
+
+ # Only one fast slave available, but 1 is reserved
+ available_slaves = [s for s in self.slaves if s.slave.slavename == 'fast2']
+
+ # Check that the regular function doesn't get it
+ slave = _nextFastSlave(self.builder, available_slaves, only_fast=True)
+ self.assertEquals(buildbotcustom.misc.nReservedFastSlaves, 1)
+ self.assert_(slave is None)
+
+ # But our reserved function now does
+ slave = _nextFastReservedSlave(self.builder, available_slaves, only_fast=True)
+ self.assert_(slave.slave.slavename == 'fast2')
+
Please sign in to comment.
Something went wrong with that request. Please try again.