Merge "Exclude empty tiers from max_replicas"

swift/common/ring/builder.py

@@ -1134,7 +1134,8 @@ class RingBuilder(object):
 
     def _build_max_replicas_by_tier(self):
         """
-        Returns a dict of (tier: replica_count) for all tiers in the ring.
+        Returns a defaultdict of (tier: replica_count) for all tiers in the
+        ring excluding zero weight devices.
 
         There will always be a () entry as the root of the structure, whose
         replica_count will equal the ring's replica_count.
@@ -1182,7 +1183,8 @@ class RingBuilder(object):
         """
         # Used by walk_tree to know what entries to create for each recursive
         # call.
-        tier2children = build_tier_tree(self._iter_devs())
+        tier2children = build_tier_tree(d for d in self._iter_devs() if
+                                        d['weight'])
 
         def walk_tree(tier, replica_count):
             mr = {tier: replica_count}
@@ -1192,7 +1194,9 @@ class RingBuilder(object):
                     submax = math.ceil(float(replica_count) / len(subtiers))
                     mr.update(walk_tree(subtier, submax))
             return mr
-        return walk_tree((), self.replicas)
+        mr = defaultdict(float)
+        mr.update(walk_tree((), self.replicas))
+        return mr
 
     def _devs_for_part(self, part):
         """

test/unit/common/ring/test_builder.py

@@ -1467,6 +1467,70 @@ class TestRingBuilder(unittest.TestCase):
                            key=operator.itemgetter('id'))
         self.assertEqual(part_devs, [rb.devs[0], rb.devs[1]])
 
+    def test_dispersion_with_zero_weight_devices(self):
+        rb = ring.RingBuilder(8, 3.0, 0)
+        # add two devices to a single server in a single zone
+        rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1,
+                    'ip': '127.0.0.1', 'port': 10000, 'device': 'sda'})
+        rb.add_dev({'id': 1, 'region': 0, 'zone': 0, 'weight': 1,
+                    'ip': '127.0.0.1', 'port': 10000, 'device': 'sdb'})
+        # and a zero weight device
+        rb.add_dev({'id': 2, 'region': 0, 'zone': 0, 'weight': 0,
+                    'ip': '127.0.0.1', 'port': 10000, 'device': 'sdc'})
+        rb.rebalance()
+        self.assertEqual(rb.dispersion, 0.0)
+        self.assertEqual(rb._dispersion_graph, {
+            (0,): [0, 0, 0, 256],
+            (0, 0): [0, 0, 0, 256],
+            (0, 0, '127.0.0.1:10000'): [0, 0, 0, 256],
+            (0, 0, '127.0.0.1:10000', 0): [0, 128, 128, 0],
+            (0, 0, '127.0.0.1:10000', 1): [0, 128, 128, 0],
+        })
+
+    def test_dispersion_with_zero_weight_devices_with_parts(self):
+        rb = ring.RingBuilder(8, 3.0, 1)
+        # add three devices to a single server in a single zone
+        rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 1,
+                    'ip': '127.0.0.1', 'port': 10000, 'device': 'sda'})
+        rb.add_dev({'id': 1, 'region': 0, 'zone': 0, 'weight': 1,
+                    'ip': '127.0.0.1', 'port': 10000, 'device': 'sdb'})
+        rb.add_dev({'id': 2, 'region': 0, 'zone': 0, 'weight': 1,
+                    'ip': '127.0.0.1', 'port': 10000, 'device': 'sdc'})
+        rb.rebalance(seed=1)
+        self.assertEqual(rb.dispersion, 0.0)
+        self.assertEqual(rb._dispersion_graph, {
+            (0,): [0, 0, 0, 256],
+            (0, 0): [0, 0, 0, 256],
+            (0, 0, '127.0.0.1:10000'): [0, 0, 0, 256],
+            (0, 0, '127.0.0.1:10000', 0): [0, 256, 0, 0],
+            (0, 0, '127.0.0.1:10000', 1): [0, 256, 0, 0],
+            (0, 0, '127.0.0.1:10000', 2): [0, 256, 0, 0],
+        })
+        # now mark a device 2 for decom
+        rb.set_dev_weight(2, 0.0)
+        # we'll rebalance but can't move any parts
+        rb.rebalance(seed=1)
+        # zero weight tier has one copy of *every* part
+        self.assertEqual(rb.dispersion, 100.0)
+        self.assertEqual(rb._dispersion_graph, {
+            (0,): [0, 0, 0, 256],
+            (0, 0): [0, 0, 0, 256],
+            (0, 0, '127.0.0.1:10000'): [0, 0, 0, 256],
+            (0, 0, '127.0.0.1:10000', 0): [0, 256, 0, 0],
+            (0, 0, '127.0.0.1:10000', 1): [0, 256, 0, 0],
+            (0, 0, '127.0.0.1:10000', 2): [0, 256, 0, 0],
+        })
+        rb.pretend_min_part_hours_passed()
+        rb.rebalance(seed=3)
+        self.assertEqual(rb.dispersion, 0.0)
+        self.assertEqual(rb._dispersion_graph, {
+            (0,): [0, 0, 0, 256],
+            (0, 0): [0, 0, 0, 256],
+            (0, 0, '127.0.0.1:10000'): [0, 0, 0, 256],
+            (0, 0, '127.0.0.1:10000', 0): [0, 128, 128, 0],
+            (0, 0, '127.0.0.1:10000', 1): [0, 128, 128, 0],
+        })
+
 
 if __name__ == '__main__':
     unittest.main()