Skip to content

Commit

Permalink
buildrequest collapse
Browse files Browse the repository at this point in the history
 - TODO:
   - update master/docs/manual/customization.rst
  • Loading branch information
Xavier Delannoy committed Oct 14, 2014
1 parent 82dce81 commit 327bf4c
Show file tree
Hide file tree
Showing 17 changed files with 379 additions and 251 deletions.
22 changes: 11 additions & 11 deletions master/buildbot/config.py
Expand Up @@ -84,7 +84,7 @@ def __init__(self):
self.logMaxSize = None
self.logMaxTailSize = None
self.properties = properties.Properties()
self.mergeRequests = None
self.collapseRequests = None
self.codebaseGenerator = None
self.prioritizeBuilders = None
self.multiMaster = False
Expand Down Expand Up @@ -129,7 +129,7 @@ def __init__(self):
'db', "db_poll_interval", "db_url", "eventHorizon",
"logCompressionLimit", "logCompressionMethod", "logEncoding",
"logHorizon", "logMaxSize", "logMaxTailSize", "manhole",
"mergeRequests", "metrics", "mq", "multiMaster", "prioritizeBuilders",
"collapseRequests", "metrics", "mq", "multiMaster", "prioritizeBuilders",
"projectName", "projectURL", "properties", "protocols", "revlink",
"schedulers", "slavePortnum", "slaves", "status", "title", "titleURL",
"user_managers", "validation", 'www'
Expand Down Expand Up @@ -292,12 +292,12 @@ def copy_str_param(name, alt_key=None):
else:
self.properties.update(properties, filename)

mergeRequests = config_dict.get('mergeRequests')
if (mergeRequests not in (None, True, False)
and not callable(mergeRequests)):
error("mergeRequests must be a callable, True, or False")
collapseRequests = config_dict.get('collapseRequests')
if (collapseRequests not in (None, True, False)
and not callable(collapseRequests)):
error("collapseRequests must be a callable, True, or False")
else:
self.mergeRequests = mergeRequests
self.collapseRequests = collapseRequests

