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

@@ -997,6 +997,7 @@ swift-ring-builder <builder_file> dispersion <search_filter> [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 <builder_file> dispersion <search_filter> [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 <builder_file> dispersion <search_filter> [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%%' % (

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):

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

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()