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

third_party/buildbot_8_4p1/buildbot/status/builder.py

Index: third_party/buildbot_8_4p1/buildbot/status/builder.py
diff --git a/third_party/buildbot_8_4p1/buildbot/status/builder.py b/third_party/buildbot_8_4p1/buildbot/status/builder.py
index 3f42eb6f7e445be7020d9cd79108f854d7abc97c..f10fd5604892c1d0cdf3fb2761d8af94aa45c6fc 100644
--- a/third_party/buildbot_8_4p1/buildbot/status/builder.py
+++ b/third_party/buildbot_8_4p1/buildbot/status/builder.py
@@ -17,6 +17,7 @@
 import weakref
 import gc
 import os, re, itertools
+import random
 from cPickle import load, dump
 
 from zope.interface import implements
@@ -323,17 +324,62 @@ class BuilderStatus(styles.Versioned):
     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]
@@ -616,6 +662,12 @@ class BuilderStatus(styles.Versioned):
         # 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