Skip to content

Commit

Permalink
Merge pull request #2309 from tomprince/pickle-detritus
Browse files Browse the repository at this point in the history
Remove some more pickle detritus.
  • Loading branch information
tardyp committed Jul 12, 2016
2 parents 00a48de + 4489296 commit 7bc8acd
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 146 deletions.
39 changes: 31 additions & 8 deletions master/buildbot/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,17 @@ def error(error, always_raise=False):
raise ConfigErrors([error])


class ConfigWarning(Warning):
"""
Warning for deprecated configuration options.
"""


def warnDeprecated(version, msg):
# for now just log the deprecation
log.msg("NOTE: [%s and later] %s" % (version, msg))
warnings.warn(
"[%s and later] %s" % (version, msg),
category=ConfigWarning,
)


def loadConfigDict(basedir, configFileName):
Expand Down Expand Up @@ -173,7 +181,6 @@ def __init__(self):
self.titleURL = 'http://buildbot.net'
self.buildbotURL = 'http://localhost:8080/'
self.changeHorizon = None
self.eventHorizon = 50
self.logHorizon = None
self.buildHorizon = None
self.logCompressionLimit = 4 * 1024
Expand Down Expand Up @@ -227,7 +234,7 @@ def __init__(self):
_known_config_keys = set([
"buildbotURL", "buildCacheSize", "builders", "buildHorizon", "caches",
"change_source", "codebaseGenerator", "changeCacheSize", "changeHorizon",
'db', "db_poll_interval", "db_url", "eventHorizon",
'db', "db_poll_interval", "db_url",
"logCompressionLimit", "logCompressionMethod", "logEncoding",
"logHorizon", "logMaxSize", "logMaxTailSize", "manhole",
"collapseRequests", "metrics", "mq", "multiMaster", "prioritizeBuilders",
Expand Down Expand Up @@ -337,10 +344,15 @@ def copy_str_param(name, alt_key=None):
copy_str_param('buildbotURL')

copy_int_param('changeHorizon')
copy_int_param('eventHorizon')
copy_int_param('logHorizon')
copy_int_param('buildHorizon')

if 'eventHorizon' in config_dict:
warnDeprecated(
'0.9.0',
'`eventHorizon` is deprecated and will be removed in a future version.',
)

copy_int_param('logCompressionLimit')

self.logCompressionMethod = config_dict.get(
Expand Down Expand Up @@ -579,9 +591,12 @@ def mapper(b):

for builder in builders:
if builder and os.path.isabs(builder.builddir):
warnings.warn("Absolute path '%s' for builder may cause "
"mayhem. Perhaps you meant to specify workerbuilddir "
"instead.")
warnings.warn(
"Absolute path '%s' for builder may cause "
"mayhem. Perhaps you meant to specify workerbuilddir "
"instead.",
category=ConfigWarning,
)

self.builders = builders

Expand Down Expand Up @@ -852,6 +867,14 @@ def check_ports(self):
if self.workers:
error("workers are configured, but c['protocols'] not")

@property
def eventHorizon(self):
warnings.warn(
"`eventHorizon` is deprecated and will be removed in a future version.",
category=DeprecationWarning,
)
return 0


class BuilderConfig(util_config.ConfiguredMixin, WorkerAPICompatMixin):

Expand Down
2 changes: 0 additions & 2 deletions master/buildbot/process/botmaster.py
Original file line number Diff line number Diff line change
Expand Up @@ -229,8 +229,6 @@ def stopService(self):
if self.buildrequest_consumer_unclaimed:
self.buildrequest_consumer_unclaimed.stopConsuming()
self.buildrequest_consumer_unclaimed = None
for b in itervalues(self.builders):
b.builder_status.addPointEvent(["master", "shutdown"])
return service.AsyncMultiService.stopService(self)

def getLockByID(self, lockid):
Expand Down
8 changes: 0 additions & 8 deletions master/buildbot/process/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -205,8 +205,6 @@ def addLatentWorker(self, worker):
break
else:
sb = workerforbuilder.LatentWorkerForBuilder(worker, self)
self.builder_status.addPointEvent(
['added', 'latent', worker.workername])
self.workers.append(sb)
self.botmaster.maybeStartBuildsForBuilder(self.name)
deprecatedWorkerClassMethod(locals(), addLatentWorker)
Expand Down Expand Up @@ -248,7 +246,6 @@ def attached(self, worker, commands):
return d

def _attached(self, sb):
self.builder_status.addPointEvent(['connect', sb.worker.workername])
self.attaching_workers.remove(sb)
self.workers.append(sb)

Expand All @@ -261,9 +258,6 @@ def _not_attached(self, why, worker):
# TODO: remove from self.workers (except that detached() should get
# run first, right?)
log.err(why, 'worker failed to attach')
self.builder_status.addPointEvent(['failed', 'connect',
worker.workername])
# TODO: add an HTMLLogFile of the exception

def detached(self, worker):
"""This is called when the connection to the bot is lost."""
Expand All @@ -283,7 +277,6 @@ def detached(self, worker):
if wfb in self.workers:
self.workers.remove(wfb)

self.builder_status.addPointEvent(['disconnect', worker.workername])
# inform the WorkerForBuilder that their worker went away
wfb.detached()
self.updateBigStatus()
Expand Down Expand Up @@ -493,7 +486,6 @@ def getBuild(self, number):

def ping(self):
if not self.original.workers:
self.original.builder_status.addPointEvent(["ping", "no worker"])
return defer.succeed(False) # interfaces.NoWorkerError
dl = []
for w in self.original.workers:
Expand Down
39 changes: 1 addition & 38 deletions master/buildbot/process/workerforbuilder.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,6 @@ def ping(self, status=None):
d = defer.Deferred()
self.ping_watchers.append(d)
if newping:
if status:
event = status.addEvent(["pinging"])
d2 = defer.Deferred()
d2.addCallback(self._pong_status, event)
self.ping_watchers.insert(0, d2)
# I think it will make the tests run smoother if the status
# is updated before the ping completes
Ping().ping(self.worker.conn).addCallback(self._pong)

return d
Expand All @@ -145,13 +138,6 @@ def _pong(self, res):
for d in watchers:
d.callback(res)

def _pong_status(self, res, event):
if res:
event.text = ["ping", "success"]
else:
event.text = ["ping", "failed"]
event.finish()

def detached(self):
log.msg("Worker %s detached from %s" % (self.worker.workername,
self.builder_name))
Expand Down Expand Up @@ -238,32 +224,9 @@ def prepare(self, build):
return d

def substantiate(self, build):
d = self.worker.substantiate(self, build)
if not self.worker.substantiated:
event = self.builder.builder_status.addEvent(
["substantiating"])

def substantiated(res):
msg = ["substantiate", "success"]
if isinstance(res, basestring):
msg.append(res)
elif isinstance(res, (tuple, list)):
msg.extend(res)
event.text = msg
event.finish()
return res

def substantiation_failed(res):
event.text = ["substantiate", "failed"]
# TODO add log of traceback to event
event.finish()
return res
d.addCallbacks(substantiated, substantiation_failed)
return d
return self.worker.substantiate(self, build)

def ping(self, status=None):
if not self.worker.substantiated:
if status:
status.addEvent(["ping", "latent"]).finish()
return defer.succeed(True)
return AbstractWorkerForBuilder.ping(self, status)
79 changes: 4 additions & 75 deletions master/buildbot/status/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,7 @@ def cacheMiss(self, number, **kwargs):
return None

def prune(self, events_only=False):
# begin by pruning our own events
eventHorizon = self.master.config.eventHorizon
self.events = self.events[-eventHorizon:]
pass

# IBuilderStatus methods
def getName(self):
Expand Down Expand Up @@ -191,10 +189,7 @@ def getBuild(self, number, revision=None):
return None

def getEvent(self, number):
try:
return self.events[number]
except IndexError:
return None
return None

def _getBuildBranches(self, build):
return set([ss.branch
Expand Down Expand Up @@ -246,70 +241,8 @@ def generateFinishedBuilds(self, branches=None,
return

def eventGenerator(self, branches=None, categories=None, committers=None, projects=None, minTime=0):
"""This function creates a generator which will provide all of this
Builder's status events, starting with the most recent and
progressing backwards in time. """

# remember the oldest-to-earliest flow here. "next" means earlier.

# TODO: interleave build steps and self.events by timestamp.
# TODO: um, I think we're already doing that.

# TODO: there's probably something clever we could do here to
# interleave two event streams (one from self.getBuild and the other
# from self.getEvent), which would be simpler than this control flow

if branches is None:
branches = set()
else:
branches = set(branches)
if categories is None:
categories = []
if committers is None:
committers = []
if projects is None:
projects = []

eventIndex = -1
e = self.getEvent(eventIndex)

for Nb in range(1, self.nextBuildNumber + 1):
b = self.getBuild(-Nb)
if not b:
# HACK: If this is the first build we are looking at, it is
# possible it's in progress but locked before it has written a
# pickle; in this case keep looking.
if Nb == 1:
continue
break
if b.getTimes()[0] < minTime:
break
# if we were asked to filter on branches, and none of the
# sourcestamps match, skip this build
if branches and not branches & self._getBuildBranches(b):
continue
if categories and not b.getBuilder().matchesAnyTag(tags=categories):
continue
if committers and not [True for c in b.getChanges() if c.who in committers]:
continue
if projects and not b.getProperty('project') in projects:
continue
steps = b.getSteps()
for Ns in range(1, len(steps) + 1):
if steps[-Ns].started:
step_start = steps[-Ns].getTimes()[0]
while e is not None and e.getTimes()[0] > step_start:
yield e
eventIndex -= 1
e = self.getEvent(eventIndex)
yield steps[-Ns]
yield b
while e is not None:
yield e
eventIndex -= 1
e = self.getEvent(eventIndex)
if e and e.getTimes()[0] < minTime:
break
if False:
yield

def subscribe(self, receiver):
# will get builderChangedState, buildStarted, buildFinished,
Expand Down Expand Up @@ -338,8 +271,6 @@ def addEvent(self, text=None):
if text is None:
text = []
e.text = text
self.events.append(e)
self.prune(events_only=True)
return e # they are free to mangle it further

def addPointEvent(self, text=None):
Expand All @@ -351,8 +282,6 @@ def addPointEvent(self, text=None):
if text is None:
text = []
e.text = text
self.events.append(e)
self.prune(events_only=True)
return e # for consistency, but they really shouldn't touch it

def setBigState(self, state):
Expand Down
3 changes: 0 additions & 3 deletions master/buildbot/test/fake/fakemaster.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,6 @@ def newBuild(self):
def buildStarted(self, builderStatus):
pass

def addPointEvent(self, text):
pass


class FakeLogRotation(object):
rotateLength = 42
Expand Down
6 changes: 4 additions & 2 deletions master/buildbot/test/unit/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@
titleURL='http://buildbot.net',
buildbotURL='http://localhost:8080/',
changeHorizon=None,
eventHorizon=50,
logHorizon=None,
buildHorizon=None,
logCompressionLimit=4096,
Expand Down Expand Up @@ -460,7 +459,10 @@ def test_load_global_changeHorizon_none(self):
self.do_test_load_global(dict(changeHorizon=None), changeHorizon=None)

def test_load_global_eventHorizon(self):
self.do_test_load_global(dict(eventHorizon=10), eventHorizon=10)
with assertProducesWarning(
config.ConfigWarning,
message_pattern=r"`eventHorizon` is deprecated and will be removed in a future version."):
self.do_test_load_global(dict(eventHorizon=10))

def test_load_global_logHorizon(self):
self.do_test_load_global(dict(logHorizon=10), logHorizon=10)
Expand Down
4 changes: 0 additions & 4 deletions master/docs/developer/config.rst
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ described in :ref:`developer-Reconfiguration`.
The current change horizon, from :bb:cfg:`changeHorizon`.

.. py:attribute:: eventHorizon
The current event horizon, from :bb:cfg:`eventHorizon`.

.. py:attribute:: logHorizon
The current log horizon, from :bb:cfg:`logHorizon`.
Expand Down
9 changes: 3 additions & 6 deletions master/docs/manual/cfg-global.rst
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,6 @@ Data Lifetime

.. bb:cfg:: changeHorizon
.. bb:cfg:: buildHorizon
.. bb:cfg:: eventHorizon
.. bb:cfg:: logHorizon
Horizons
Expand All @@ -283,19 +282,17 @@ Horizons

c['changeHorizon'] = 200
c['buildHorizon'] = 100
c['eventHorizon'] = 50
c['logHorizon'] = 40
c['buildCacheSize'] = 15

Buildbot stores historical information on disk in the form of "Pickle" files and compressed logfiles.
Buildbot stores historical information in its database.
In a large installation, these can quickly consume disk space, yet in many cases developers never consult this historical information.

The :bb:cfg:`changeHorizon` key determines how many changes the master will keep a record of.
One place these changes are displayed is on the waterfall page.
This parameter defaults to 0, which means keep all changes indefinitely.

The :bb:cfg:`buildHorizon` specifies the minimum number of builds for each builder which should be kept on disk.
The :bb:cfg:`eventHorizon` specifies the minimum number of events to keep--events mostly describe connections and disconnections of workers, and are seldom helpful to developers.
The :bb:cfg:`buildHorizon` specifies the minimum number of builds for each builder which should be kept.
The :bb:cfg:`logHorizon` gives the minimum number of builds for which logs should be maintained; this parameter must be less than or equal to :bb:cfg:`buildHorizon`.
Builds older than :bb:cfg:`logHorizon` but not older than :bb:cfg:`buildHorizon` will maintain their overall status and the status of each step, but the logfiles will be deleted.

Expand All @@ -321,7 +318,7 @@ Caches
}

The :bb:cfg:`caches` configuration key contains the configuration for Buildbot's in-memory caches.
These caches keep frequently-used objects in memory to avoid unnecessary trips to the database or to pickle files.
These caches keep frequently-used objects in memory to avoid unnecessary trips to the database.
Caches are divided by object type, and each has a configurable maximum size.

The default size for each cache is 1, except where noted below.
Expand Down

0 comments on commit 7bc8acd

Please sign in to comment.