From 49de7db532ffaba9fbd3c7e912e007b9d8a36d7c Mon Sep 17 00:00:00 2001 From: Clay Gerrard Date: Fri, 29 Dec 2017 08:53:21 -0800 Subject: [PATCH] add swift-ring-builder option to recalculate dispersion Since dispersion info is cached, this can easily happen if we make changes to how dispersion info is calculated or stored (e.g. we extend the dispersion calculation to consider dispersion of all part-replicas in the related change) Related-Change-Id: Ifefff0260deac0c3e8b369a1e158686c89936686 Change-Id: I714deb9e349cd114a21ec591216a9496aaf9e0d1 --- swift/cli/ringbuilder.py | 10 +++++++++- swift/common/ring/builder.py | 2 +- swift/common/ring/utils.py | 5 +++-- test/unit/cli/test_ringbuilder.py | 22 ++++++++++++++++++++++ 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/swift/cli/ringbuilder.py b/swift/cli/ringbuilder.py index 882c484326..6669bff867 100644 --- a/swift/cli/ringbuilder.py +++ b/swift/cli/ringbuilder.py @@ -997,6 +997,7 @@ swift-ring-builder dispersion [options] Output report on dispersion. + --recalculate option will rebuild cached dispersion info and save builder --verbose option will display dispersion graph broken down by tier You can filter which tiers are evaluated to drill down using a regex @@ -1035,6 +1036,8 @@ swift-ring-builder dispersion [options] exit(EXIT_ERROR) usage = Commands.dispersion.__doc__.strip() parser = optparse.OptionParser(usage) + parser.add_option('--recalculate', action='store_true', + help='Rebuild cached dispersion info and save') parser.add_option('-v', '--verbose', action='store_true', help='Display dispersion report for tiers') options, args = parser.parse_args(argv) @@ -1042,8 +1045,13 @@ swift-ring-builder dispersion [options] search_filter = args[3] else: search_filter = None + orig_version = builder.version report = dispersion_report(builder, search_filter=search_filter, - verbose=options.verbose) + verbose=options.verbose, + recalculate=options.recalculate) + if builder.version != orig_version: + # we've already done the work, better go ahead and save it! + builder.save(builder_file) print('Dispersion is %.06f, Balance is %.06f, Overload is %0.2f%%' % ( builder.dispersion, builder.get_balance(), builder.overload * 100)) print('Required overload is %.6f%%' % ( diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index bcc79e973e..717dd753a6 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -555,7 +555,6 @@ class RingBuilder(object): {'status': finish_status, 'count': gather_count + 1}) self.devs_changed = False - self.version += 1 changed_parts = self._build_dispersion_graph(old_replica2part2dev) # clean up the cache @@ -639,6 +638,7 @@ class RingBuilder(object): parts_at_risk += max(part_risk_depth.values()) self._dispersion_graph = dispersion_graph self.dispersion = 100.0 * parts_at_risk / (self.parts * self.replicas) + self.version += 1 return changed_parts def validate(self, stats=False): diff --git a/swift/common/ring/utils.py b/swift/common/ring/utils.py index e763d2a4d6..494264ac1c 100644 --- a/swift/common/ring/utils.py +++ b/swift/common/ring/utils.py @@ -606,8 +606,9 @@ def build_dev_from_opts(opts): 'replication_port': replication_port, 'weight': opts.weight} -def dispersion_report(builder, search_filter=None, verbose=False): - if not builder._dispersion_graph: +def dispersion_report(builder, search_filter=None, + verbose=False, recalculate=False): + if recalculate or not builder._dispersion_graph: builder._build_dispersion_graph() max_allowed_replicas = builder._build_max_replicas_by_tier() worst_tier = None diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index c63df94471..42c8667d4b 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -2255,6 +2255,28 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): self.assertIn('dispersion', out.lower()) self.assertFalse(err) + def test_dispersion_command_recalculate(self): + rb = RingBuilder(8, 3, 0) + for i in range(3): + i += 1 + rb.add_dev({'region': 1, 'zone': i, 'weight': 1.0, + 'ip': '127.0.0.%d' % i, 'port': 6000, 'device': 'sda'}) + # extra device in z1 + rb.add_dev({'region': 1, 'zone': 1, 'weight': 1.0, + 'ip': '127.0.0.1', 'port': 6000, 'device': 'sdb'}) + rb.rebalance() + self.assertEqual(rb.dispersion, 16.666666666666668) + # simulate an out-of-date dispersion calculation + rb.dispersion = 50 + rb.save(self.tempfile) + old_version = rb.version + out, err = self.run_srb('dispersion') + self.assertIn('Dispersion is 50.000000', out) + out, err = self.run_srb('dispersion --recalculate') + self.assertIn('Dispersion is 16.666667', out) + rb = RingBuilder.load(self.tempfile) + self.assertEqual(rb.version, old_version + 1) + def test_use_ringfile_as_builderfile(self): mock_stdout = six.StringIO() mock_stderr = six.StringIO()