Merge pull request #6231 from tardyp/builderSorter

simplify _defaultSorter function

master/buildbot/newsfragments/prioritize_builder.feature

@@ -0,0 +1 @@
+simplify :bb:cfg:`prioritizeBuilders` default function to make an example easier to customize

master/buildbot/process/buildrequestdistributor.py

@@ -15,11 +15,10 @@
 
 
 import copy
+import math
 import random
 from datetime import datetime
 
-from dateutil.tz import tzutc
-
 from twisted.internet import defer
 from twisted.python import log
 from twisted.python.failure import Failure
@@ -30,6 +29,7 @@
 from buildbot.util import deferwaiter
 from buildbot.util import epoch2datetime
 from buildbot.util import service
+from buildbot.util.async_sort import async_sort
 
 
 class BuildChooserBase:
@@ -369,44 +369,23 @@ def resetPendingBuildersList(new_builders):
     def _defaultSorter(self, master, builders):
         timer = metrics.Timer("BuildRequestDistributor._defaultSorter()")
         timer.start()
-        # perform an asynchronous schwarzian transform, transforming None
-        # into sys.maxint so that it sorts to the end
-
-        def xform(bldr):
-            d = defer.maybeDeferred(bldr.getOldestRequestTime)
-            d.addCallback(lambda time:
-                          (((time is None) and None or time), bldr))
-            return d
-        xformed = yield defer.gatherResults(
-            [xform(bldr) for bldr in builders])
-
-        # sort the transformed list synchronously, comparing None to the end of
-        # the list
-        def xformedKey(a):
-            """
-            Key function can be used to sort a list
-            where each list element is a tuple:
-                (datetime.datetime, Builder)
-
-            @return: a tuple of (date, builder name)
-            """
-            (date, builder) = a
-            if date is None:
-                # Choose a really big date, so that any
-                # date set to 'None' will appear at the
-                # end of the list during comparisons.
-                date = datetime.max
-                # Need to set the timezone on the date, in order
-                # to perform comparisons with other dates which
-                # have the time zone set.
-                date = date.replace(tzinfo=tzutc())
-            return (date, builder.name)
-        xformed.sort(key=xformedKey)
-
-        # and reverse the transform
-        rv = [xf[1] for xf in xformed]
+
+        @defer.inlineCallbacks
+        def key(bldr):
+            # Sort by time of oldest build request
+            time = yield bldr.getOldestRequestTime()
+            if time is None:
+                # for builders that do not have pending buildrequest, we just use large number
+                time = math.inf
+            else:
+                if isinstance(time, datetime):
+                    time = time.timestamp()
+            return (time, bldr.name)
+
+        yield async_sort(builders, key)
+
         timer.stop()
-        return rv
+        return builders
 
     @defer.inlineCallbacks
     def _sortBuilders(self, buildernames):

master/buildbot/test/unit/util/test_async_sort.py

@@ -0,0 +1,55 @@
+# 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
+
+
+from twisted.internet import defer
+from twisted.python import log
+from twisted.trial import unittest
+
+from buildbot.test.util.logging import LoggingMixin
+from buildbot.util.async_sort import async_sort
+
+
+class AsyncSort(unittest.TestCase, LoggingMixin):
+
+    def setUp(self) -> None:
+        self.setUpLogging()
+        return super().setUp()
+
+    @defer.inlineCallbacks
+    def test_sync_call(self):
+        l = ["b", "c", "a"]
+        yield async_sort(l, lambda x: x)
+        return self.assertEqual(l, ["a", "b", "c"])
+
+    @defer.inlineCallbacks
+    def test_async_call(self):
+        l = ["b", "c", "a"]
+        yield async_sort(l, defer.succeed)
+        self.assertEqual(l, ["a", "b", "c"])
+
+    @defer.inlineCallbacks
+    def test_async_fail(self):
+        l = ["b", "c", "a"]
+        self.patch(log, "err", lambda f: None)
+
+        class SortFail(Exception):
+            pass
+        with self.assertRaises(SortFail):
+            yield async_sort(l, lambda x:
+                defer.succeed(x) if x != "a" else defer.fail(SortFail("ono")))
+
+        self.assertEqual(len(self.flushLoggedErrors(SortFail)), 1)
+        self.assertEqual(l, ["b", "c", "a"])

master/buildbot/util/async_sort.py

@@ -0,0 +1,42 @@
+# Copyright Buildbot Team Members
+#
+# Permission is hereby granted, free of charge, to any person obtaining
+# a copy of this software and associated documentation files (the
+# "Software"), to deal in the Software without restriction, including
+# without limitation the rights to use, copy, modify, merge, publish,
+# distribute, sublicense, and/or sell copies of the Software, and to
+# permit persons to whom the Software is furnished to do so, subject to
+# the following conditions:
+#
+# The above copyright notice and this permission notice shall be
+# included in all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+# EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+# MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+# NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+# LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+# OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+# WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+
+
+from twisted.internet import defer
+
+
+@defer.inlineCallbacks
+def async_sort(l, key, max_parallel=10):
+    """perform an asynchronous sort with parallel run of the key algorithm
+    """
+
+    sem = defer.DeferredSemaphore(max_parallel)
+    try:
+        keys = yield defer.gatherResults(
+            [sem.run(key, i) for i in l])
+    except defer.FirstError as e:
+        raise e.subFailure.value
+
+    # Index the keys by the id of the original item in list
+    keys = {id(l[i]): v for i, v in enumerate(keys)}
+
+    # now we can sort the list in place
+    l.sort(key=lambda x: keys[id(x)])

master/docs/manual/customization.rst

@@ -191,6 +191,10 @@ the following approach might be helpful:
 
 .. code-block:: python
 
+    from buildbot.util.async_sort import async_sort
+    from twisted.internet import defer
+
+    @defer.inlineCallbacks
     def prioritizeBuilders(buildmaster, builders):
         """Prioritize builders. First, prioritize inactive builders.
         Second, consider the last time a job was completed (no job is infinite past).
@@ -200,8 +204,13 @@ the following approach might be helpful:
         def isBuilding(b):
             return bool(b.building) or bool(b.old_building)
 
-        builders.sort(key = lambda b: (isBuilding(b), b.getNewestCompleteTime(),
-                                       b.getOldestRequestTime()))
+        @defer.inlineCallbacks
+        def key(b):
+            newest_complete_time = yield b.getNewestCompleteTime()
+            oldest_request_time = yield b.getOldestRequestTime()
+            return (isBuilding(b), newest_complete_time, oldest_request_time)
+
+        async_sort(builders, key)
         return builders
 
     c['prioritizeBuilders'] = prioritizeBuilders