From 562f7e8906140ece51187854ed535a3ef0798a00 Mon Sep 17 00:00:00 2001 From: Pete Zaitcev Date: Wed, 28 Jan 2015 12:21:25 -0700 Subject: [PATCH] Allow set_overload to take value as percent ...and output overload as a percent like dispersion and balance. Also raise a warning if someone tries to set overload higher than 100% (unless the specifically requested a percent value great than 100). Change-Id: Id030123153ea746671a8f1ca306d4b86e903fa22 --- swift/cli/ringbuilder.py | 28 ++++++++--- test/unit/cli/test_ringbuilder.py | 77 +++++++++++++++++++++++-------- 2 files changed, 80 insertions(+), 25 deletions(-) diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index 3bf073550e..b02e33aa11 100755 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -254,7 +254,8 @@ swift-ring-builder balance, builder.dispersion) print 'The minimum number of hours before a partition can be ' \ 'reassigned is %s' % builder.min_part_hours - print 'The overload factor is %.6f' % builder.overload + print 'The overload factor is %0.2f%% (%.6f)' % ( + builder.overload * 100, builder.overload) if builder.devs: print 'Devices: id region zone ip address port ' \ 'replication ip replication port name ' \ @@ -740,7 +741,8 @@ swift-ring-builder dispersion [options] search_filter = None report = dispersion_report(builder, search_filter=search_filter, verbose=options.verbose) - print 'Dispersion is %.06f' % builder.dispersion + print 'Dispersion is %.06f, Balance is %.06f, Overload is %0.2f%%' % ( + builder.dispersion, builder.get_balance(), builder.overload * 100) if report['worst_tier']: status = EXIT_WARNING print 'Worst tier is %.06f (%s)' % (report['max_dispersion'], @@ -902,7 +904,7 @@ swift-ring-builder set_replicas def set_overload(): """ -swift-ring-builder set_overload +swift-ring-builder set_overload [%] Changes the overload factor to the given . A rebalance is needed to make the change take effect. @@ -912,22 +914,36 @@ swift-ring-builder set_overload exit(EXIT_ERROR) new_overload = argv[3] + if new_overload.endswith('%'): + percent = True + new_overload = new_overload.rstrip('%') + else: + percent = False try: new_overload = float(new_overload) except ValueError: print Commands.set_overload.__doc__.strip() - print "\"%s\" is not a valid number." % new_overload + print "%r is not a valid number." % new_overload exit(EXIT_ERROR) + if percent: + new_overload *= 0.01 if new_overload < 0: print "Overload must be non-negative." exit(EXIT_ERROR) + if new_overload > 1 and not percent: + print "!?! Warning overload is greater than 100% !?!" + status = EXIT_WARNING + else: + status = EXIT_SUCCESS + builder.set_overload(new_overload) - print 'The overload is now %.6f.' % builder.overload + print 'The overload factor is now %0.2f%% (%.6f)' % ( + builder.overload * 100, builder.overload) print 'The change will take effect after the next rebalance.' builder.save(argv[1]) - exit(EXIT_SUCCESS) + exit(status) def main(arguments=None): diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index 2fb231f9d6..ebe2a2639a 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -25,7 +25,25 @@ from swift.common import exceptions from swift.common.ring import RingBuilder -class TestCommands(unittest.TestCase): +class RunSwiftRingBuilderMixin(object): + + def run_srb(self, *argv): + mock_stdout = StringIO.StringIO() + mock_stderr = StringIO.StringIO() + + srb_args = ["", self.tempfile] + [str(s) for s in argv] + + try: + with mock.patch("sys.stdout", mock_stdout): + with mock.patch("sys.stderr", mock_stderr): + swift.cli.ringbuilder.main(srb_args) + except SystemExit as err: + if err.code not in (0, 1): # (success, warning) + raise + return (mock_stdout.getvalue(), mock_stderr.getvalue()) + + +class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): def __init__(self, *args, **kwargs): super(TestCommands, self).__init__(*args, **kwargs) @@ -38,7 +56,7 @@ class TestCommands(unittest.TestCase): "127.0.0.1R127.0.0.1", "R:6000", "_some meta data"] tmpf = tempfile.NamedTemporaryFile() - self.tmpfile = tmpf.name + self.tempfile = self.tmpfile = tmpf.name def tearDown(self): try: @@ -235,6 +253,42 @@ class TestCommands(unittest.TestCase): ring = RingBuilder.load(self.tmpfile) self.assertEqual(ring.overload, 0.0) + def test_set_overload_percent(self): + self.create_sample_ring() + argv = "set_overload 10%".split() + out, err = self.run_srb(*argv) + ring = RingBuilder.load(self.tmpfile) + self.assertEqual(ring.overload, 0.1) + self.assertTrue('10.00%' in out) + self.assertTrue('0.100000' in out) + + def test_set_overload_percent_strange_input(self): + self.create_sample_ring() + argv = "set_overload 26%%%%".split() + out, err = self.run_srb(*argv) + ring = RingBuilder.load(self.tmpfile) + self.assertEqual(ring.overload, 0.26) + self.assertTrue('26.00%' in out) + self.assertTrue('0.260000' in out) + + def test_server_overload_crazy_high(self): + self.create_sample_ring() + argv = "set_overload 10".split() + out, err = self.run_srb(*argv) + ring = RingBuilder.load(self.tmpfile) + self.assertEqual(ring.overload, 10.0) + self.assertTrue('Warning overload is greater than 100%' in out) + self.assertTrue('1000.00%' in out) + self.assertTrue('10.000000' in out) + # but it's cool if you do it on purpose + argv[-1] = '1000%' + out, err = self.run_srb(*argv) + ring = RingBuilder.load(self.tmpfile) + self.assertEqual(ring.overload, 10.0) + self.assertTrue('Warning overload is greater than 100%' not in out) + self.assertTrue('1000.00%' in out) + self.assertTrue('10.000000' in out) + def test_validate(self): self.create_sample_ring() ring = RingBuilder.load(self.tmpfile) @@ -307,12 +361,12 @@ class TestCommands(unittest.TestCase): self.assertEquals(e.code, 1) -class TestRebalanceCommand(unittest.TestCase): +class TestRebalanceCommand(unittest.TestCase, RunSwiftRingBuilderMixin): def __init__(self, *args, **kwargs): super(TestRebalanceCommand, self).__init__(*args, **kwargs) tmpf = tempfile.NamedTemporaryFile() - self.tempfile = tmpf.name + self.tempfile = self.tmpfile = tmpf.name def tearDown(self): try: @@ -320,21 +374,6 @@ class TestRebalanceCommand(unittest.TestCase): except OSError: pass - def run_srb(self, *argv): - mock_stdout = StringIO.StringIO() - mock_stderr = StringIO.StringIO() - - srb_args = ["", self.tempfile] + [str(s) for s in argv] - - try: - with mock.patch("sys.stdout", mock_stdout): - with mock.patch("sys.stderr", mock_stderr): - swift.cli.ringbuilder.main(srb_args) - except SystemExit as err: - if err.code not in (0, 1): # (success, warning) - raise - return (mock_stdout.getvalue(), mock_stderr.getvalue()) - def test_rebalance_warning_appears(self): self.run_srb("create", 8, 3, 24) # all in one machine: totally balanceable