Merge branch 'master' of github.com:buildbot/buildbot

* 'master' of github.com:buildbot/buildbot:
  Fix accumulation of rows in scheduler_changes table
  Make the buildslave exit if it can't login
  Adding docs for changeCacheSize
  Add 'changeCacheSize' configuration key.

master/NEWS

@@ -4,6 +4,12 @@ Major User visible changes in Buildbot.             -*- outline -*-
 
 * NEXT RELEASE
 
+** New configuration key, 'changeCacheSize'
+
+This sets the number of changes that buildbot will keep in memory at once.
+Users of distributed version control systems should consider setting this to a
+high value (e.g. 10,000)
+
 ** Required Jinja Version
 
 The Buildmaster now requires Jinja-2.1 or higher.

master/buildbot/db/connector.py

@@ -1039,5 +1039,8 @@ def _txn_get_pending_brids_for_builder(self, t, buildername):
     def has_pending_operations(self):
         return bool(self._pending_operation_count)
 
+    def setChangeCacheSize(self, max_size):
+        self._change_cache.setMaxSize(max_size)
+
 
 threadable.synchronize(DBConnector)

master/buildbot/master.py

@@ -595,10 +595,10 @@ def loadConfig(self, f, check_synchronously_only=False):
                       "slavePortnum", "debugPassword", "logCompressionLimit",
                       "manhole", "status", "projectName", "projectURL",
                       "buildbotURL", "properties", "prioritizeBuilders",
-                      "eventHorizon", "buildCacheSize", "logHorizon", "buildHorizon",
-                      "changeHorizon", "logMaxSize", "logMaxTailSize",
-                      "logCompressionMethod", "db_url", "multiMaster",
-                      "db_poll_interval",
+                      "eventHorizon", "buildCacheSize", "changeCacheSize",
+                      "logHorizon", "buildHorizon", "changeHorizon",
+                      "logMaxSize", "logMaxTailSize", "logCompressionMethod",
+                      "db_url", "multiMaster", "db_poll_interval",
                       )
         for k in config.keys():
             if k not in known_keys:
@@ -623,6 +623,7 @@ def loadConfig(self, f, check_synchronously_only=False):
             buildbotURL = config.get('buildbotURL')
             properties = config.get('properties', {})
             buildCacheSize = config.get('buildCacheSize', None)
+            changeCacheSize = config.get('changeCacheSize', None)
             eventHorizon = config.get('eventHorizon', 50)
             logHorizon = config.get('logHorizon', None)
             buildHorizon = config.get('buildHorizon', None)
@@ -851,6 +852,7 @@ def loadConfig(self, f, check_synchronously_only=False):
             self.botmaster.prioritizeBuilders = prioritizeBuilders
 
         self.buildCacheSize = buildCacheSize
+        self.changeCacheSize = changeCacheSize
         self.eventHorizon = eventHorizon
         self.logHorizon = logHorizon
         self.buildHorizon = buildHorizon
