Merge "add swift-ring-builder option to recalculate dispersion"

swift/cli/ringbuilder.py

@@ -1004,6 +1004,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
@@ -1042,6 +1043,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)
@@ -1049,8 +1052,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

@@ -2278,6 +2278,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()