From 75953752e41819b0d2ca770d4d13ed1ccaec685a Mon Sep 17 00:00:00 2001 From: Xing Yang Date: Wed, 27 May 2015 12:58:04 -0400 Subject: [PATCH] Fix capacity filter to allow oversubscription Currently the capacity filter does not allow oversubscrition if the free physical capacity is less than the new volume size. This was based on the assumption that a new thinly provisioned volume will be consumed right away. This assumption does not allow us to take full advantage of thin provisioning. The fix is to allow oversubscription if thin provisioning is supported and max_over_subscription_ratio is greater than or equal to 1. The free physical capacity will not be used to compare with the new volume size for a backend that supports oversubscription in thin provisioning. The oversubscription ratio and virtual free capacity will still be used to decide whether a backend can be chosen for thin provisioning. Closes-Bug: #1458976 Change-Id: I4c8904445654db839c25ded420915f83c3756f94 --- cinder/scheduler/filters/capacity_filter.py | 23 +++---- .../tests/unit/scheduler/test_host_filters.py | 62 ++++++++++++++++--- 2 files changed, 65 insertions(+), 20 deletions(-) diff --git a/cinder/scheduler/filters/capacity_filter.py b/cinder/scheduler/filters/capacity_filter.py index 9d377bf62..be13e67f0 100644 --- a/cinder/scheduler/filters/capacity_filter.py +++ b/cinder/scheduler/filters/capacity_filter.py @@ -80,22 +80,13 @@ class CapacityFilter(filters.BaseHostFilter): msg_args = {"host": host_state.host, "requested": volume_size, "available": free} - if free < volume_size: - LOG.warning(_LW("Insufficient free space for volume creation " - "on host %(host)s (requested / avail): " - "%(requested)s/%(available)s"), msg_args) - return free >= volume_size - else: - LOG.debug("Space information for volume creation " - "on host %(host)s (requested / avail): " - "%(requested)s/%(available)s", msg_args) # Only evaluate using max_over_subscription_ratio if # thin_provisioning_support is True. Check if the ratio of # provisioned capacity over total capacity has exceeded over # subscription ratio. if (host_state.thin_provisioning_support and - host_state.max_over_subscription_ratio >= 1): + host_state.max_over_subscription_ratio > 1): provisioned_ratio = ((host_state.provisioned_capacity_gb + volume_size) / total) if provisioned_ratio >= host_state.max_over_subscription_ratio: @@ -113,4 +104,14 @@ class CapacityFilter(filters.BaseHostFilter): free_virtual = free * host_state.max_over_subscription_ratio return free_virtual >= volume_size - return free >= volume_size + if free < volume_size: + LOG.warning(_LW("Insufficient free space for volume creation " + "on host %(host)s (requested / avail): " + "%(requested)s/%(available)s"), msg_args) + return False + + LOG.debug("Space information for volume creation " + "on host %(host)s (requested / avail): " + "%(requested)s/%(available)s", msg_args) + + return True diff --git a/cinder/tests/unit/scheduler/test_host_filters.py b/cinder/tests/unit/scheduler/test_host_filters.py index 592f2be56..70e82696d 100644 --- a/cinder/tests/unit/scheduler/test_host_filters.py +++ b/cinder/tests/unit/scheduler/test_host_filters.py @@ -162,6 +162,28 @@ class CapacityFilterTestCase(HostFiltersTestCase): 'service': service}) self.assertTrue(filt_cls.host_passes(host, filter_properties)) + @mock.patch('cinder.utils.service_is_up') + def test_filter_over_subscription_less_than_1(self, _mock_serv_is_up): + _mock_serv_is_up.return_value = True + filt_cls = self.class_map['CapacityFilter']() + filter_properties = {'size': 200, + 'capabilities:thin_provisioning_support': + ' True', + 'capabilities:thick_provisioning_support': + ' False'} + service = {'disabled': False} + host = fakes.FakeHostState('host1', + {'total_capacity_gb': 500, + 'free_capacity_gb': 100, + 'provisioned_capacity_gb': 400, + 'max_over_subscription_ratio': 0.8, + 'reserved_percentage': 0, + 'thin_provisioning_support': True, + 'thick_provisioning_support': False, + 'updated_at': None, + 'service': service}) + self.assertFalse(filt_cls.host_passes(host, filter_properties)) + @mock.patch('cinder.utils.service_is_up') def test_filter_over_subscription_fails(self, _mock_serv_is_up): _mock_serv_is_up.return_value = True @@ -175,8 +197,8 @@ class CapacityFilterTestCase(HostFiltersTestCase): host = fakes.FakeHostState('host1', {'total_capacity_gb': 500, 'free_capacity_gb': 200, - 'provisioned_capacity_gb': 500, - 'max_over_subscription_ratio': 1.0, + 'provisioned_capacity_gb': 700, + 'max_over_subscription_ratio': 1.5, 'reserved_percentage': 5, 'thin_provisioning_support': True, 'thick_provisioning_support': False, @@ -188,7 +210,7 @@ class CapacityFilterTestCase(HostFiltersTestCase): def test_filter_over_subscription_fails2(self, _mock_serv_is_up): _mock_serv_is_up.return_value = True filt_cls = self.class_map['CapacityFilter']() - filter_properties = {'size': 30, + filter_properties = {'size': 2000, 'capabilities:thin_provisioning_support': ' True', 'capabilities:thick_provisioning_support': @@ -197,8 +219,8 @@ class CapacityFilterTestCase(HostFiltersTestCase): host = fakes.FakeHostState('host1', {'total_capacity_gb': 500, 'free_capacity_gb': 30, - 'provisioned_capacity_gb': 500, - 'max_over_subscription_ratio': 1.0, + 'provisioned_capacity_gb': 9000, + 'max_over_subscription_ratio': 20, 'reserved_percentage': 0, 'thin_provisioning_support': True, 'thick_provisioning_support': False, @@ -219,7 +241,7 @@ class CapacityFilterTestCase(HostFiltersTestCase): host = fakes.FakeHostState('host1', {'total_capacity_gb': 500, 'free_capacity_gb': 100, - 'provisioned_capacity_gb': 500, + 'provisioned_capacity_gb': 1000, 'max_over_subscription_ratio': 2.0, 'reserved_percentage': 5, 'thin_provisioning_support': True, @@ -264,8 +286,8 @@ class CapacityFilterTestCase(HostFiltersTestCase): service = {'disabled': False} host = fakes.FakeHostState('host1', {'total_capacity_gb': 500, - 'free_capacity_gb': 100, - 'provisioned_capacity_gb': 400, + 'free_capacity_gb': 0, + 'provisioned_capacity_gb': 800, 'max_over_subscription_ratio': 2.0, 'reserved_percentage': 5, 'thin_provisioning_support': True, @@ -296,6 +318,28 @@ class CapacityFilterTestCase(HostFiltersTestCase): 'service': service}) self.assertTrue(filt_cls.host_passes(host, filter_properties)) + @mock.patch('cinder.utils.service_is_up') + def test_filter_reserved_thin_true_passes(self, _mock_serv_is_up): + _mock_serv_is_up.return_value = True + filt_cls = self.class_map['CapacityFilter']() + filter_properties = {'size': 100, + 'capabilities:thin_provisioning_support': + ' True', + 'capabilities:thick_provisioning_support': + ' False'} + service = {'disabled': False} + host = fakes.FakeHostState('host1', + {'total_capacity_gb': 500, + 'free_capacity_gb': 80, + 'provisioned_capacity_gb': 600, + 'max_over_subscription_ratio': 2.0, + 'reserved_percentage': 5, + 'thin_provisioning_support': True, + 'thick_provisioning_support': False, + 'updated_at': None, + 'service': service}) + self.assertTrue(filt_cls.host_passes(host, filter_properties)) + @mock.patch('cinder.utils.service_is_up') def test_filter_reserved_thin_thick_true_fails2(self, _mock_serv_is_up): _mock_serv_is_up.return_value = True @@ -309,7 +353,7 @@ class CapacityFilterTestCase(HostFiltersTestCase): host = fakes.FakeHostState('host1', {'total_capacity_gb': 500, 'free_capacity_gb': 99, - 'provisioned_capacity_gb': 400, + 'provisioned_capacity_gb': 1000, 'max_over_subscription_ratio': 2.0, 'reserved_percentage': 5, 'thin_provisioning_support': True, -- 2.45.2