diff --git a/swift/common/ring/ring.py b/swift/common/ring/ring.py index fa1ad9d8a6..08f47c7ae9 100644 --- a/swift/common/ring/ring.py +++ b/swift/common/ring/ring.py @@ -35,6 +35,12 @@ from swift.common.utils import hash_path, validate_configuration from swift.common.ring.utils import tiers_for_dev +def calc_replica_count(replica2part2dev_id): + base = len(replica2part2dev_id) - 1 + extra = 1.0 * len(replica2part2dev_id[-1]) / len(replica2part2dev_id[0]) + return base + extra + + class RingData(object): """Partitioned consistent hashing ring data (used for serialization).""" @@ -49,6 +55,11 @@ class RingData(object): if dev is not None: dev.setdefault("region", 1) + @property + def replica_count(self): + """Number of replicas (full or partial) used in the ring.""" + return calc_replica_count(self._replica2part2dev_id) + @classmethod def deserialize_v1(cls, gz_file, metadata_only=False): """ @@ -285,7 +296,7 @@ class Ring(object): @property def replica_count(self): """Number of replicas (full or partial) used in the ring.""" - return len(self._replica2part2dev_id) + return calc_replica_count(self._replica2part2dev_id) @property def partition_count(self): diff --git a/swift/common/storage_policy.py b/swift/common/storage_policy.py index dc79a6414b..75650e5bd7 100644 --- a/swift/common/storage_policy.py +++ b/swift/common/storage_policy.py @@ -617,13 +617,13 @@ class ECStoragePolicy(BaseStoragePolicy): considering the number of nodes in the primary list from the ring. """ - configured_fragment_count = len(ring_data._replica2part2dev_id) + configured_fragment_count = ring_data.replica_count required_fragment_count = \ (self.ec_n_unique_fragments) * self.ec_duplication_factor if configured_fragment_count != required_fragment_count: raise RingLoadError( 'EC ring for policy %s needs to be configured with ' - 'exactly %d replicas. Got %d.' % ( + 'exactly %d replicas. Got %s.' % ( self.name, required_fragment_count, configured_fragment_count)) diff --git a/test/unit/cli/test_ringbuilder.py b/test/unit/cli/test_ringbuilder.py index c1603c5500..0efac59911 100644 --- a/test/unit/cli/test_ringbuilder.py +++ b/test/unit/cli/test_ringbuilder.py @@ -133,7 +133,8 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): msg += '%3d: %s\n' % (i, line) self.fail(msg) - def create_sample_ring(self, part_power=6, overload=None, empty=False): + def create_sample_ring(self, part_power=6, replicas=3, overload=None, + empty=False): """ Create a sample ring with four devices @@ -149,7 +150,7 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): except OSError: pass - ring = RingBuilder(part_power, 3, 1) + ring = RingBuilder(part_power, replicas, 1) if overload is not None: ring.set_overload(overload) if not empty: @@ -2205,6 +2206,25 @@ class TestCommands(unittest.TestCase, RunSwiftRingBuilderMixin): exp_results = {'valid_exit_codes': [2]} self.run_srb(*argv, exp_results=exp_results) + def test_write_builder_fractional_replicas(self): + # Test builder file already exists + self.create_sample_ring(replicas=1.2) + argv = ["", self.tmpfile, "rebalance"] + self.assertSystemExit(EXIT_SUCCESS, ringbuilder.main, argv) + + ring_file = os.path.join(os.path.dirname(self.tmpfile), + os.path.basename(self.tmpfile) + ".ring.gz") + os.remove(self.tmpfile) # loses file... + + argv = ["", ring_file, "write_builder", "24"] + self.assertIsNone(ringbuilder.main(argv)) + + # Note that we've picked up an extension + builder = RingBuilder.load(self.tmpfile + '.builder') + # Note that this is different from the original! But it more-closely + # reflects the reality that we have an extra replica for 12 of 64 parts + self.assertEqual(builder.replicas, 1.1875) + def test_write_builder_after_device_removal(self): # Test regenerating builder file after having removed a device # and lost the builder file diff --git a/test/unit/common/ring/test_ring.py b/test/unit/common/ring/test_ring.py index 091cdeb705..5c68328bb8 100644 --- a/test/unit/common/ring/test_ring.py +++ b/test/unit/common/ring/test_ring.py @@ -163,6 +163,21 @@ class TestRingData(unittest.TestCase): self.assertEqual(oct(stat.S_IMODE(os.stat(ring_fname).st_mode)), '0644') + def test_replica_count(self): + rd = ring.RingData( + [[0, 1, 0, 1], [0, 1, 0, 1]], + [{'id': 0, 'zone': 0, 'ip': '10.1.1.0', 'port': 7000}, + {'id': 1, 'zone': 1, 'ip': '10.1.1.1', 'port': 7000}], + 30) + self.assertEqual(rd.replica_count, 2) + + rd = ring.RingData( + [[0, 1, 0, 1], [0, 1, 0]], + [{'id': 0, 'zone': 0, 'ip': '10.1.1.0', 'port': 7000}, + {'id': 1, 'zone': 1, 'ip': '10.1.1.1', 'port': 7000}], + 30) + self.assertEqual(rd.replica_count, 1.75) + class TestRing(TestRingBase): @@ -217,6 +232,11 @@ class TestRing(TestRingBase): mock.patch.object(utils, 'SWIFT_CONF_FILE', ''): self.assertRaises(SystemExit, ring.Ring, self.testdir, 'whatever') + def test_replica_count(self): + self.assertEqual(self.ring.replica_count, 3) + self.ring._replica2part2dev_id.append([0]) + self.assertEqual(self.ring.replica_count, 3.25) + def test_has_changed(self): self.assertFalse(self.ring.has_changed()) os.utime(self.testgz, (time() + 60, time() + 60)) diff --git a/test/unit/common/test_storage_policy.py b/test/unit/common/test_storage_policy.py index 615b346fe1..fc9b3c788e 100644 --- a/test/unit/common/test_storage_policy.py +++ b/test/unit/common/test_storage_policy.py @@ -1299,25 +1299,31 @@ class TestStoragePolicies(unittest.TestCase): ec_ndata=4, ec_nparity=2, ec_duplication_factor=2) ] - actual_load_ring_replicas = [8, 10, 7, 11] policies = StoragePolicyCollection(test_policies) class MockRingData(object): def __init__(self, num_replica): - self._replica2part2dev_id = [0] * num_replica + self.replica_count = num_replica - for policy, ring_replicas in zip(policies, actual_load_ring_replicas): - with mock.patch('swift.common.ring.ring.RingData.load', - return_value=MockRingData(ring_replicas)): - necessary_replica_num = \ - policy.ec_n_unique_fragments * policy.ec_duplication_factor - with mock.patch( - 'swift.common.ring.ring.validate_configuration'): - msg = 'EC ring for policy %s needs to be configured with ' \ - 'exactly %d replicas.' % \ - (policy.name, necessary_replica_num) - self.assertRaisesWithMessage(RingLoadError, msg, - policy.load_ring, 'mock') + def do_test(actual_load_ring_replicas): + for policy, ring_replicas in zip(policies, + actual_load_ring_replicas): + with mock.patch('swift.common.ring.ring.RingData.load', + return_value=MockRingData(ring_replicas)): + necessary_replica_num = (policy.ec_n_unique_fragments * + policy.ec_duplication_factor) + with mock.patch( + 'swift.common.ring.ring.validate_configuration'): + msg = 'EC ring for policy %s needs to be configured ' \ + 'with exactly %d replicas.' % \ + (policy.name, necessary_replica_num) + self.assertRaisesWithMessage(RingLoadError, msg, + policy.load_ring, 'mock') + + # first, do somethign completely different + do_test([8, 10, 7, 11]) + # then again, closer to true, but fractional + do_test([9.9, 14.1, 5.99999, 12.000000001]) def test_storage_policy_get_info(self): test_policies = [