@@ -938,6 +940,8 @@ def loadDatabase(self, db_spec, db_poll_interval=None):
                 to make a backup of your buildmaster before doing so.""")
 
         self.db = connector.DBConnector(db_spec)
+        if self.changeCacheSize:
+            self.db.setChangeCacheSize(self.changeCacheSize)
         self.db.start()
 
         self.botmaster.db = self.db

master/buildbot/schedulers/timed.py

@@ -226,9 +226,10 @@ def getPendingBuildTimes(self):
     def run(self):
         d = defer.succeed(None)
         db = self.parent.db
-        # always call classify_changes, so that we can keep last_processed
-        # up to date, in case we are configured with onlyIfChanged.
-        d.addCallback(lambda ign: db.runInteraction(self.classify_changes))
+        if self.onlyIfChanged:
+            # call classify_changes, so that we can keep last_processed
+            # up to date, in case we are configured with onlyIfChanged.
+            d.addCallback(lambda ign: db.runInteraction(self.classify_changes))
         d.addCallback(lambda ign: db.runInteraction(self._check_timer))
         return d
 
@@ -252,8 +253,8 @@ def _check_timer(self, t):
         return self._check_timer(t)
 
     def _maybe_start_build(self, t):
+        db = self.parent.db
         if self.onlyIfChanged:
-            db = self.parent.db
             res = db.scheduler_get_classified_changes(self.schedulerid, t)
             (important, unimportant) = res
             if not important:
@@ -275,6 +276,12 @@ def _maybe_start_build(self, t):
             # start it unconditionally
             self.start_HEAD_build(t)
 
+            # Retire any changes on this scheduler
+            res = db.scheduler_get_classified_changes(self.schedulerid, t)
+            (important, unimportant) = res
+            changeids = [c.number for c in important + unimportant]
+            db.scheduler_retire_changes(self.schedulerid, changeids, t)
+
     def _addTime(self, timetuple, secs):
         return time.localtime(time.mktime(timetuple)+secs)
 

master/buildbot/test/fake/fakedb.py

@@ -1,4 +1,6 @@
-import sys
+import sys, time
+
+from twisted.internet import defer
 
 try:
     from pysqlite2 import dbapi2 as sqlite3
@@ -28,3 +30,76 @@ def get_async_connection_pool(self):
         pool = self.pool
         self.pool = None
         return pool
+
+###
+# Note, this isn't fully equivalent to a real db connection object
+# transactions aren't emulated, scheduler state is hacked, and some methods
+# are missing or are just stubbed out.
+###
+class FakeDBConn:
+    def __init__(self):
+        self.schedulers = []
+        self.changes = []
+        self.sourcestamps = []
+        self.scheduler_states = {}
+        self.classified_changes = {}
+
+    def addSchedulers(self, schedulers):
+        i = len(self.schedulers)
+        for s in schedulers:
+            self.schedulers.append(s)
+            s.schedulerid = i
+            i += 1
+        return defer.succeed(True)
+
+    def addChangeToDatabase(self, change):
+        i = len(self.changes)
+        self.changes.append(change)
+        change.number = i
+
+    def get_sourcestampid(self, ss, t):
+        i = len(self.sourcestamps)
+        self.sourcestamps.append(ss)
+        ss.ssid = ss
+        return i
+
+    def runInteraction(self, f, *args):
+        return f(None, *args)
+
+    def scheduler_get_state(self, schedulerid, t):
+        return self.scheduler_states.get(schedulerid, {"last_processed": 0, "last_build": time.time()+100})
+
+    def scheduler_set_state(self, schedulerid, t, state):
+        self.scheduler_states[schedulerid] = state
+
+    def getLatestChangeNumberNow(self, t):
+        return len(self.changes)-1
+
+    def getChangesGreaterThan(self, last_changeid, t):
+        return self.changes[last_changeid:]
+
+    def scheduler_get_classified_changes(self, schedulerid, t):
+        return self.classified_changes.get(schedulerid, ([], []))
+
+    def scheduler_classify_change(self, schedulerid, changeid, important, t):
+        if schedulerid not in self.classified_changes:
+            self.classified_changes[schedulerid] = ([], [])
+
+        if important:
+            self.classified_changes[schedulerid][0].append(self.changes[changeid])
+        else:
+            self.classified_changes[schedulerid][1].append(self.changes[changeid])
+
+    def scheduler_retire_changes(self, schedulerid, changeids, t):
+        if schedulerid not in self.classified_changes:
+            return
+        for c in self.classified_changes[schedulerid][0][:]:
+            if c.number in changeids:
+                self.classified_changes[schedulerid][0].remove(c)
+        for c in self.classified_changes[schedulerid][1][:]:
+            if c.number in changeids:
+                self.classified_changes[schedulerid][1].remove(c)
+
+    def create_buildset(self, *args):
+        pass
+

master/buildbot/test/unit/test_schedulers_timed_Nightly.py

@@ -0,0 +1,117 @@
+import time
+
+from twisted.trial import unittest
+from twisted.internet import defer
+
+from buildbot.schedulers import timed
+from buildbot.changes.manager import ChangeManager
+from buildbot.changes.changes import Change
+from buildbot.test.fake.fakedb import FakeDBConn
+
+class DummyParent:
+    def __init__(self, dbconn):
+        self.db = dbconn
+        self.change_svc = ChangeManager()
+        self.change_svc.parent = self
+
+    def publish_buildset(self, name, bsid, t):
+        pass
+
+class Nightly(unittest.TestCase):
+    def setUp(self):
+        self.dbc = FakeDBConn()
+
+    def test_dont_create_scheduler_changes(self):
+        s = timed.Nightly(
+                name="tsched",
+                builderNames=['tbuild'])
+        s.parent = DummyParent(self.dbc)
+
+        d = self.dbc.addSchedulers([s])
+
+        # Add some changes
+        for i in range(10):
+            c = Change(who='just a guy', files=[], comments="")
+            d.addCallback(lambda res: self.dbc.addChangeToDatabase(c))
+
+        def runScheduler(res):
+            return s.run()
+        d.addCallback(runScheduler)
+
+        def checkTables(res):
+            # Check that we have the number of changes we think we should have
+            self.assertEquals(len(self.dbc.changes), 10)
+
+            # Check that there are no entries in scheduler_changes
+            important, unimportant = self.dbc.classified_changes.get(s.schedulerid, ([], []))
+            self.assertEquals(len(important+unimportant), 0)
+        d.addCallback(checkTables)
+        return d
+
+    def test_create_scheduler_changes(self):
+        s = timed.Nightly(
+                name="tsched",
+                builderNames=['tbuild'],
+                onlyIfChanged=True)
+        s.parent = DummyParent(self.dbc)
+
+        d = self.dbc.addSchedulers([s])
+
+        # Add some changes
+        for i in range(10):
+            c = Change(who='just a guy', files=[], comments="")
+            d.addCallback(lambda res: self.dbc.addChangeToDatabase(c))
+
+        def runScheduler(res):
+            return s.run()
+        d.addCallback(runScheduler)
+
+        def checkTables(res):
+            # Check that we have the number of changes we think we should have
+            self.assertEquals(len(self.dbc.changes), 10)
+
+            # Check that there are entries in scheduler_changes
+            important, unimportant = self.dbc.classified_changes.get(s.schedulerid, ([], []))
+            self.assertEquals(len(important+unimportant), 10)
+        d.addCallback(checkTables)
+        return d
+
+    def test_expire_old_scheduler_changes(self):
+        s = timed.Nightly(
+                name="tsched",
+                builderNames=['tbuild'],
+                )
+        s.parent = DummyParent(self.dbc)
+
+        # Hack the scheduler so that it always runs
+        def _check_timer(t):
+            now = time.time()
+            s._maybe_start_build(t)
+            s.update_last_build(t, now)
+
+            # reschedule for the next timer
+            return now + 10
+        s._check_timer = _check_timer
+
+        d = self.dbc.addSchedulers([s])
+
+        # Add a changes
+        c = Change(who='just a guy', files=[], comments="")
+        d.addCallback(lambda res: self.dbc.addChangeToDatabase(c))
+
+        # Add some dummy scheduler_changes
+        def addSchedulerChanges(res):
+            for i in range(100):
+                self.dbc.classified_changes.setdefault(s.schedulerid, ([], []))[0].append(c)
+        d.addCallback(addSchedulerChanges)
+
+        def runScheduler(res):
+            return s.run()
+        d.addCallback(runScheduler)
+
+        def checkTables(res):
+            # Check that there are no entries in scheduler_changes
+            important, unimportant = self.dbc.classified_changes.get(s.schedulerid, ([], []))
+            self.assertEquals(len(important+unimportant), 0)
+        d.addCallback(checkTables)
+        return d

master/buildbot/util/__init__.py

@@ -113,6 +113,9 @@ def add(self, id, thing):
         self._cached_ids.append(id)
     __setitem__ = add
 
+    def setMaxSize(self, max_size):
+        self._max_size = max_size
+
 threadable.synchronize(LRUCache)
 
 

master/docs/cfg-global.texinfo

@@ -190,13 +190,15 @@ c['buildHorizon'] = 100
 c['eventHorizon'] = 50
 c['logHorizon'] = 40
 c['buildCacheSize'] = 15
+c['changeCacheSize'] = 10000
 @end example
 
 @bcindex c['logHorizon']
 @bcindex c['buildCacheSize']
 @bcindex c['changeHorizon']
 @bcindex c['buildHorizon']
 @bcindex c['eventHorizon']
+@bcindex c['changeCacheSize']
 
 Buildbot stores historical information on disk in the form of "Pickle" files
 and compressed logfiles.  In a large installation, these can quickly consume
@@ -217,11 +219,18 @@ than @code{logHorizon} but not older than @code{buildHorizon} will maintain
 their overall status and the status of each step, but the logfiles will be
 deleted.
 
-Finally, the @code{buildCacheSize} gives the number of builds for each builder
+The @code{buildCacheSize} gives the number of builds for each builder
 which are cached in memory.  This number should be larger than the number of
 builds required for commonly-used status displays (the waterfall or grid
 views), so that those displays do not miss the cache on a refresh.
 
+Finally, the @code{changeCacheSize} gives the number of changes to cache in
+memory.  This should be larger than the number of changes that typically arrive
+in the span of a few minutes, otherwise your schedulers will be reloading
+changes from the database every time they run.  For distributed version control
+systems, like git or hg, several thousand changes may arrive at once, so
+setting @code{changeCacheSize} to something like 10,000 isn't unreasonable.
+
 @node Merging BuildRequests
 @subsection Merging BuildRequests
 

slave/buildslave/pbutil.py

@@ -5,7 +5,7 @@
 from twisted.spread import pb
 
 from twisted.spread.pb import PBClientFactory
-from twisted.internet import protocol
+from twisted.internet import protocol, reactor
 from twisted.python import log
 
 class ReconnectingPBClientFactory(PBClientFactory,
@@ -96,3 +96,4 @@ def failedToGetPerspective(self, why):
         # probably authorization
         self.stopTrying() # logging in harder won't help
         log.err(why)
+        reactor.stop()