From 2c3ac543f4106171997752d5784732bda6e6bf3e Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 25 May 2017 09:40:47 -0700 Subject: [PATCH] Require that known-bad EC schemes be deprecated We said we were going to do it, we've had two releases saying we'd do it, we've even backported our saying it to Newton -- let's actually do it. Upgrade Consideration ===================== Erasure-coded storage policies using isa_l_rs_vand and nparity >= 5 must be configured as deprecated, preventing any new containers from being created with such a policy. This configuration is known to harm data durability. Any data in such policies should be migrated to a new policy. See https://bugs.launchpad.net/swift/+bug/1639691 for more information. UpgradeImpact Related-Change: I50159c9d19f2385d5f60112e9aaefa1a68098313 Change-Id: I8f9de0bec01032d9d9b58848e2a76ac92e65ab09 Closes-Bug: 1639691 --- doc/source/overview_erasure_code.rst | 9 +++++++++ etc/swift.conf-sample | 7 ++++--- swift/common/storage_policy.py | 12 ++++-------- test/unit/common/test_storage_policy.py | 13 ++++++++----- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/doc/source/overview_erasure_code.rst b/doc/source/overview_erasure_code.rst index dda02923c6..16353affad 100644 --- a/doc/source/overview_erasure_code.rst +++ b/doc/source/overview_erasure_code.rst @@ -252,6 +252,15 @@ with the specified policy name and interacting as usual. case a change in the configuration is desired, you must create a new policy and migrate the data to a new container. +.. warning:: + + Using ``isa_l_rs_vand`` with more than 4 parity fragments creates fragments + which may in some circumstances fail to reconstruct properly or (with + liberasurecode < 1.3.1) reconstruct corrupted data. New policies that need + large numbers of parity fragments should consider using ``isa_l_rs_cauchy``. + Any existing affected policies must be marked deprecated, and data in + containers with that policy should be migrated to a new policy. + Migrating Between Policies ========================== diff --git a/etc/swift.conf-sample b/etc/swift.conf-sample index e008274c40..8edb06344c 100644 --- a/etc/swift.conf-sample +++ b/etc/swift.conf-sample @@ -62,13 +62,14 @@ aliases = yellow, orange # EC configuration parameters 'ec_type', 'ec_num_data_fragments' and # 'ec_num_parity_fragments' must be specified. 'ec_type' is chosen from the # list of EC backends supported by PyECLib. The ring configured for the -# storage policy must have it's "replica" count configured to +# storage policy must have its "replica" count configured to # 'ec_num_data_fragments' + 'ec_num_parity_fragments' - this requirement is # validated when services start. 'ec_object_segment_size' is the amount of # data that will be buffered up before feeding a segment into the # encoder/decoder. More information about these configuration options and -# supported `ec_type` schemes is available in the Swift documentation. Please -# refer to Swift documentation for details on how to configure EC policies. +# supported 'ec_type' schemes is available in the Swift documentation. See +# https://docs.openstack.org/developer/swift/overview_erasure_code.html +# for more information on how to configure EC policies. # # The example 'deepfreeze10-4' policy defined below is a _sample_ # configuration with an alias of 'df10-4' as well as 10 'data' and 4 'parity' diff --git a/swift/common/storage_policy.py b/swift/common/storage_policy.py index 67034d021b..e153daf32d 100644 --- a/swift/common/storage_policy.py +++ b/swift/common/storage_policy.py @@ -468,14 +468,10 @@ class ECStoragePolicy(BaseStoragePolicy): 'See https://bugs.launchpad.net/swift/+bug/1639691 for ' 'more information.' % self.name) if not is_deprecated: - # TODO: To fully close bug 1639691, uncomment the raise and - # removing the warning below. This will be in the Pike release - # at the earliest. - logger.warning( - 'In a future release, this will prevent services from ' - 'starting unless the policy is marked as deprecated.') - # raise PolicyError('Storage policy %s MUST be deprecated' % - # self.name) + raise PolicyError( + 'Storage policy %s uses an EC configuration known to harm ' + 'data durability. This policy MUST be deprecated.' + % self.name) # Initialize PyECLib EC backend try: diff --git a/test/unit/common/test_storage_policy.py b/test/unit/common/test_storage_policy.py index cfa7baaf26..c6a712784b 100644 --- a/test/unit/common/test_storage_policy.py +++ b/test/unit/common/test_storage_policy.py @@ -676,18 +676,21 @@ class TestStoragePolicies(unittest.TestCase): ec_num_parity_fragments = 5 """) - with capture_logging('swift.common.storage_policy') as records: + with capture_logging('swift.common.storage_policy') as records, \ + self.assertRaises(PolicyError) as exc_mgr: parse_storage_policies(bad_conf) - mock_driver.assert_called_once() + self.assertEqual(exc_mgr.exception.message, + 'Storage policy bad-policy uses an EC ' + 'configuration known to harm data durability. This ' + 'policy MUST be deprecated.') + mock_driver.assert_not_called() mock_driver.reset_mock() self.assertEqual([r.levelname for r in records], - ['WARNING', 'WARNING']) + ['WARNING']) for msg in ('known to harm data durability', 'Any data in this policy should be migrated', 'https://bugs.launchpad.net/swift/+bug/1639691'): self.assertIn(msg, records[0].msg) - self.assertIn('In a future release, this will prevent services from ' - 'starting', records[1].msg) slightly_less_bad_conf = self._conf(""" [storage-policy:0]