Skip to content

Commit

Permalink
Fix rebalance crash post-2.1.0 upgrade.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
smerritt committed Oct 11, 2014
1 parent 695bb71 commit d9bfa06
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 2 deletions.
3 changes: 1 addition & 2 deletions swift/common/ring/builder.py
Expand Up @@ -347,8 +347,7 @@ def rebalance(self, seed=None):
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:
Expand Down
35 changes: 35 additions & 0 deletions test/unit/common/ring/test_builder.py
Expand Up @@ -838,6 +838,41 @@ def test_create_add_dev_add_replica_rebalance(self):
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,
Expand Down

0 comments on commit d9bfa06

Please sign in to comment.