From 2876f59d4cdbafc297d79f4e6295f05e3448ae47 Mon Sep 17 00:00:00 2001 From: Kota Tsuyuzaki Date: Wed, 20 Jul 2016 18:16:27 -0700 Subject: [PATCH] Cache fragment size for EC policy ECStoragePolicy.fragment_size is never changed on running Swift because it is from ec_segment_size and ec_type defined in swift.conf statically so let's cache the value after retrieving the value from the pyeclib driver. And more, pyeclib <= 1.2.1 (current newest) has a bug [1] to leak the reference count of the items in the returned dict (i.e. causes memory leak) so that this caching will be mitigation of the memory leak because this saves the call count fewer than current as possible. Note that the complete fix for the memory leak for pyeclib is proposed at https://review.openstack.org/#/c/344066/ 1: https://bugs.launchpad.net/pyeclib/+bug/1604335 Related-Bug: #1604335 Change-Id: I6bbaa4063dc462383c949764b6567b2bee233689 --- swift/common/storage_policy.py | 8 ++++++-- test/unit/common/test_storage_policy.py | 24 ++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/swift/common/storage_policy.py b/swift/common/storage_policy.py index 19b9f26e77..fd0b54dfa8 100644 --- a/swift/common/storage_policy.py +++ b/swift/common/storage_policy.py @@ -475,6 +475,7 @@ class ECStoragePolicy(BaseStoragePolicy): # quorum size in the EC case depends on the choice of EC scheme. self._ec_quorum_size = \ self._ec_ndata + self.pyeclib_driver.min_parity_fragments_needed() + self._fragment_size = None @property def ec_type(self): @@ -511,8 +512,11 @@ class ECStoragePolicy(BaseStoragePolicy): # segment_size we'll still only read *the whole one and only last # fragment* and pass than into pyeclib who will know what to do with # it just as it always does when the last fragment is < fragment_size. - return self.pyeclib_driver.get_segment_info( - self.ec_segment_size, self.ec_segment_size)['fragment_size'] + if self._fragment_size is None: + self._fragment_size = self.pyeclib_driver.get_segment_info( + self.ec_segment_size, self.ec_segment_size)['fragment_size'] + + return self._fragment_size @property def ec_scheme_description(self): diff --git a/test/unit/common/test_storage_policy.py b/test/unit/common/test_storage_policy.py index 3fd721b732..14f4ac7ff3 100755 --- a/test/unit/common/test_storage_policy.py +++ b/test/unit/common/test_storage_policy.py @@ -27,6 +27,7 @@ from swift.common.storage_policy import ( VALID_EC_TYPES, DEFAULT_EC_OBJECT_SEGMENT_SIZE, BindPortsCache) from swift.common.ring import RingData from swift.common.exceptions import RingValidationError +from pyeclib.ec_iface import ECDriver @BaseStoragePolicy.register('fake') @@ -1244,6 +1245,29 @@ class TestStoragePolicies(unittest.TestCase): expected_info = expected[(int(policy), False)] self.assertEqual(policy.get_info(config=False), expected_info) + def test_ec_fragment_size_cached(self): + policy = ECStoragePolicy( + 0, 'ec2-1', ec_type=DEFAULT_TEST_EC_TYPE, + ec_ndata=2, ec_nparity=1, object_ring=FakeRing(replicas=3), + ec_segment_size=DEFAULT_EC_OBJECT_SEGMENT_SIZE, is_default=True) + + ec_driver = ECDriver(ec_type=DEFAULT_TEST_EC_TYPE, + k=2, m=1) + expected_fragment_size = ec_driver.get_segment_info( + DEFAULT_EC_OBJECT_SEGMENT_SIZE, + DEFAULT_EC_OBJECT_SEGMENT_SIZE)['fragment_size'] + + with mock.patch.object( + policy.pyeclib_driver, 'get_segment_info') as fake: + fake.return_value = { + 'fragment_size': expected_fragment_size} + + for x in range(10): + self.assertEqual(expected_fragment_size, + policy.fragment_size) + # pyeclib_driver.get_segment_info is called only once + self.assertEqual(1, fake.call_count) + if __name__ == '__main__': unittest.main()