swift-ring-builder can't remove a device with zero weight
If a device with 0 weight is tried to remove, the following rebalance
does not write changes into builder file.
Scenario:
$ swift-ring-builder object.builder set_weight --id 1 0.00
$ swift-ring-builder object.builder rebalance
Wait for moving files out of the device id=1.
$ swift-ring-builder object.builder remove --id 1
$ swift-ring-builder object.builder rebalance
In fact, the device id=1 is not removed after rebalance (must be --force used).
Change-Id: Iad5a444023eae9882a3addd7f119ff4d18559ddd
(cherry picked from commit 71993d84e8
)
This commit is contained in:
parent
a4c1825a02
commit
eabe02cd71
|
@ -859,7 +859,7 @@ swift-ring-builder <builder_file> rebalance [options]
|
|||
devs_changed = builder.devs_changed
|
||||
try:
|
||||
last_balance = builder.get_balance()
|
||||
parts, balance = builder.rebalance(seed=get_seed(3))
|
||||
parts, balance, removed_devs = builder.rebalance(seed=get_seed(3))
|
||||
except exceptions.RingBuilderError as e:
|
||||
print '-' * 79
|
||||
print("An error has occurred during ring validation. Common\n"
|
||||
|
@ -869,10 +869,10 @@ swift-ring-builder <builder_file> rebalance [options]
|
|||
(e,))
|
||||
print '-' * 79
|
||||
exit(EXIT_ERROR)
|
||||
if not (parts or options.force):
|
||||
print 'No partitions could be reassigned.'
|
||||
print 'Either none need to be or none can be due to ' \
|
||||
'min_part_hours [%s].' % builder.min_part_hours
|
||||
if not (parts or options.force or removed_devs):
|
||||
print('No partitions could be reassigned.')
|
||||
print('Either none need to be or none can be due to '
|
||||
'min_part_hours [%s].' % builder.min_part_hours)
|
||||
exit(EXIT_WARNING)
|
||||
# If we set device's weight to zero, currently balance will be set
|
||||
# special value(MAX_BALANCE) until zero weighted device return all
|
||||
|
|
|
@ -375,7 +375,7 @@ class RingBuilder(object):
|
|||
self._initial_balance()
|
||||
self.devs_changed = False
|
||||
self._build_dispersion_graph()
|
||||
return self.parts, self.get_balance()
|
||||
return self.parts, self.get_balance(), 0
|
||||
changed_parts = 0
|
||||
self._update_last_part_moves()
|
||||
last_balance = 0
|
||||
|
@ -387,6 +387,7 @@ class RingBuilder(object):
|
|||
self._set_parts_wanted()
|
||||
self._reassign_parts(new_parts)
|
||||
changed_parts += len(new_parts)
|
||||
removed_devs = 0
|
||||
while True:
|
||||
reassign_parts = self._gather_reassign_parts()
|
||||
changed_parts += len(reassign_parts)
|
||||
|
@ -397,6 +398,7 @@ class RingBuilder(object):
|
|||
remove_dev_id = self._remove_devs.pop()['id']
|
||||
self.logger.debug("Removing dev %d", remove_dev_id)
|
||||
self.devs[remove_dev_id] = None
|
||||
removed_devs += 1
|
||||
balance = self.get_balance()
|
||||
if balance < 1 or abs(last_balance - balance) < 1 or \
|
||||
changed_parts == self.parts:
|
||||
|
@ -406,7 +408,7 @@ class RingBuilder(object):
|
|||
self.version += 1
|
||||
|
||||
changed_parts = self._build_dispersion_graph(old_replica2part2dev)
|
||||
return changed_parts, balance
|
||||
return changed_parts, balance, removed_devs
|
||||
|
||||
def _build_dispersion_graph(self, old_replica2part2dev=None):
|
||||
"""
|
||||
|
|
|
@ -1684,6 +1684,21 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin):
|
|||
err = e
|
||||
self.assertEquals(err.code, 2)
|
||||
|
||||
def test_rebalance_remove_zero_weighted_device(self):
|
||||
self.create_sample_ring()
|
||||
ring = RingBuilder.load(self.tmpfile)
|
||||
ring.set_dev_weight(1, 0.0)
|
||||
ring.rebalance()
|
||||
ring.remove_dev(1)
|
||||
ring.save(self.tmpfile)
|
||||
|
||||
# Test rebalance after remove 0 weighted device
|
||||
argv = ["", self.tmpfile, "rebalance", "3"]
|
||||
self.assertRaises(SystemExit, ringbuilder.main, argv)
|
||||
ring = RingBuilder.load(self.tmpfile)
|
||||
self.assertTrue(ring.validate())
|
||||
self.assertEqual(ring.devs[1], None)
|
||||
|
||||
def test_write_ring(self):
|
||||
self.create_sample_ring()
|
||||
argv = ["", self.tmpfile, "rebalance"]
|
||||
|
|
|
@ -275,6 +275,22 @@ class TestRingBuilder(unittest.TestCase):
|
|||
rb.rebalance()
|
||||
rb.validate()
|
||||
|
||||
def test_remove_zero_weighted(self):
|
||||
rb = ring.RingBuilder(8, 3, 0)
|
||||
rb.add_dev({'id': 0, 'device': 'd0', 'ip': '10.0.0.1',
|
||||
'port': 6002, 'weight': 1000.0, 'region': 0, 'zone': 1})
|
||||
rb.add_dev({'id': 1, 'device': 'd1', 'ip': '10.0.0.2',
|
||||
'port': 6002, 'weight': 0.0, 'region': 0, 'zone': 2})
|
||||
rb.add_dev({'id': 2, 'device': 'd2', 'ip': '10.0.0.3',
|
||||
'port': 6002, 'weight': 1000.0, 'region': 0, 'zone': 3})
|
||||
rb.add_dev({'id': 3, 'device': 'd3', 'ip': '10.0.0.1',
|
||||
'port': 6002, 'weight': 1000.0, 'region': 0, 'zone': 1})
|
||||
rb.rebalance()
|
||||
|
||||
rb.remove_dev(1)
|
||||
parts, balance, removed = rb.rebalance()
|
||||
self.assertEqual(removed, 1)
|
||||
|
||||
def test_shuffled_gather(self):
|
||||
if self._shuffled_gather_helper() and \
|
||||
self._shuffled_gather_helper():
|
||||
|
@ -332,7 +348,7 @@ class TestRingBuilder(unittest.TestCase):
|
|||
rb.add_dev({'region': 1, 'zone': 2, 'weight': 4000.0,
|
||||
'ip': '10.1.1.3', 'port': 10000, 'device': 'sdb'})
|
||||
|
||||
_, balance = rb.rebalance(seed=2)
|
||||
_, balance, _ = rb.rebalance(seed=2)
|
||||
|
||||
# maybe not *perfect*, but should be close
|
||||
self.assert_(balance <= 1)
|
||||
|
@ -717,7 +733,7 @@ class TestRingBuilder(unittest.TestCase):
|
|||
|
||||
# it's as balanced as it gets, so nothing moves anymore
|
||||
rb.pretend_min_part_hours_passed()
|
||||
parts_moved, _balance = rb.rebalance(seed=1)
|
||||
parts_moved, _balance, _removed = rb.rebalance(seed=1)
|
||||
self.assertEqual(parts_moved, 0)
|
||||
|
||||
def test_region_fullness_with_balanceable_ring(self):
|
||||
|
@ -776,7 +792,7 @@ class TestRingBuilder(unittest.TestCase):
|
|||
rb.add_dev({'id': 3, 'region': 1, 'zone': 1, 'weight': 0.25,
|
||||
'ip': '127.0.0.1', 'port': 10004, 'device': 'sda1'})
|
||||
rb.pretend_min_part_hours_passed()
|
||||
changed_parts, _balance = rb.rebalance(seed=2)
|
||||
changed_parts, _balance, _removed = rb.rebalance(seed=2)
|
||||
|
||||
# there's not enough room in r1 for every partition to have a replica
|
||||
# in it, so only 86 assignments occur in r1 (that's ~1/5 of the total,
|
||||
|
@ -824,7 +840,8 @@ class TestRingBuilder(unittest.TestCase):
|
|||
for weight in range(0, 101, 10):
|
||||
rb.set_dev_weight(5, weight)
|
||||
rb.pretend_min_part_hours_passed()
|
||||
changed_parts, _balance = rb.rebalance(seed=2)
|
||||
changed_parts, _balance, _removed = rb.rebalance(seed=2)
|
||||
rb.validate()
|
||||
moved_partitions.append(changed_parts)
|
||||
# Ensure that the second region has enough partitions
|
||||
# Otherwise there will be replicas at risk
|
||||
|
|
Loading…
Reference in New Issue