From b2e1797889f292aef43ffb581c5f67ed53f66070 Mon Sep 17 00:00:00 2001 From: Felipe Rodrigues Date: Thu, 1 Oct 2020 11:21:02 +0000 Subject: [PATCH] [NetApp] Implement cached status pool In order to optimize the NetApp ONTAP driver, this patch is caching the status of driver pools and reusing for the each share server, given that the pool is not separated by share server. The option `netapp_cached_aggregates_status_lifetime` is added for controlling the time that the cached values is considered valid. Closes-Bug: #1900469 Change-Id: I14a059615fc29c7c173c035bb51d39e0bbb8b70a --- .../netapp/dataontap/cluster_mode/lib_base.py | 24 ++++++++-- manila/share/drivers/netapp/options.py | 8 +++- manila/share/drivers/netapp/utils.py | 27 +++++++++++ .../dataontap/cluster_mode/test_lib_base.py | 3 ++ .../tests/share/drivers/netapp/test_utils.py | 47 +++++++++++++++++++ ...pp-cache-pool-status-6dc7da824b9f41c1.yaml | 10 ++++ 6 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 releasenotes/notes/1900469-netapp-cache-pool-status-6dc7da824b9f41c1.yaml diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index c6ce5f3020..9b9d9138b3 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -155,6 +155,7 @@ class NetAppCmodeFileStorageLibrary(object): self._cluster_info = {} self._default_nfs_config = None self.is_nfs_config_supported = False + self._cache_pool_status = None self._app_version = kwargs.get('app_version', 'unknown') @@ -188,6 +189,9 @@ class NetAppCmodeFileStorageLibrary(object): LOG.debug('The default NFS configuration: %s', self._default_nfs_config) + self._cache_pool_status = na_utils.DataCache( + self.configuration.netapp_cached_aggregates_status_lifetime) + @na_utils.trace def _set_cluster_info(self): self._cluster_info['nve_support'] = ( @@ -390,13 +394,18 @@ class NetAppCmodeFileStorageLibrary(object): :param share_server: ShareServer class instance. """ - return self._get_pools() + + if self._cache_pool_status.is_expired(): + return self._get_pools() + + return self._cache_pool_status.get_data() @na_utils.trace def _get_pools(self, filter_function=None, goodness_function=None): """Retrieve list of pools available to this backend.""" pools = [] + cached_pools = [] aggr_space = self._get_aggregate_space() aggregates = aggr_space.keys() @@ -427,8 +436,8 @@ class NetAppCmodeFileStorageLibrary(object): pool = { 'pool_name': aggr_name, - 'filter_function': filter_function, - 'goodness_function': goodness_function, + 'filter_function': None, + 'goodness_function': None, 'total_capacity_gb': total_capacity_gb, 'free_capacity_gb': free_capacity_gb, 'allocated_capacity_gb': allocated_capacity_gb, @@ -455,7 +464,14 @@ class NetAppCmodeFileStorageLibrary(object): aggr_name) pool['utilization'] = na_utils.round_down(utilization) - pools.append(pool) + cached_pools.append(pool) + pool_with_func = copy.deepcopy(pool) + pool_with_func['filter_function'] = filter_function + pool_with_func['goodness_function'] = goodness_function + + pools.append(pool_with_func) + + self._cache_pool_status.update_data(cached_pools) return pools diff --git a/manila/share/drivers/netapp/options.py b/manila/share/drivers/netapp/options.py index 83e0e448cd..399e0115c1 100644 --- a/manila/share/drivers/netapp/options.py +++ b/manila/share/drivers/netapp/options.py @@ -134,7 +134,13 @@ netapp_provisioning_opts = [ default='fpolicy_policy_%(share_id)s'), cfg.StrOpt('netapp_fpolicy_event_name_template', help='NetApp FPolicy policy name template.', - default='fpolicy_event_%(protocol)s_%(share_id)s'), ] + default='fpolicy_event_%(protocol)s_%(share_id)s'), + cfg.IntOpt('netapp_cached_aggregates_status_lifetime', + min=0, + default=60, + help='The maximum time in seconds that the cached aggregates ' + 'status will be considered valid. Trying to read the ' + 'expired cache leads to refreshing it.'), ] netapp_cluster_opts = [ cfg.StrOpt('netapp_vserver', diff --git a/manila/share/drivers/netapp/utils.py b/manila/share/drivers/netapp/utils.py index d1a74fdf1a..68428913ca 100644 --- a/manila/share/drivers/netapp/utils.py +++ b/manila/share/drivers/netapp/utils.py @@ -22,6 +22,7 @@ import re from oslo_concurrency import processutils as putils from oslo_log import log +from oslo_utils import timeutils import six from manila import exception @@ -238,3 +239,29 @@ class OpenStackInfo(object): return '%(version)s|%(release)s|%(vendor)s|%(platform)s' % { 'version': self._version, 'release': self._release, 'vendor': self._vendor, 'platform': self._platform} + + +class DataCache(object): + """DataCache class for caching NetApp information. + + The cache validity is measured by a stop watch that is + not thread-safe. + """ + + def __init__(self, duration): + self._stop_watch = timeutils.StopWatch(duration) + self._cached_data = None + + def is_expired(self): + return not self._stop_watch.has_started() or self._stop_watch.expired() + + def get_data(self): + return self._cached_data + + def update_data(self, cached_data): + if not self._stop_watch.has_started(): + self._stop_watch.start() + else: + self._stop_watch.restart() + + self._cached_data = cached_data diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py index 89cd15d0fc..5aee184d11 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py @@ -489,6 +489,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object(self.library, '_get_pools', mock.Mock(return_value=fake.POOLS)) + self.library._cache_pool_status = na_utils.DataCache(60) result = self.library.get_share_server_pools(fake.SHARE_SERVER) @@ -499,6 +500,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object( self.library, '_get_aggregate_space', mock.Mock(return_value=fake.AGGREGATE_CAPACITIES)) + self.library._cache_pool_status = na_utils.DataCache(60) self.library._have_cluster_creds = True self.library._revert_to_snapshot_support = True self.library._cluster_info = fake.CLUSTER_INFO @@ -516,6 +518,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object( self.library, '_get_aggregate_space', mock.Mock(return_value=fake.AGGREGATE_CAPACITIES_VSERVER_CREDS)) + self.library._cache_pool_status = na_utils.DataCache(60) self.library._have_cluster_creds = False self.library._revert_to_snapshot_support = True self.library._cluster_info = fake.CLUSTER_INFO diff --git a/manila/tests/share/drivers/netapp/test_utils.py b/manila/tests/share/drivers/netapp/test_utils.py index 90412a9871..228c7e5290 100644 --- a/manila/tests/share/drivers/netapp/test_utils.py +++ b/manila/tests/share/drivers/netapp/test_utils.py @@ -394,3 +394,50 @@ class OpenstackInfoTestCase(test.TestCase): info._update_openstack_info() self.assertTrue(mock_updt_from_dpkg.called) + + +@ddt.ddt +class DataCacheTestCase(test.TestCase): + + def setUp(self): + super(DataCacheTestCase, self).setUp() + + self.cache = na_utils.DataCache(60) + self.cache._stop_watch = mock.Mock() + + @ddt.data(True, False) + def test_is_expired(self, is_expired): + not_expired = not is_expired + self.mock_object( + self.cache._stop_watch, 'has_started', + mock.Mock(return_value=not_expired)) + + self.mock_object( + self.cache._stop_watch, 'expired', + mock.Mock(return_value=is_expired)) + + self.assertEqual(is_expired, self.cache.is_expired()) + + def test_get_data(self): + fake_data = 10 + self.cache._cached_data = fake_data + self.assertEqual(fake_data, self.cache.get_data()) + + @ddt.data(True, False) + def test_update_data(self, started): + self.mock_object( + self.cache._stop_watch, 'has_started', + mock.Mock(return_value=started)) + mock_start = self.mock_object(self.cache._stop_watch, 'start', + mock.Mock()) + mock_restart = self.mock_object(self.cache._stop_watch, 'restart', + mock.Mock()) + fake_data = 10 + + self.cache.update_data(fake_data) + + self.assertEqual(self.cache._cached_data, fake_data) + if not started: + mock_start.assert_called_once() + else: + mock_restart.assert_called_once() diff --git a/releasenotes/notes/1900469-netapp-cache-pool-status-6dc7da824b9f41c1.yaml b/releasenotes/notes/1900469-netapp-cache-pool-status-6dc7da824b9f41c1.yaml new file mode 100644 index 0000000000..b349538cd9 --- /dev/null +++ b/releasenotes/notes/1900469-netapp-cache-pool-status-6dc7da824b9f41c1.yaml @@ -0,0 +1,10 @@ +--- +fixes: + - | + In order to optimize the NetApp ONTAP driver, this patch is caching + the status of driver pools and reusing for the each share server, given + that the pool is not separated by share server. It adds the option + `netapp_cached_aggregates_status_lifetime` for controlling the time that + the cached values is considered valid. Please refer to the + `Launchpad bug #1900469 `_ + for more details.