From c0315a89dfd13bcccbc8ddc5d57adb9afd085afc Mon Sep 17 00:00:00 2001 From: Mark Gius Date: Wed, 21 Sep 2011 13:17:50 -0700 Subject: [PATCH] Fix for bug 845952 Devices scheduled to be removed are assigned a device of 65535. When looking for parts to reassign from heavy nodes, these parts need to be skipped. Includes review suggestions Change-Id: I61f40c36509bf998834c123b0f80117ca6def3ff --- swift/common/ring/builder.py | 8 +++++++- test/unit/common/ring/test_builder.py | 26 ++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index d07c3cecf1..28be941e1e 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -440,6 +440,7 @@ class RingBuilder(object): Returns an array('I') of partitions to be reassigned by gathering them from removed devices and overweight devices. """ + removed_dev_parts = set() reassign_parts = array('I') if self._remove_devs: dev_ids = [d['id'] for d in self._remove_devs if d['parts']] @@ -450,7 +451,8 @@ class RingBuilder(object): if part2dev[part] in dev_ids: part2dev[part] = 0xffff self._last_part_moves[part] = 0 - reassign_parts.append(part) + removed_dev_parts.add(part) + start = self._last_part_gather_start / 4 + randint(0, self.parts / 2) self._last_part_gather_start = start for replica in xrange(self.replicas): @@ -459,6 +461,8 @@ class RingBuilder(object): for part in half: if self._last_part_moves[part] < self.min_part_hours: continue + if part in removed_dev_parts: + continue dev = self.devs[part2dev[part]] if dev['parts_wanted'] < 0: part2dev[part] = 0xffff @@ -466,6 +470,8 @@ class RingBuilder(object): dev['parts_wanted'] += 1 dev['parts'] -= 1 reassign_parts.append(part) + + reassign_parts.extend(removed_dev_parts) shuffle(reassign_parts) return reassign_parts diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index 77be1df634..2574ea1848 100644 --- a/test/unit/common/ring/test_builder.py +++ b/test/unit/common/ring/test_builder.py @@ -194,6 +194,32 @@ class TestRingBuilder(unittest.TestCase): counts[dev_id] = counts.get(dev_id, 0) + 1 self.assertEquals(counts[3], 256) + def test_add_rebalance_add_rebalance_delete_rebalance(self): + """ Test for https://bugs.launchpad.net/swift/+bug/845952 """ + # min_part of 0 to allow for rapid rebalancing + rb = ring.RingBuilder(8, 3, 0) + rb.add_dev({'id': 0, 'zone': 0, 'weight': 1, 'ip': '127.0.0.1', + 'port': 10000, 'device': 'sda1'}) + rb.add_dev({'id': 1, 'zone': 1, 'weight': 1, 'ip': '127.0.0.1', + 'port': 10001, 'device': 'sda1'}) + rb.add_dev({'id': 2, 'zone': 2, 'weight': 1, 'ip': '127.0.0.1', + 'port': 10002, 'device': 'sda1'}) + + rb.rebalance() + + rb.add_dev({'id': 3, 'zone': 0, 'weight': 1, 'ip': '127.0.0.1', + 'port': 10003, 'device': 'sda1'}) + rb.add_dev({'id': 4, 'zone': 1, 'weight': 1, 'ip': '127.0.0.1', + 'port': 10004, 'device': 'sda1'}) + rb.add_dev({'id': 5, 'zone': 2, 'weight': 1, 'ip': '127.0.0.1', + 'port': 10005, 'device': 'sda1'}) + + rb.rebalance() + + rb.remove_dev(1) + + rb.rebalance() + def test_validate(self): rb = ring.RingBuilder(8, 3, 1) rb.add_dev({'id': 0, 'zone': 0, 'weight': 1, 'ip': '127.0.0.1',