Merge "Tighten the move-one-replica test"

swift/common/ring/builder.py

@@ -1064,8 +1064,6 @@ class RingBuilder(object):
             overweight_dev_replica.sort(
                 key=lambda dr: dr[0]['parts_wanted'])
             for dev, replica in overweight_dev_replica:
-                if (not self._can_part_move(part)):
-                    break
                 if any(replica_plan[tier]['min'] <=
                        replicas_at_tier[tier] <
                        replica_plan[tier]['max']
@@ -1084,6 +1082,7 @@ class RingBuilder(object):
                 for tier in dev['tiers']:
                     replicas_at_tier[tier] -= 1
                 self._set_part_moved(part)
+                break
 
     def _gather_parts_for_balance(self, assign_parts, replica_plan):
         """
@@ -1140,8 +1139,6 @@ class RingBuilder(object):
             overweight_dev_replica.sort(
                 key=lambda dr: dr[0]['parts_wanted'])
             for dev, replica in overweight_dev_replica:
-                if (not self._can_part_move(part)):
-                    break
                 # this is the most overweight_device holding a replica of this
                 # part we don't know where it's going to end up - but we'll
                 # pick it up and hope for the best.
@@ -1153,6 +1150,7 @@ class RingBuilder(object):
                     part, replica, dev['id'])
                 self._replica2part2dev[replica][part] = NONE_DEV
                 self._set_part_moved(part)
+                break
 
     def _reassign_parts(self, reassign_parts, replica_plan):
         """

test/unit/common/ring/test_builder.py

@@ -758,22 +758,24 @@ class TestRingBuilder(unittest.TestCase):
         expected = defaultdict(int, {0: 256, 1: 256, 2: 86, 3: 85, 4: 85})
         self.assertEqual(expected, self._partition_counts(rb, key='zone'))
 
-        parts_with_moved_count = defaultdict(int)
+        zone_histogram = defaultdict(int)
         for part in range(rb.parts):
-            zones = set()
-            for replica in range(rb.replicas):
-                zones.add(rb.devs[rb._replica2part2dev[replica][part]]['zone'])
-            moved_replicas = len(zones - {0, 1})
-            parts_with_moved_count[moved_replicas] += 1
+            zones = [
+                rb.devs[rb._replica2part2dev[replica][part]]['zone']
+                for replica in range(rb.replicas)]
+            zone_histogram[tuple(sorted(zones))] += 1
 
         # We expect that every partition moved exactly one replica
-        expected = {1: 256}
-        self.assertEqual(parts_with_moved_count, expected)
+        expected = {
+            (0, 1, 2): 86,
+            (0, 1, 3): 85,
+            (0, 1, 4): 85,
+        }
+        self.assertEqual(zone_histogram, expected)
 
         # After rebalancing two more times, we expect that everything is in a
         # good state
         rb.rebalance(seed=3)
-        rb.rebalance(seed=3)
 
         self.assertEqual(0, rb.dispersion)
         # a balance of w/i a 1% isn't too bad for 3 replicas on 7