Utilize buildCache in a smarter way to prevent pathological disk I/O storms.

third_party/buildbot_8_4p1/buildbot/status/builder.py

 # This file is part of Buildbot.  Buildbot is free software: you can
 # redistribute it and/or modify it under the terms of the GNU General Public
 # License as published by the Free Software Foundation, version 2.
 #
 # This program is distributed in the hope that it will be useful, but WITHOUT
 # ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
 # FOR A PARTICULAR PURPOSE.  See the GNU General Public License for more
 # details.
 #
 # You should have received a copy of the GNU General Public License along with
 # this program; if not, write to the Free Software Foundation, Inc., 51
 # Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
 #
 # Copyright Buildbot Team Members
 
 
 import weakref
 import gc
 import os, re, itertools
+import random
 from cPickle import load, dump
 
 from zope.interface import implements
 from twisted.python import log, runtime
 from twisted.persisted import styles
 from buildbot.process import metrics
 from buildbot import interfaces, util
 from buildbot.util.lru import SyncLRUCache
 from buildbot.status.event import Event
 from buildbot.status.build import BuildStatus
@@ skipping 286 matching lines @@
         for build in self.generateFinishedBuilds(num_builds=1):
             assert build and build.isFinished, \
                    'builder %s build %s is not finished' % (
                    self.getName(), build)
             return build
         return None
 
     def getCategory(self):
         return self.category
 
-    def getBuild(self, number):
+    def _resolveBuildNumber(self, number):
         if number < 0:
             number = self.nextBuildNumber + number
         if number < 0 or number >= self.nextBuildNumber:
             return None
+        return number
 
+    def _safeGetBuild(self, number):
         try:
             return self.getBuildByNumber(number)
         except IndexError:
             return None
 
+    def getBuild(self, number):
+        number = self._resolveBuildNumber(number)
+
+        if number is None:
+            return None
+
+        return self._safeGetBuild(number)
+
+    def getBuilds(self, numbers):
+        """Cache-aware method to get multiple builds.
+
+        Prevents cascading evict/load when multiple builds are requested in
+        succession: requesting build 1 evicts build 2, requesting build 2 evicts
+        build 3, etc.
+
+        We query the buildCache and load hits first, then misses.  When loading,
+        we randomize the load order to alleviate the problem when external web
+        requests load builds sequentially (they don't have access to this
+        function).
+        """
+
+        numbers = [self._resolveBuildNumber(x) for x in numbers]
+        valid_numbers = [
+                (i, n) for i, n in enumerate(numbers) if n is not None]

[iannucci, 2013/07/02 01:15:27]

Seems like this filtering could be done in the for loop below, and it would be
clearer (and would avoid looping over numbers several times)?

[Mike Stip (use stip instead), 2013/07/02 02:18:34]

Done.

+
+        # We shuffle here in case external web points access builds
+        # sequentially. Randomizing the LRU pattern will break any eviction
+        # chains.
+        random.shuffle(valid_numbers)

[iannucci, 2013/07/02 01:15:27]

comment is redundant w/ docstring

[Mike Stip (use stip instead), 2013/07/02 02:18:34]

Done.

+
+        builds = [None] * len(numbers)
+        misses = []
+        for idx, number in valid_numbers:
+            if number in self.buildCache.cache:
+                builds[idx] = self._safeGetBuild(number)
+            else:
+                misses.append((idx, number))
+
+        for idx, number in misses:
+            builds[idx] = self._safeGetBuild(number)
+
+        return builds
+
     def getEvent(self, number):
         try:
             return self.events[number]
         except IndexError:
             return None
 
     def generateFinishedBuilds(self, branches=[],
                                num_builds=None,
                                max_buildnum=None,
                                finished_before=None,
@@ skipping 262 matching lines @@
         result['basedir'] = os.path.basename(self.basedir)
         result['category'] = self.category
         result['slaves'] = self.slavenames
         #result['url'] = self.parent.getURLForThing(self)
         # TODO(maruel): Add cache settings? Do we care?
 
         # Transient
         # Collect build numbers.
         # Important: Only grab the *cached* builds numbers to reduce I/O.
         current_builds = [b.getNumber() for b in self.currentBuilds]
+
+        # Populates buildCache with last N builds.
+        buildnums = range(1, self.buildCacheSize - 1)
+        buildnums = [-i for i in buildnums]

[iannucci, 2013/07/02 01:15:27]

Would it be worth having a helper function for this 'get last N' pattern?

[Mike Stip (use stip instead), 2013/07/02 02:18:34]

well now it's one line...

+        self.getBuilds(buildnums)
+
         cached_builds = list(set(self.buildCache.cache.keys() + current_builds))
         cached_builds.sort()
         result['cachedBuilds'] = cached_builds
         result['currentBuilds'] = current_builds
         result['state'] = self.getState()[0]
         # lies, but we don't have synchronous access to this info; use
         # asDict_async instead
         result['pendingBuilds'] = 0
         return result
 
     def asDict_async(self):
         """Just like L{asDict}, but with a nonzero pendingBuilds."""
         result = self.asDict()
         d = self.getPendingBuildRequestStatuses()
         def combine(statuses):
             result['pendingBuilds'] = len(statuses)
             return result
         d.addCallback(combine)
         return d
 
     def getMetrics(self):
         return self.botmaster.parent.metrics
 
 # vim: set ts=4 sts=4 sw=4 et: