From a1ae142d5bcaf83ddde568ba4b957e23cc2b5e1c Mon Sep 17 00:00:00 2001 From: vxlinux Date: Thu, 4 Jan 2018 16:18:37 +0800 Subject: [PATCH] Merge repeat code for rebalance MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are three similar code segments in rebalance process as follows： tiers = ['cluster', 'regions', 'zones', 'servers', 'devices'] for i, tier_name in enumerate(tiers): replicas_at_tier = sum(weighted_replicas_by_tier[t] for t in weighted_replicas_by_tier if len(t) == i) if abs(self.replicas - replicas_at_tier) > 1e-10: raise exceptions.RingValidationError( '%s != %s at tier %s' % ( replicas_at_tier, self.replicas, tier_name)) I think we can encapsulate this code segment to a private function and replace those code segments with a function call Change-Id: I89439286b211f2c5ef19deffa77c202f48f07cf8 --- swift/common/ring/builder.py | 37 ++++----- test/unit/common/ring/test_builder.py | 104 ++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 24 deletions(-) diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index 717dd753a6..81759df1f9 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -1361,6 +1361,16 @@ class RingBuilder(object): def _sort_key_for(dev): return (dev['parts_wanted'], random.randint(0, 0xFFFF), dev['id']) + def _validate_replicas_at_tier(self, replicas_by_tier): + tiers = ['cluster', 'regions', 'zones', 'servers', 'devices'] + for i, tier_name in enumerate(tiers): + replicas_at_tier = sum(replicas_by_tier[t] for t in + replicas_by_tier if len(t) == i) + if abs(self.replicas - replicas_at_tier) > 1e-10: + raise exceptions.RingValidationError( + '%s != %s at tier %s' % ( + replicas_at_tier, self.replicas, tier_name)) + def _build_max_replicas_by_tier(self, bound=math.ceil): """ Returns a defaultdict of (tier: replica_count) for all tiers in the @@ -1475,14 +1485,7 @@ class RingBuilder(object): # belts & suspenders/paranoia - at every level, the sum of # weighted_replicas should be very close to the total number of # replicas for the ring - tiers = ['cluster', 'regions', 'zones', 'servers', 'devices'] - for i, tier_name in enumerate(tiers): - replicas_at_tier = sum(weighted_replicas_by_tier[t] for t in - weighted_replicas_by_tier if len(t) == i) - if abs(self.replicas - replicas_at_tier) > 1e-10: - raise exceptions.RingValidationError( - '%s != %s at tier %s' % ( - replicas_at_tier, self.replicas, tier_name)) + self._validate_replicas_at_tier(weighted_replicas_by_tier) return weighted_replicas_by_tier @@ -1585,14 +1588,7 @@ class RingBuilder(object): # belts & suspenders/paranoia - at every level, the sum of # wanted_replicas should be very close to the total number of # replicas for the ring - tiers = ['cluster', 'regions', 'zones', 'servers', 'devices'] - for i, tier_name in enumerate(tiers): - replicas_at_tier = sum(wanted_replicas[t] for t in - wanted_replicas if len(t) == i) - if abs(self.replicas - replicas_at_tier) > 1e-10: - raise exceptions.RingValidationError( - '%s != %s at tier %s' % ( - replicas_at_tier, self.replicas, tier_name)) + self._validate_replicas_at_tier(wanted_replicas) return wanted_replicas @@ -1621,14 +1617,7 @@ class RingBuilder(object): # belts & suspenders/paranoia - at every level, the sum of # target_replicas should be very close to the total number # of replicas for the ring - tiers = ['cluster', 'regions', 'zones', 'servers', 'devices'] - for i, tier_name in enumerate(tiers): - replicas_at_tier = sum(target_replicas[t] for t in - target_replicas if len(t) == i) - if abs(self.replicas - replicas_at_tier) > 1e-10: - raise exceptions.RingValidationError( - '%s != %s at tier %s' % ( - replicas_at_tier, self.replicas, tier_name)) + self._validate_replicas_at_tier(target_replicas) return target_replicas diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index 757aa2330a..0b9565b52d 100644 --- a/test/unit/common/ring/test_builder.py +++ b/test/unit/common/ring/test_builder.py @@ -446,6 +446,110 @@ class TestRingBuilder(unittest.TestCase): # maybe not *perfect*, but should be close self.assertLessEqual(balance, 1) + def test_validate_replicate_by_tier(self): + rb = ring.RingBuilder(5, 3, 0) + # replicas 3.0 and three devices with two weight 10 and one weight 5 + rb.add_dev({'id': 0, 'region': 0, 'zone': 0, 'weight': 10, + 'ip': '127.0.0.1', 'port': 6000, 'device': 'object1'}) + rb.add_dev({'id': 1, 'region': 1, 'zone': 1, 'weight': 10, + 'ip': '127.0.0.1', 'port': 6000, 'device': 'object2'}) + rb.add_dev({'id': 2, 'region': 2, 'zone': 2, 'weight': 5, + 'ip': '127.0.0.1', 'port': 6000, 'device': 'object3'}) + rb.rebalance() + + # validate weighted_replicas + weighted_replicas = \ + defaultdict(float, + {(0, 0): 1.0, (1,): 1.0, + (2,): 0.9999999999999999, + (0, 0, '127.0.0.1', 0): 1.0, + (1, 1, '127.0.0.1', 1): 1.0, + (0, 0, '127.0.0.1'): 1.0, + (1, 1, '127.0.0.1'): 1.0, (): 3.0, + (2, 2, '127.0.0.1'): 0.9999999999999999, + (2, 2): 0.9999999999999999, + (2, 2, '127.0.0.1', 2): 0.9999999999999999, + (1, 1): 1.0, (0,): 1.0 + }) + try: + rb._validate_replicas_at_tier(weighted_replicas) + except Exception as e: + self.fail('weighted_replicas is invalid for %s' % e) + + # validate wanted_replicas + wanted_replicas = \ + defaultdict(float, + {(0, 0): 1.0, (1,): 1.0, (2,): 1.0, + (0, 0, '127.0.0.1', 0): 1.0, + (1, 1, '127.0.0.1', 1): 1.0, + (0, 0, '127.0.0.1'): 1.0, + (1, 1, '127.0.0.1'): 1.0, (): 3.0, + (2, 2, '127.0.0.1'): 1.0, + (2, 2): 1.0, (2, 2, '127.0.0.1', 2): 1.0, + (1, 1): 1.0, (0,): 1.0 + }) + try: + rb._validate_replicas_at_tier(wanted_replicas) + except Exception as e: + self.fail('wanted_replicas is invalid for %s' % e) + + # validate target_replicas + target_replicas = \ + defaultdict(float, + {(0, 0): 1.0, (1,): 1.0, + (2,): 0.9999999999999999, + (0, 0, '127.0.0.1', 0): 1.0, + (1, 1, '127.0.0.1', 1): 1.0, + (0, 0, '127.0.0.1'): 1.0, + (1, 1, '127.0.0.1'): 1.0, + (2, 2, '127.0.0.1'): 0.9999999999999999, + (2, 2): 0.9999999999999999, (): 3.0, + (2, 2, '127.0.0.1', 2): 0.9999999999999999, + (1, 1): 1.0, (0,): 1.0 + }) + try: + rb._validate_replicas_at_tier(target_replicas) + except Exception as e: + self.fail('target_replicas is invalid for %s' % e) + + # set overload to 10% + rb.set_overload(0.1) + rb.rebalance() + # validate target_replicas + target_replicas = \ + defaultdict(float, + {(0, 0): 1.0, (1,): 1.0, (2,): 1.0, + (0, 0, '127.0.0.1', 0): 1.0, + (1, 1, '127.0.0.1', 1): 1.0, + (0, 0, '127.0.0.1'): 1.0, + (1, 1, '127.0.0.1'): 1.0, + (2, 2, '127.0.0.1'): 1.0, (2, 2): 1.0, (): 3.0, + (2, 2, '127.0.0.1', 2): 1.0, + (1, 1): 1.0, (0,): 1.0 + }) + try: + rb._validate_replicas_at_tier(target_replicas) + except Exception as e: + self.fail('target_replicas is invalid after overload for %s' + % e) + + # invalidate case + pseudo_replicas = \ + defaultdict(float, + {(0, 0): 1.1, (1,): 1.0, (2,): 1.0, + (0, 0, '127.0.0.1', 0): 1.0, + (1, 1, '127.0.0.1', 1): 1.0, + (0, 0, '127.0.0.1'): 1.0, + (1, 1, '127.0.0.1'): 1.0, + (2, 2, '127.0.0.1'): 1.0, + (2, 2): 1.0, (): 3.0, + (2, 2, '127.0.0.1', 2): 1.0, + (1, 1): 1.0, (0,): 1.0 + }) + with self.assertRaises(exceptions.RingValidationError) as ctx: + rb._validate_replicas_at_tier(pseudo_replicas) + self.assertEqual('3.1 != 3 at tier zones', str(ctx.exception)) + def test_multitier_partial(self): # Multitier test, nothing full rb = ring.RingBuilder(8, 3, 1)