From d9bfa06be026e8acdb75fed0ae130b5c209251ce Mon Sep 17 00:00:00 2001 From: Samuel Merritt Date: Fri, 10 Oct 2014 17:08:06 -0700 Subject: [PATCH] Fix rebalance crash post-2.1.0 upgrade. If you had a builder file from an earlier version of Swift (before 6d77c37) and you attempted to rebalance it with a new version of Swift (after 6d77c37, before this commit), you could get a crash. Commit 6d77c37 changed the calculation for parts_wanted slightly (round up instead of round down), so if a rebalance is attempted without recomputing parts_wanted, then swift-ring-builder crashes. The crash only occurs if the first command run is "rebalance". Adding a device, removing a device, changing a device's weight, or changing replica count (basically, anything that gives you a reason to rebalance) will all cause parts_wanted to be recalculated, so subsequent rebalances will succeed. Change-Id: I6f7a69701f74546a00d3183aa7763e9e708408c9 --- swift/common/ring/builder.py | 3 +-- test/unit/common/ring/test_builder.py | 35 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index bad148247f..43b9288a48 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -347,8 +347,7 @@ class RingBuilder(object): last_balance = 0 new_parts, removed_part_count = self._adjust_replica2part2dev_size() changed_parts += removed_part_count - if new_parts or removed_part_count: - self._set_parts_wanted() + self._set_parts_wanted() self._reassign_parts(new_parts) changed_parts += len(new_parts) while True: diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index c8476278a4..ef524f0a93 100644 --- a/test/unit/common/ring/test_builder.py +++ b/test/unit/common/ring/test_builder.py @@ -838,6 +838,41 @@ class TestRingBuilder(unittest.TestCase): rb.rebalance() # this would crash since parts_wanted was not set rb.validate() + def test_rebalance_post_upgrade(self): + rb = ring.RingBuilder(8, 3, 1) + # 5 devices: 5 is the smallest number that does not divide 3 * 2^8, + # which forces some rounding to happen. + rb.add_dev({'id': 0, 'region': 0, 'region': 0, 'zone': 0, 'weight': 1, + 'ip': '127.0.0.1', 'port': 10000, 'device': 'sda'}) + rb.add_dev({'id': 1, 'region': 0, 'region': 0, 'zone': 0, 'weight': 1, + 'ip': '127.0.0.1', 'port': 10000, 'device': 'sdb'}) + rb.add_dev({'id': 2, 'region': 0, 'region': 0, 'zone': 0, 'weight': 1, + 'ip': '127.0.0.1', 'port': 10000, 'device': 'sdc'}) + rb.add_dev({'id': 3, 'region': 0, 'region': 0, 'zone': 0, 'weight': 1, + 'ip': '127.0.0.1', 'port': 10000, 'device': 'sdd'}) + rb.add_dev({'id': 4, 'region': 0, 'region': 0, 'zone': 0, 'weight': 1, + 'ip': '127.0.0.1', 'port': 10000, 'device': 'sde'}) + rb.rebalance() + rb.validate() + + # Older versions of the ring builder code would round down when + # computing parts_wanted, while the new code rounds up. Make sure we + # can handle a ring built by the old method. + # + # This code mimics the old _set_parts_wanted. + weight_of_one_part = rb.weight_of_one_part() + for dev in rb._iter_devs(): + if not dev['weight']: + dev['parts_wanted'] = -rb.parts * rb.replicas + else: + dev['parts_wanted'] = ( + int(weight_of_one_part * dev['weight']) - + dev['parts']) + + rb.pretend_min_part_hours_passed() + rb.rebalance() # this crashes unless rebalance resets parts_wanted + rb.validate() + def test_add_replicas_then_rebalance_respects_weight(self): rb = ring.RingBuilder(8, 3, 1) rb.add_dev({'id': 0, 'region': 0, 'region': 0, 'zone': 0, 'weight': 3,