Fix buildset completion

remove db.cancel_buildrequests and monitor buildsets for completion
every time a buildrequest is completed

master/buildbot/db/connector.py

@@ -21,7 +21,6 @@
 
 from buildbot import util
 from buildbot.util import collections as bbcollections
-from buildbot.status.results import SUCCESS, WARNINGS, FAILURE
 from buildbot.db import pool, model, changes, schedulers, sourcestamps
 from buildbot.db import state, buildsets, buildrequests, builds
 
@@ -230,68 +229,6 @@ def _runInteraction_done(self, res, start, t): # TODO: remove
         self._end_operation(t)
         return res
 
-    # used by BuildRequestControl.cancel and Builder.cancelBuildRequest
-    def cancel_buildrequests(self, brids):
-        return self.runInteractionNow(self._txn_cancel_buildrequest, brids)
-    def _txn_cancel_buildrequest(self, t, brids):
-        # TODO: we aren't entirely sure if it'd be safe to just delete the
-        # buildrequest: what else might be waiting on it that would then just
-        # hang forever?. _check_buildset() should handle it well (an empty
-        # buildset will appear complete and SUCCESS-ful). But we haven't
-        # thought it through enough to be sure. So for now, "cancel" means
-        # "mark as complete and FAILURE".
-        while brids:
-            batch, brids = brids[:100], brids[100:]
-
-            if True:
-                now = time.time()
-                q = self.quoteq("UPDATE buildrequests"
-                                " SET complete=1, results=?, complete_at=?"
-                                " WHERE id IN " + self.parmlist(len(batch)))
-                t.execute(q, [FAILURE, now]+batch)
-            else:
-                q = self.quoteq("DELETE FROM buildrequests"
-                                " WHERE id IN " + self.parmlist(len(batch)))
-                t.execute(q, batch)
-
-            # now, does this cause any buildsets to complete?
-            q = self.quoteq("SELECT bs.id"
-                            " FROM buildsets AS bs, buildrequests AS br"
-                            " WHERE br.buildsetid=bs.id AND bs.complete=0"
-                            "  AND br.id in "
-                            + self.parmlist(len(batch)))
-            t.execute(q, batch)
-            bsids = [bsid for (bsid,) in t.fetchall()]
-            for bsid in bsids:
-                self._check_buildset(t, bsid, now)
-
-
-    def _check_buildset(self, t, bsid, now):
-        q = self.quoteq("SELECT br.complete,br.results"
-                        " FROM buildsets AS bs, buildrequests AS br"
-                        " WHERE bs.complete=0"
-                        "  AND br.buildsetid=bs.id AND bs.id=?")
-        t.execute(q, (bsid,))
-        results = t.fetchall()
-        is_complete = True
-        bs_results = SUCCESS
-        for (complete, r) in results:
-            if not complete:
-                # still waiting
-                is_complete = False
-            # mark the buildset as a failure if anything worse than
-            # WARNINGS resulted from any one of the buildrequests
-            if r not in (SUCCESS, WARNINGS):
-                bs_results = FAILURE
-        if is_complete:
-            # they were all successful
-            q = self.quoteq("UPDATE buildsets"
-                            " SET complete=1, complete_at=?, results=?"
-                            " WHERE id=?")
-            t.execute(q, (now, bs_results, bsid))
-            # notify the master
-            self.master.buildsetComplete(bsid, bs_results)
-
     def doCleanup(self):
         """
         Perform any periodic database cleanup tasks.

master/buildbot/master.py

@@ -40,6 +40,7 @@
 from buildbot.schedulers.base import isScheduler
 from buildbot.process.botmaster import BotMaster
 from buildbot.process import debug
+from buildbot.status.results import SUCCESS, WARNINGS, FAILURE
 
 ########################################
 
@@ -781,13 +782,46 @@ def subscribeToBuildsets(self, callback):
         """
         return self._new_buildset_subs.subscribe(callback)
 
-    def buildsetComplete(self, bsid, result):
+    @defer.deferredGenerator
+    def maybeBuildsetComplete(self, bsid):
         """
-        Notifies the master that the given buildset with ID C{bsid} is
-        complete, with result C{result}.
+        Instructs the master to check whether the buildset is complete,
+        and notify appropriately if it is.
+
+        Note that buildset completions are only reported on the master
+        on which the last build request completes.
         """
