From 1f3304c5153e01988b8f4493875b6489e93f76d0 Mon Sep 17 00:00:00 2001 From: Ben Martin Date: Mon, 14 Dec 2015 15:28:17 -0600 Subject: [PATCH] Print min_part_hours lockout time remaining swift-ring-builder currently only displays min_part_hours and not the amount of time remaining before a rebalance can occur. This information is readily available and has been displayed as a quality of life improvement. Additionally, a bug where the time since the last rebalance was always updated when rebalance was called regardless of if any partitions were reassigned. This can lead to partitions being unable to be reassigned as they never age according to the time since last rebalance. Change-Id: Ie0e2b5e25140cbac7465f31a26a4998beb3892e9 Closes-Bug: #1526017 --- swift/cli/ringbuilder.py | 16 ++++++-- swift/common/ring/builder.py | 13 ++++++- test/unit/cli/test_ringbuilder.py | 61 ++++++++++++++++++++++++++++++- 3 files changed, 84 insertions(+), 6 deletions(-) diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index 192a788518..072be037ad 100755 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -25,6 +25,7 @@ from os.path import basename, abspath, dirname, exists, join as pathjoin from sys import argv as sys_argv, exit, stderr, stdout from textwrap import wrap from time import time +from datetime import timedelta import optparse import math @@ -444,7 +445,9 @@ swift-ring-builder builder.parts, builder.replicas, regions, zones, dev_count, balance, dispersion_trailer)) print('The minimum number of hours before a partition can be ' - 'reassigned is %s' % builder.min_part_hours) + 'reassigned is %s (%s remaining)' % ( + builder.min_part_hours, + timedelta(seconds=builder.min_part_seconds_left))) print('The overload factor is %0.2f%% (%.6f)' % ( builder.overload * 100, builder.overload)) if builder.devs: @@ -787,6 +790,14 @@ swift-ring-builder 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 try: last_balance = builder.get_balance() @@ -802,8 +813,7 @@ swift-ring-builder rebalance [options] exit(EXIT_ERROR) 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) + 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 diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index 7629bbb900..193302d6e8 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -139,6 +139,12 @@ class RingBuilder(object): finally: self.logger.disabled = True + @property + def min_part_seconds_left(self): + """Get the total seconds until a rebalance can be performed""" + elapsed_seconds = int(time() - self._last_part_moves_epoch) + return max((self.min_part_hours * 3600) - elapsed_seconds, 0) + def weight_of_one_part(self): """ Returns the weight of each partition as calculated from the @@ -729,11 +735,12 @@ class RingBuilder(object): def pretend_min_part_hours_passed(self): """ Override min_part_hours by marking all partitions as having been moved - 255 hours ago. This can be used to force a full rebalance on the next - call to rebalance. + 255 hours ago and last move epoch to 'the beginning of time'. This can + be used to force a full rebalance on the next call to rebalance. """ for part in range(self.parts): self._last_part_moves[part] = 0xff + self._last_part_moves_epoch = 0 def get_part_devices(self, part): """ @@ -835,6 +842,8 @@ class RingBuilder(object): more recently than min_part_hours. """ elapsed_hours = int(time() - self._last_part_moves_epoch) / 3600 + if elapsed_hours <= 0: + return for part in range(self.parts): # The "min(self._last_part_moves[part] + elapsed_hours, 0xff)" # which was here showed up in profiling, so it got inlined. diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index 25200b35a9..88e081ee85 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -1739,7 +1739,7 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): "64 partitions, 3.000000 replicas, 4 regions, 4 zones, " \ "4 devices, 100.00 balance, 0.00 dispersion\n" \ "The minimum number of hours before a partition can be " \ - "reassigned is 1\n" \ + "reassigned is 1 (0:00:00 remaining)\n" \ "The overload factor is 0.00%% (0.000000)\n" \ "Devices: id region zone ip address port " \ "replication ip replication port name weight " \ @@ -1796,6 +1796,7 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): ring = RingBuilder.load(self.tmpfile) ring.set_dev_weight(3, 0.0) ring.rebalance() + ring.pretend_min_part_hours_passed() ring.remove_dev(3) ring.save(self.tmpfile) @@ -1806,6 +1807,64 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): self.assertTrue(ring.validate()) self.assertEqual(ring.devs[3], None) + def test_rebalance_resets_time_remaining(self): + self.create_sample_ring() + ring = RingBuilder.load(self.tmpfile) + + time_path = 'swift.common.ring.builder.time' + argv = ["", self.tmpfile, "rebalance", "3"] + time = 0 + + # first rebalance, should have 1 hour left before next rebalance + time += 3600 + with mock.patch(time_path, return_value=time): + self.assertEqual(ring.min_part_seconds_left, 0) + self.assertRaises(SystemExit, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + self.assertEqual(ring.min_part_seconds_left, 3600) + + # min part hours passed, change ring and save for rebalance + ring.set_dev_weight(0, ring.devs[0]['weight'] * 2) + ring.save(self.tmpfile) + + # second rebalance, should have 1 hour left + time += 3600 + with mock.patch(time_path, return_value=time): + self.assertEqual(ring.min_part_seconds_left, 0) + self.assertRaises(SystemExit, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + self.assertTrue(ring.min_part_seconds_left, 3600) + + 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, + 'ip': '127.0.0.1', 'port': 6010, 'device': 'sda1'}) + ring.add_dev({'id': 1, 'region': 0, 'zone': 0, 'weight': 1, + 'ip': '127.0.0.1', 'port': 6020, 'device': 'sdb1'}) + ring.add_dev({'id': 2, 'region': 0, 'zone': 0, 'weight': 1, + 'ip': '127.0.0.1', 'port': 6030, 'device': 'sdc1'}) + + time_path = 'swift.common.ring.builder.time' + argv = ["", self.tmpfile, "rebalance", "3"] + + with mock.patch(time_path, return_value=0): + ring.rebalance() + ring.save(self.tmpfile) + + # min part hours not passed + with mock.patch(time_path, return_value=(3600 * 0.6)): + self.assertRaises(SystemExit, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + self.assertEqual(ring.min_part_seconds_left, 3600 * 0.4) + + ring.save(self.tmpfile) + + # min part hours passed, no partitions need to be moved + with mock.patch(time_path, return_value=(3600 * 1.5)): + self.assertRaises(SystemExit, ringbuilder.main, argv) + ring = RingBuilder.load(self.tmpfile) + self.assertEqual(ring.min_part_seconds_left, 0) + def test_rebalance_with_seed(self): self.create_sample_ring() # Test rebalance using explicit seed parameter