From 98acf42f92667fd4ed8189d8eae726a733782291 Mon Sep 17 00:00:00 2001 From: Eohyung Lee Date: Tue, 23 Apr 2013 16:09:57 +0900 Subject: [PATCH] Fix rebalance for zero weighted devices. If we set device's weight to zero, currently balance will be set special value(999.99) until zero weighted device return all its partitions. So we cannot check balance has changed. Thus we need to check balance or last_balance is special value. Change-Id: I5b7db8b8e48db0c4771c51a764bda689869817d5 Fixes: bug #1171731 --- bin/swift-ring-builder | 12 +++++++++--- swift/common/ring/builder.py | 10 ++++++---- test/unit/common/ring/test_builder.py | 7 ++++--- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/bin/swift-ring-builder b/bin/swift-ring-builder index 8187c6d571..999b759016 100755 --- a/bin/swift-ring-builder +++ b/bin/swift-ring-builder @@ -27,6 +27,7 @@ from time import time from swift.common import exceptions from swift.common.ring import RingBuilder +from swift.common.ring.builder import MAX_BALANCE from swift.common.utils import lock_parent_directory MAJOR_VERSION = 1 @@ -110,7 +111,7 @@ swift-ring-builder continue if not dev['weight']: if dev['parts']: - balance = 999.99 + balance = MAX_BALANCE else: balance = 0 else: @@ -144,7 +145,7 @@ swift-ring-builder search for dev in devs: if not dev['weight']: if dev['parts']: - balance = 999.99 + balance = MAX_BALANCE else: balance = 0 else: @@ -519,7 +520,12 @@ swift-ring-builder rebalance print 'Either none need to be or none can be due to ' \ 'min_part_hours [%s].' % builder.min_part_hours exit(EXIT_WARNING) - if not devs_changed and abs(last_balance - balance) < 1: + # If we set device's weight to zero, currently balance will be set + # special value(MAX_BALANCE) until zero weighted device return all + # its partitions. So we cannot check balance has changed. + # Thus we need to check balance or last_balance is special value. + if not devs_changed and abs(last_balance - balance) < 1 and \ + not (last_balance == MAX_BALANCE and balance == MAX_BALANCE): print 'Cowardly refusing to save rebalance as it did not change ' \ 'at least 1%.' exit(EXIT_WARNING) diff --git a/swift/common/ring/builder.py b/swift/common/ring/builder.py index e073edbeea..ca2699d58d 100644 --- a/swift/common/ring/builder.py +++ b/swift/common/ring/builder.py @@ -27,6 +27,8 @@ from swift.common import exceptions from swift.common.ring import RingData from swift.common.ring.utils import tiers_for_dev, build_tier_tree +MAX_BALANCE = 999.99 + class RingBuilder(object): """ @@ -414,9 +416,9 @@ class RingBuilder(object): if dev_usage[dev['id']]: # If a device has no weight, but has partitions, then # its overage is considered "infinity" and therefore - # always the worst possible. We show 999.99 for + # always the worst possible. We show MAX_BALANCE for # convenience. - worst = 999.99 + worst = MAX_BALANCE break continue skew = abs(100.0 * dev_usage[dev['id']] / @@ -444,8 +446,8 @@ class RingBuilder(object): if dev['parts']: # If a device has no weight, but has partitions, then its # overage is considered "infinity" and therefore always the - # worst possible. We show 999.99 for convenience. - balance = 999.99 + # worst possible. We show MAX_BALANCE for convenience. + balance = MAX_BALANCE break continue dev_balance = abs(100.0 * dev['parts'] / diff --git a/test/unit/common/ring/test_builder.py b/test/unit/common/ring/test_builder.py index 6a73176002..510b3d2b46 100644 --- a/test/unit/common/ring/test_builder.py +++ b/test/unit/common/ring/test_builder.py @@ -23,6 +23,7 @@ from mock import Mock, call as mock_call from swift.common import exceptions from swift.common import ring +from swift.common.ring.builder import MAX_BALANCE class TestRingBuilder(unittest.TestCase): @@ -763,7 +764,7 @@ class TestRingBuilder(unittest.TestCase): rb.set_dev_weight(2, 0) rb.rebalance() - self.assertEquals(rb.validate(stats=True)[1], 999.99) + self.assertEquals(rb.validate(stats=True)[1], MAX_BALANCE) # Test not all partitions doubly accounted for rb.devs[1]['parts'] -= 1 @@ -799,12 +800,12 @@ class TestRingBuilder(unittest.TestCase): # Validate that zero weight devices with no partitions don't count on # the 'worst' value. - self.assertNotEquals(rb.validate(stats=True)[1], 999.99) + self.assertNotEquals(rb.validate(stats=True)[1], MAX_BALANCE) rb.add_dev({'id': 4, 'region': 0, 'zone': 0, 'weight': 0, 'ip': '127.0.0.1', 'port': 10004, 'device': 'sda1'}) rb.pretend_min_part_hours_passed() rb.rebalance() - self.assertNotEquals(rb.validate(stats=True)[1], 999.99) + self.assertNotEquals(rb.validate(stats=True)[1], MAX_BALANCE) def test_get_part_devices(self): rb = ring.RingBuilder(8, 3, 1)