-        # note that buildset completions are only reported on this master
-        self._complete_buildset_subs.deliver(bsid, result)
+        wfd = defer.waitForDeferred(
+            self.db.buildrequests.getBuildRequests(bsid=bsid, complete=False))
+        yield wfd
+        brdicts = wfd.getResult()
+
+        # if there are incomplete buildrequests, bail out
+        if brdicts:
+            return
+
+        wfd = defer.waitForDeferred(
+            self.db.buildrequests.getBuildRequests(bsid=bsid))
+        yield wfd
+        brdicts = wfd.getResult()
+
+        # figure out the overall results of the buildset
+        cumulative_results = SUCCESS
+        for brdict in brdicts:
+            if brdict['results'] not in (SUCCESS, WARNINGS):
+                cumulative_results = FAILURE
+
+        # mark it as completed in the database
+        wfd = defer.waitForDeferred(
+            self.db.buildsets.completeBuildset(bsid, cumulative_results))
+        yield wfd
+        wfd.getResult()
+
+        # and deliver to any listeners
+        self._buildsetComplete(bsid, cumulative_results)
+
+    def _buildsetComplete(self, bsid, results):
+        self._complete_buildset_subs.deliver(bsid, results)
 
     def subscribeToBuildsetCompletions(self, callback):
         """

master/buildbot/process/builder.py

@@ -213,9 +213,6 @@ def getOldestRequestTime(self):
         else:
             yield None
 
-    def cancelBuildRequest(self, brid): # TODO: kill
-        return self.db.cancel_buildrequests([brid])
-
     def consumeTheSoulOfYourPredecessor(self, old):
         """Suck the brain out of an old Builder.
 
@@ -559,13 +556,24 @@ def buildFinished(self, build, sb, bids):
             brids = [br.id for br in build.requests]
             db = self.master.db
             d = db.buildrequests.completeBuildRequests(brids, results)
+            d.addCallback(
+                lambda _ : self._maybeBuildsetsComplete(build.requests))
             # nothing in particular to do with this deferred, so just log it if
             # it fails..
             d.addErrback(log.err, 'while marking build requests as completed')
 
         if sb.slave:
             sb.slave.releaseLocks()
 
+    @defer.deferredGenerator
+    def _maybeBuildsetsComplete(self, requests):
+        # inform the master that we may have completed a number of buildsets
+        for br in requests:
+            wfd = defer.waitForDeferred(
+                self.master.maybeBuildsetComplete(br.bsid))
+            yield wfd
+            wfd.getResult()
+
     def _resubmit_buildreqs(self, build):
         brids = [br.id for br in build.requests]
         return self.db.buildrequests.unclaimBuildRequests(brids)

master/buildbot/process/buildrequest.py

@@ -15,9 +15,11 @@
 
 import calendar
 from zope.interface import implements
+from twisted.python import log
 from twisted.internet import defer
 from buildbot import interfaces, sourcestamp
 from buildbot.process import properties
+from buildbot.status.results import FAILURE
 
 class BuildRequest(object):
     """
@@ -139,6 +141,11 @@ def mergeReasons(self, others):
     def getSubmitTime(self):
         return self.submittedAt
 
+    def cancelBuildRequest(self):
+        d = self.db.buildrequests.completeBuildRequests([self.id], FAILURE)
+        d.addCallback(lambda _ : self.master.maybeBuildsetComplete(self.bsid))
+        return d
+
 class BuildRequestControl:
     implements(interfaces.IBuildRequestControl)
 
@@ -154,4 +161,5 @@ def unsubscribe(self, observer):
         raise NotImplementedError
 
     def cancel(self):
-        self.original_builder.cancelBuildRequest(self.brid)
+        d = self.original_request.cancelBuildRequest()
+        d.addErrback(log.err, 'while cancelling build request')

master/buildbot/test/unit/test_master.py

@@ -84,7 +84,7 @@ def test_buildset_completion_subscription(self):
         sub = self.master.subscribeToBuildsetCompletions(cb)
         self.assertIsInstance(sub, subscription.Subscription)
 
-        self.master.buildsetComplete(938593, 999)
+        self.master._buildsetComplete(938593, 999)
         # assert the notification sub was called correctly
         cb.assert_called_with(938593, 999)