diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index 463165bd21..b96e52a463 100644 --- a/swift/common/ring/builder.py +++ b/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): """ diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index 8a3aed7267..6382b2cb39 100644 --- a/test/unit/common/ring/test_builder.py +++ b/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