codebaseGenerator = config_dict.get('codebaseGenerator')
if (codebaseGenerator is not None and
Expand Down Expand Up @@ -696,7 +696,7 @@ def __init__(self, name=None, slavename=None, slavenames=None,
builddir=None, slavebuilddir=None, factory=None,
tags=None, category=None,
nextSlave=None, nextBuild=None, locks=None, env=None,
properties=None, mergeRequests=None, description=None,
properties=None, collapseRequests=None, description=None,
canStartBuild=None):

# name is required, and can't start with '_'
Expand Down Expand Up @@ -783,7 +783,7 @@ def __init__(self, name=None, slavename=None, slavenames=None,
if not isinstance(self.env, dict):
error("builder's env must be a dictionary")
self.properties = properties or {}
self.mergeRequests = mergeRequests
self.collapseRequests = collapseRequests

self.description = description

Expand All @@ -809,8 +809,8 @@ def getConfigDict(self):
rv['env'] = self.env
if self.properties:
rv['properties'] = self.properties
if self.mergeRequests is not None:
rv['mergeRequests'] = self.mergeRequests
if self.collapseRequests is not None:
rv['collapseRequests'] = self.collapseRequests
if self.description:
rv['description'] = self.description
return rv
Expand Down
13 changes: 10 additions & 3 deletions master/buildbot/data/buildsets.py
Expand Up @@ -18,7 +18,9 @@
from buildbot.data import base
from buildbot.data import sourcestamps as sourcestampsapi
from buildbot.data import types
from buildbot.process.buildrequest import BuildRequestCollapser
from buildbot.status.results import FAILURE
from buildbot.status.results import SKIPPED
from buildbot.status.results import SUCCESS
from buildbot.status.results import WARNINGS
from buildbot.util import datetime2epoch
Expand Down Expand Up @@ -140,6 +142,8 @@ def addBuildset(self, waited_for, scheduler=None, sourcestamps=[], reason=u'',
submitted_at=epoch2datetime(submitted_at),
parent_buildid=parent_buildid, parent_relationship=parent_relationship)

yield BuildRequestCollapser(self.master, brids.values()).collapse()

# get each of the sourcestamps for this buildset (sequentially)
bsdict = yield self.master.db.buildsets.getBuildset(bsid)
sourcestamps = [
Expand Down Expand Up @@ -185,11 +189,14 @@ def maybeBuildsetComplete(self, bsid, _reactor=reactor):

brdicts = yield self.master.db.buildrequests.getBuildRequests(bsid=bsid)

# figure out the overall results of the buildset: SUCCESS unless
# at least one build was not SUCCESS or WARNINGS.
# figure out the overall results of the buildset:
# - SUCCESS unless at least one build was not SUCCESS or WARNINGS.
# - Or SKIPPED
cumulative_results = SUCCESS
for brdict in brdicts:
if brdict['results'] not in (SUCCESS, WARNINGS):
if brdict['results'] == SKIPPED:
cumulative_results = SKIPPED
elif brdict['results'] not in (SUCCESS, WARNINGS):
cumulative_results = FAILURE

# get a copy of the buildset
Expand Down
4 changes: 0 additions & 4 deletions master/buildbot/process/botmaster.py
Expand Up @@ -57,10 +57,6 @@ def __init__(self, master):
# self.locks holds the real Lock instances
self.locks = {}

# self.mergeRequests is the callable override for merging build
# requests
self.mergeRequests = None

self.shuttingDown = False

self.lastSlavePortnum = None
Expand Down
30 changes: 15 additions & 15 deletions master/buildbot/process/builder.py
Expand Up @@ -540,26 +540,26 @@ def maybeStartBuild(self, slavebuilder, breqs, _reactor=reactor):
# a few utility functions to make the maybeStartBuild a bit shorter and
# easier to read

def getMergeRequestsFn(self):
"""Helper function to determine which mergeRequests function to use
from L{_mergeRequests}, or None for no merging"""
def getCollapseRequestsFn(self):
"""Helper function to determine which collapseRequests function to use
from L{_collapseRequests}, or None for no merging"""
# first, seek through builder, global, and the default
mergeRequests_fn = self.config.mergeRequests
if mergeRequests_fn is None:
mergeRequests_fn = self.master.config.mergeRequests
if mergeRequests_fn is None:
mergeRequests_fn = True
collapseRequests_fn = self.config.collapseRequests
if collapseRequests_fn is None:
collapseRequests_fn = self.master.config.collapseRequests
if collapseRequests_fn is None:
collapseRequests_fn = True

# then translate False and True properly
if mergeRequests_fn is False:
mergeRequests_fn = None
elif mergeRequests_fn is True:
mergeRequests_fn = Builder._defaultMergeRequestFn
if collapseRequests_fn is False:
collapseRequests_fn = None
elif collapseRequests_fn is True:
collapseRequests_fn = Builder._defaultCollapseRequestFn

return mergeRequests_fn
return collapseRequests_fn

def _defaultMergeRequestFn(self, req1, req2):
return req1.canBeMergedWith(req2)
def _defaultCollapseRequestFn(self, brdict1, brdict2):
return buildrequest.BuildRequest.canBeCollapsed(self.master, brdict1, brdict2)


class BuilderControl:
Expand Down
115 changes: 102 additions & 13 deletions master/buildbot/process/buildrequest.py
Expand Up @@ -16,14 +16,103 @@
import calendar

from buildbot import interfaces
from buildbot.data import resultspec
from buildbot.db import buildrequests
from buildbot.process import properties
from buildbot.status.results import FAILURE
from buildbot.status.results import SKIPPED
from twisted.internet import defer
from twisted.python import log
from zope.interface import implements


class BuildRequestCollapser(object):
# brids is a list of the new added buildrequests id
# This class is called before generated the 'new' event for the buildrequest

# Before adding new buildset/buildrequests, we must examine each unclaimed
# buildrequest.
# EG:
# 1. get the list of all unclaimed buildrequests:
# - We must exclude all buildsets which have at least 1 claimed buildrequest
# 2. For each unclaimed buildrequests, if compatible with the new request
# (sourcestamps match, except for revision) Then:
# 2.1. claim it
# 2.2. complete it with result SKIPPED

def __init__(self, master, brids):
self.master = master
self.brids = brids

@defer.inlineCallbacks
def _getUnclaimedBRDicts(self, builderid):
# As soon as a buildset contains at least one claimed buildRequest, then it's too late
# to collapse other buildRequests of this buildset.
claim_brdicts = yield self.master.data.get(('builders',
builderid,
'buildrequests'),
[resultspec.Filter('claimed',
'eq',
[True])])
ign_buildSetIDs = [brdict['buildsetid'] for brdict in claim_brdicts]

# Retrieve the list of Brdicts for all unclaimed builds which "can" be collapsed
unclaim_brdicts = yield self.master.data.get(('builders',
builderid,
'buildrequests'),
[resultspec.Filter('claimed',
'eq',
[False]),
resultspec.Filter('buildsetid',
'ne',
ign_buildSetIDs)])
# sort by submitted_at, so the first is the oldest
unclaim_brdicts.sort(key=lambda brd: brd['submitted_at'])
defer.returnValue(unclaim_brdicts)

@defer.inlineCallbacks
def collapse(self):
collapseBRs = []

for brid in self.brids:
# Get the BuildRequest object
brdict = yield self.master.data.get(('buildrequests', brid))
# Retreive the buildername
builderid = brdict['builderid']
bldrdict = yield self.master.data.get(('builders', builderid))
# Get the builder object
bldr = self.master.botmaster.builders.get(bldrdict['name'])
# Get the Collapse BuildRequest function (from the configuration)
collapseRequestsFn = bldr.getCollapseRequestsFn() if bldr else None
unclaim_brDicts = yield self._getUnclaimedBRDicts(builderid)

# short circuit if there is no merging to do
if not collapseRequestsFn or not unclaim_brDicts:
continue

for unclaim_brDict in unclaim_brDicts:
if unclaim_brDict['buildrequestid'] == brdict['buildrequestid']:
continue

canCollapse = yield collapseRequestsFn(bldr, brdict, unclaim_brDict)

if canCollapse is True:
collapseBRs.append(unclaim_brDict)

brids = [brDict['buildrequestid'] for brDict in collapseBRs]
if collapseBRs:
bsids = list(set([brDict['buildsetid'] for brDict in collapseBRs]))
# Claim the buildrequests
yield self.master.data.updates.claimBuildRequests(brids)
# complete the buildrequest with result SKIPPED.
yield self.master.data.updates.completeBuildRequests(brids,
SKIPPED)
for bsid in bsids:
# Buildset will be complete with result=SKIPPED
yield self.master.data.updates.maybeBuildsetComplete(bsid)
defer.returnValue(brids)


class TempSourceStamp(object):
# temporary fake sourcestamp; attributes are added below

Expand Down Expand Up @@ -52,7 +141,7 @@ class BuildRequest(object):
A rolled-up encapsulation of all of the data relevant to a build request.
This class is used by the C{nextBuild} and C{mergeRequests} configuration
This class is used by the C{nextBuild} and C{collapseRequests} configuration
parameters, as well as in starting a build. Construction of a BuildRequest
object is a heavyweight process involving a lot of database queries, so
it should be avoided where possible. See bug #1894.
Expand Down Expand Up @@ -150,32 +239,32 @@ def _make_br(cls, brid, brdict, master):

defer.returnValue(buildrequest)

@staticmethod
@defer.inlineCallbacks
def canBeMergedWith(self, other):
def canBeCollapsed(master, brdict1, brdict2):
"""
Returns true if both requests can be merged, via Deferred.
Returns true if both buildrequest can be merged, via Deferred.
This implements Buildbot's default merging strategy.
This implements Buildbot's default collapse strategy.
"""

# short-circuit: if these are for the same buildset, merge away
if self.bsid == other.bsid:
# short-circuit: if these are for the same buildset, collapse away
if brdict1['buildsetid'] == brdict2['buildsetid']:
defer.returnValue(True)
return

# get the buidlsets for each buildrequest
selfBuildsets = yield self.master.data.get(
('buildsets', str(self.bsid)))
otherBuildsets = yield self.master.data.get(
('buildsets', str(other.bsid)))
selfBuildsets = yield master.data.get(
('buildsets', str(brdict1['buildsetid'])))
otherBuildsets = yield master.data.get(
('buildsets', str(brdict2['buildsetid'])))

# extract sourcestamps, as dictionaries by codebase
selfSources = dict((ss['codebase'], ss)
for ss in selfBuildsets['sourcestamps'])
otherSources = dict((ss['codebase'], ss)
for ss in otherBuildsets['sourcestamps'])

# if the sets of codebases do not match, we can't merge
# if the sets of codebases do not match, we can't collapse
if set(selfSources) != set(otherSources):
defer.returnValue(False)
return
Expand All @@ -194,7 +283,7 @@ def canBeMergedWith(self, other):
if selfSS['project'] != otherSS['project']:
defer.returnValue(False)
return
# anything with a patch won't be merged
# anything with a patch won't be collapsed
if selfSS['patch'] or otherSS['patch']:
defer.returnValue(False)
return
Expand Down
21 changes: 1 addition & 20 deletions master/buildbot/process/buildrequestdistributor.py
Expand Up @@ -44,8 +44,6 @@ class BuildChooserBase(object):
# The default implementation of this class implements a default
# chooseNextBuild() that delegates out to two other functions:
# * bc.popNextBuild() - get the next (slave, breq) pair
# * bc.mergeRequests(breq) - perform a merge for this breq and return
# the list of breqs consumed by the merge (including breq itself)

def __init__(self, bldr, master):
self.bldr = bldr
Expand All @@ -62,25 +60,14 @@ def chooseNextBuild(self):
defer.returnValue((None, None))
return

breqs = yield self.mergeRequests(breq)
for b in breqs:
self._removeBuildRequest(b)

defer.returnValue((slave, breqs))
defer.returnValue((slave, [breq]))

# Must be implemented by subclass
def popNextBuild(self):
# Pick the next (slave, breq) pair; note this is pre-merge, so
# it's just one breq
raise NotImplementedError("Subclasses must implement this!")

# Must be implemented by subclass
def mergeRequests(self, breq):
# Merge the chosen breq with any other breqs that are compatible
# Returns a list of the breqs chosen (and should include the
# original breq as well!)
raise NotImplementedError("Subclasses must implement this!")

# - Helper functions that are generally useful to all subclasses -
@defer.inlineCallbacks
def _fetchUnclaimedBrdicts(self):
Expand Down Expand Up @@ -189,8 +176,6 @@ def __init__(self, bldr, master):

self.nextBuild = self.bldr.config.nextBuild

self.mergeRequestsFn = self.bldr.getMergeRequestsFn()

@defer.inlineCallbacks
def popNextBuild(self):
nextBuild = (None, None)
Expand Down Expand Up @@ -231,10 +216,6 @@ def popNextBuild(self):

defer.returnValue(nextBuild)

def mergeRequests(self, breq):
# TODO: merging is not supported in nine for the moment
return defer.succeed([breq])

@defer.inlineCallbacks
def _getNextUnclaimedBuildRequest(self):
# ensure the cache is there
Expand Down

0 comments on commit 327bf4c

Please sign in to comment.