diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index e16afd663d..e47ad986f4 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -978,12 +978,6 @@ class RingBuilder(object): if dev_id == NONE_DEV: continue dev = self.devs[dev_id] - # the min part hour check is ignored iff a device has more - # than one replica of a part assigned to it - which would have - # only been possible on rings built with older version of code - if (self._last_part_moves[part] < self.min_part_hours and - not replicas_at_tier[dev['tiers'][-1]] > 1): - break if all(replicas_at_tier[tier] <= replica_plan[tier]['max'] for tier in dev['tiers']): @@ -996,8 +990,12 @@ class RingBuilder(object): undispersed_dev_replicas.sort( key=lambda dr: dr[0]['parts_wanted']) for dev, replica in undispersed_dev_replicas: - if self._last_part_moves[part] < self.min_part_hours: - break + # the min part hour check is ignored iff a device has more + # than one replica of a part assigned to it - which would have + # only been possible on rings built with older version of code + if (self._last_part_moves[part] < self.min_part_hours and + not replicas_at_tier[dev['tiers'][-1]] > 1): + continue dev['parts_wanted'] += 1 dev['parts'] -= 1 assign_parts[part].append(replica) diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index 110c6a4846..0f7443b2ea 100644 --- a/test/unit/common/ring/test_builder.py +++ b/test/unit/common/ring/test_builder.py @@ -1113,6 +1113,46 @@ class TestRingBuilder(unittest.TestCase): rb.rebalance() self.assertEqual(rb.get_balance(), 0) + def test_multiple_duplicate_device_assignment(self): + rb = ring.RingBuilder(4, 4, 1) + devs = [ + 'r1z1-127.0.0.1:33440/d1', + 'r1z1-127.0.0.1:33441/d2', + 'r1z1-127.0.0.1:33442/d3', + 'r1z1-127.0.0.1:33443/d4', + 'r1z1-127.0.0.2:33440/d5', + 'r1z1-127.0.0.2:33441/d6', + 'r1z1-127.0.0.2:33442/d7', + 'r1z1-127.0.0.2:33442/d8', + ] + for add_value in devs: + dev = utils.parse_add_value(add_value) + dev['weight'] = 1.0 + rb.add_dev(dev) + rb.rebalance() + rb._replica2part2dev = [ + # these are the relevant one's here + # | | | | | + # v v v v v + array('H', [0, 1, 2, 3, 3, 0, 0, 0, 4, 6, 4, 4, 4, 4, 4, 4]), + array('H', [0, 1, 3, 1, 1, 1, 1, 1, 5, 7, 5, 5, 5, 5, 5, 5]), + array('H', [0, 1, 2, 2, 2, 2, 2, 2, 4, 6, 6, 6, 6, 6, 6, 6]), + array('H', [0, 3, 2, 3, 3, 3, 3, 3, 5, 7, 7, 7, 7, 7, 7, 7]) + # ^ + # | + # this sort of thing worked already + ] + # fix up bookkeeping + new_dev_parts = defaultdict(int) + for part2dev_id in rb._replica2part2dev: + for dev_id in part2dev_id: + new_dev_parts[dev_id] += 1 + for dev in rb._iter_devs(): + dev['parts'] = new_dev_parts[dev['id']] + rb.pretend_min_part_hours_passed() + rb.rebalance() + rb.validate() + def test_region_fullness_with_balanceable_ring(self): rb = ring.RingBuilder(8, 3, 1) rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1,