Rebalance with min_part_seconds_left > 0

As described in bug #1558754 a rebalance after removing a device fails
if min_part_seconds_left > 0, despite the note that a rebalance should
remove partitions from removed devices on the next run.

This patch skips the early exit, and lets the builder itself handling
the rebalance. Partitions that shouldn't move now are still not moved
(except for removed devices), and there is still a warning if no
partition has been moved due to the fact that min_part_hours did not yet
pass.

A small test has been added to ensure rebalancing after removing a
device works without using the --force option (tests fails on current
master). Another test ensures that a rebalance after a recent change
(for example increasing a device's weight) does not move partitions and
still reports the former warning message.

Closes-Bug: 1558754
Change-Id: I083022d066338cbe6234bab491c7a8e8e0a7b517
This commit is contained in:
Christian Schwede 2016-06-08 09:20:26 +00:00
parent b90f2d7a23
commit 47901ea4d7
2 changed files with 64 additions and 9 deletions

View File

@ -851,15 +851,8 @@ swift-ring-builder <builder_file> rebalance [options]
handler.setFormatter(formatter)
logger.addHandler(handler)
if builder.min_part_seconds_left > 0 and not options.force:
print('No partitions could be reassigned.')
print('The time between rebalances must be at least '
'min_part_hours: %s hours (%s remaining)' % (
builder.min_part_hours,
timedelta(seconds=builder.min_part_seconds_left)))
exit(EXIT_WARNING)
devs_changed = builder.devs_changed
min_part_seconds_left = builder.min_part_seconds_left
try:
last_balance = builder.get_balance()
parts, balance, removed_devs = builder.rebalance(seed=get_seed(3))
@ -874,7 +867,13 @@ swift-ring-builder <builder_file> rebalance [options]
exit(EXIT_ERROR)
if not (parts or options.force or removed_devs):
print('No partitions could be reassigned.')
print('There is no need to do so at this time')
if min_part_seconds_left > 0:
print('The time between rebalances must be at least '
'min_part_hours: %s hours (%s remaining)' % (
builder.min_part_hours,
timedelta(seconds=builder.min_part_seconds_left)))
else:
print('There is no need to do so at this time')
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

View File

@ -25,6 +25,7 @@ import unittest
import uuid
import shlex
import shutil
import time
from swift.cli import ringbuilder
from swift.cli.ringbuilder import EXIT_SUCCESS, EXIT_WARNING, EXIT_ERROR
@ -1886,6 +1887,27 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin):
ring = RingBuilder.load(self.tmpfile)
self.assertEqual(ring.min_part_seconds_left, 3600)
def test_time_remaining(self):
self.create_sample_ring()
self.run_srb('rebalance')
now = time.time()
with mock.patch('swift.common.ring.builder.time', return_value=now):
out, err = self.run_srb('rebalance')
self.assertIn('No partitions could be reassigned', out)
self.assertIn('must be at least min_part_hours', out)
self.assertIn('1 hours (1:00:00 remaining)', out)
the_future = now + 3600
with mock.patch('swift.common.ring.builder.time',
return_value=the_future):
out, err = self.run_srb('rebalance')
self.assertIn('No partitions could be reassigned', out)
self.assertIn('There is no need to do so at this time', out)
# or you can pretend_min_part_hours_passed
self.run_srb('pretend_min_part_hours_passed')
out, err = self.run_srb('rebalance')
self.assertIn('No partitions could be reassigned', out)
self.assertIn('There is no need to do so at this time', out)
def test_rebalance_failure_does_not_reset_last_moves_epoch(self):
ring = RingBuilder(8, 3, 1)
ring.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1,
@ -1922,6 +1944,40 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin):
argv = ["", self.tmpfile, "rebalance", "--seed", "2"]
self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv)
def test_rebalance_removed_devices(self):
self.create_sample_ring()
argvs = [
["", self.tmpfile, "rebalance", "3"],
["", self.tmpfile, "remove", "d0"],
["", self.tmpfile, "rebalance", "3"]]
for argv in argvs:
self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv)
def test_rebalance_min_part_hours_not_passed(self):
self.create_sample_ring()
argvs = [
["", self.tmpfile, "rebalance", "3"],
["", self.tmpfile, "set_weight", "d0", "1000"]]
for argv in argvs:
self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv)
ring = RingBuilder.load(self.tmpfile)
last_replica2part2dev = ring._replica2part2dev
mock_stdout = six.StringIO()
argv = ["", self.tmpfile, "rebalance", "3"]
with mock.patch("sys.stdout", mock_stdout):
self.assertSystemExit(EXIT_WARNING, ringbuilder.main, argv)
expected = "No partitions could be reassigned.\n" + \
"The time between rebalances must be " + \
"at least min_part_hours: 1 hours"
self.assertTrue(expected in mock_stdout.getvalue())
# Messages can be faked, so let's assure that the partition assignment
# did not change at all, despite the warning
ring = RingBuilder.load(self.tmpfile)
self.assertEqual(last_replica2part2dev, ring._replica2part2dev)
def test_write_ring(self):
self.create_sample_ring()
argv = ["", self.tmpfile, "rebalance"]