Merge "Allow rebalance to take a seed."

4 files changed, 51 insertions, 8 deletions

diff --git a/AUTHORS b/AUTHORS
index 827068287..caca35b7f 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -56,6 +56,7 @@ Ed Leafe (ed.leafe@rackspace.com)
 Tong Li (litong01@us.ibm.com)
 Victor Lowther (victor.lowther@gmail.com)
 Zhong Yue Luo (lzyeval@gmail.com)
+Christopher MacGown (chris@pistoncloud.com)
 Dragos Manolescu (dragosm@hp.com)
 Juan J. Martinez (juan@memset.com)
 Marcelo Martins (btorch@gmail.com)
diff --git a/bin/swift-ring-builder b/bin/swift-ring-builder
index 4349c4389..d2ec782fb 100755
--- a/bin/swift-ring-builder
+++ b/bin/swift-ring-builder
@@ -473,14 +473,20 @@ swift-ring-builder <builder_file> remove <search-value> [search-value ...]
 
     def rebalance():
         """
-swift-ring-builder <builder_file> rebalance
+swift-ring-builder <builder_file> rebalance <seed>
     Attempts to rebalance the ring by reassigning partitions that haven't been
     recently reassigned.
         """
+        def get_seed(index):
+            try:
+                return argv[index]
+            except IndexError:
+                pass
+
         devs_changed = builder.devs_changed
         try:
             last_balance = builder.get_balance()
-            parts, balance = builder.rebalance()
+            parts, balance = builder.rebalance(seed=get_seed(3))
         except exceptions.RingBuilderError, e:
             print '-' * 79
             print ("An error has occurred during ring validation. Common\n"
diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py
index 09d630358..ba1f5b920 100644
--- a/swift/common/ring/builder.py
+++ b/swift/common/ring/builder.py
@@ -16,12 +16,11 @@
 import bisect
 import itertools
 import math
+import random
 import cPickle as pickle
 
-
 from array import array
 from collections import defaultdict
-from random import randint, shuffle
 from time import time
 
 from swift.common import exceptions
@@ -276,7 +275,7 @@ class RingBuilder(object):
         self.devs_changed = True
         self.version += 1
 
-    def rebalance(self):
+    def rebalance(self, seed=None):
         """
         Rebalance the ring.
 
@@ -294,6 +293,10 @@ class RingBuilder(object):
 
         :returns: (number_of_partitions_altered, resulting_balance)
         """
+
+        if seed:
+            random.seed(seed)
+
         self._ring = None
         if self._last_part_moves_epoch is None:
             self._initial_balance()
@@ -576,7 +579,10 @@ class RingBuilder(object):
         # We randomly pick a new starting point in the "circular" ring of
         # partitions to try to get a better rebalance when called multiple
         # times.
-        start = self._last_part_gather_start / 4 + randint(0, self.parts / 2)
+
+        start = self._last_part_gather_start / 4
+        start += random.randint(0, self.parts / 2)  # GRAH PEP8!!!
+
         self._last_part_gather_start = start
         for replica in xrange(self.replicas):
             part2dev = self._replica2part2dev[replica]
@@ -604,7 +610,7 @@ class RingBuilder(object):
         # it would concentrate load during failure recovery scenarios
         # (increasing risk). The "right" answer has yet to be debated to
         # conclusion, but working code wins for now.
-        shuffle(reassign_parts_list)
+        random.shuffle(reassign_parts_list)
         return reassign_parts_list
 
     def _reassign_parts(self, reassign_parts):
@@ -630,6 +636,7 @@ class RingBuilder(object):
         """
         for dev in self._iter_devs():
             dev['sort_key'] = self._sort_key_for(dev)
+
         available_devs = \
             sorted((d for d in self._iter_devs() if d['weight']),
                    key=lambda x: x['sort_key'])
@@ -720,7 +727,7 @@ class RingBuilder(object):
         # parts_wanted end up sorted above positive parts_wanted.
         return '%016x.%04x.%04x' % (
             (self.parts * self.replicas) + dev['parts_wanted'],
-            randint(0, 0xffff),
+            random.randint(0, 0xFFFF),
             dev['id'])
 
     def _build_max_replicas_by_tier(self):
diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py
index 6ffc2edc9..f58008237 100644
--- a/test/unit/common/ring/test_builder.py
+++ b/test/unit/common/ring/test_builder.py
@@ -68,6 +68,35 @@ class TestRingBuilder(unittest.TestCase):
         r4 = rb.get_ring()
         self.assert_(r3 is r4)
 
+    def test_rebalance_with_seed(self):
+        devs = [(0, 10000), (1, 10001), (2, 10002), (1, 10003)]
+        ring_builders = []
+        for n in range(3):
+            rb = ring.RingBuilder(8, 3, 1)
+            for idx, (zone, port) in enumerate(devs):
+                rb.add_dev({'id': idx, 'zone': zone, 'weight': 1,
+                            'ip': '127.0.0.1', 'port': port, 'device': 'sda1'})
+            ring_builders.append(rb)
+
+        rb0 = ring_builders[0]
+        rb1 = ring_builders[1]
+        rb2 = ring_builders[2]
+
+        r0 = rb0.get_ring()
+        self.assertTrue(rb0.get_ring() is r0)
+
+
+        rb0.rebalance()  # NO SEED
+        rb1.rebalance(seed=10)
+        rb2.rebalance(seed=10)
+
+        r1 = rb1.get_ring()
+        r2 = rb2.get_ring()
+
+        self.assertFalse(rb0.get_ring() is r0)
+        self.assertNotEquals(r0.to_dict(), r1.to_dict())
+        self.assertEquals(r1.to_dict(), r2.to_dict())
+
     def test_add_dev(self):
         rb = ring.RingBuilder(8, 3, 1)
         dev = \