openstack/swift
Code Issues Proposed changes

Fix rebalance crash post-2.1.0 upgrade.

Browse Source 
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
This commit is contained in:
Samuel Merritt 2014-10-10 17:08:06 -07:00
parent 695bb710c4
commit d9bfa06be0
2 changed files with 36 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
swift/common/ring/builder.py
View File

@ -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:

35
test/unit/common/ring/test_builder.py
View File

@ -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,