From: salvatore Date: Fri, 21 Aug 2015 08:44:25 +0000 (+0200) Subject: Do not track active reservations X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=e10b008f7a2a1cb45ae5f77082f8d45b51274489;p=openstack-build%2Fneutron-build.git Do not track active reservations Reservations have a transient nature: a reservation lifespan typically begins and ends with a single request. Therefore tracking reserved amounts for each tenant and resource is not nearly as efficient as tracking resource usage. Indeed it is fairly easy to verify that the overhead for tracking reserved amounts is much greater than the one needed for counting active reservations for each tenant and resource. This patch removes the logic for tracking reservations, and replaces it with an explicit count of active reservations. Please note that this patch does not adjust accordingly the ResourceUsage DB model. This will be done in a separate patch with an expand migration; this should avoid most merge conflicts before the final patch for restoring reservation logic merges. Related-Blueprint: better-quotas Change-Id: Ib5e3bd61c1bc0fc8a5d612dae5c1740a8834a980 --- diff --git a/neutron/db/quota/api.py b/neutron/db/quota/api.py index 8ff8cb971..6e1a0e47a 100644 --- a/neutron/db/quota/api.py +++ b/neutron/db/quota/api.py @@ -29,12 +29,8 @@ def utcnow(): class QuotaUsageInfo(collections.namedtuple( - 'QuotaUsageInfo', ['resource', 'tenant_id', 'used', 'reserved', 'dirty'])): - - @property - def total(self): - """Total resource usage (reserved and used).""" - return self.reserved + self.used + 'QuotaUsageInfo', ['resource', 'tenant_id', 'used', 'dirty'])): + """Information about resource quota usage.""" class ReservationInfo(collections.namedtuple( @@ -66,7 +62,6 @@ def get_quota_usage_by_resource_and_tenant(context, resource, tenant_id, return QuotaUsageInfo(result.resource, result.tenant_id, result.in_use, - result.reserved, result.dirty) @@ -76,7 +71,6 @@ def get_quota_usage_by_resource(context, resource): return [QuotaUsageInfo(item.resource, item.tenant_id, item.in_use, - item.reserved, item.dirty) for item in query] @@ -86,12 +80,11 @@ def get_quota_usage_by_tenant_id(context, tenant_id): return [QuotaUsageInfo(item.resource, item.tenant_id, item.in_use, - item.reserved, item.dirty) for item in query] def set_quota_usage(context, resource, tenant_id, - in_use=None, reserved=None, delta=False): + in_use=None, delta=False): """Set resource quota usage. :param context: instance of neutron context with db session @@ -100,10 +93,8 @@ def set_quota_usage(context, resource, tenant_id, being set :param in_use: integer specifying the new quantity of used resources, or a delta to apply to current used resource - :param reserved: integer specifying the new quantity of reserved resources, - or a delta to apply to current reserved resources - :param delta: Specififies whether in_use or reserved are absolute numbers - or deltas (default to False) + :param delta: Specifies whether in_use is an absolute number + or a delta (default to False) """ query = common_db_api.model_query(context, quota_models.QuotaUsage) query = query.filter_by(resource=resource).filter_by(tenant_id=tenant_id) @@ -120,16 +111,11 @@ def set_quota_usage(context, resource, tenant_id, if delta: in_use = usage_data.in_use + in_use usage_data.in_use = in_use - if reserved is not None: - if delta: - reserved = usage_data.reserved + reserved - usage_data.reserved = reserved # After an explicit update the dirty bit should always be reset usage_data.dirty = False return QuotaUsageInfo(usage_data.resource, usage_data.tenant_id, usage_data.in_use, - usage_data.reserved, usage_data.dirty) diff --git a/neutron/db/quota/driver.py b/neutron/db/quota/driver.py index 3b72ffdd4..1645b75bf 100644 --- a/neutron/db/quota/driver.py +++ b/neutron/db/quota/driver.py @@ -126,21 +126,8 @@ class DbQuotaDriver(object): return dict((k, v) for k, v in quotas.items()) - def _handle_expired_reservations(self, context, tenant_id, - resource, expired_amount): - LOG.debug(("Adjusting usage for resource %(resource)s: " - "removing %(expired)d reserved items"), - {'resource': resource, - 'expired': expired_amount}) - # TODO(salv-orlando): It should be possible to do this - # operation for all resources with a single query. - # Update reservation usage - quota_api.set_quota_usage( - context, - resource, - tenant_id, - reserved=-expired_amount, - delta=True) + def _handle_expired_reservations(self, context, tenant_id): + LOG.debug("Deleting expired reservations for tenant:%s" % tenant_id) # Delete expired reservations (we don't want them to accrue # in the database) quota_api.remove_expired_reservations( @@ -209,8 +196,7 @@ class DbQuotaDriver(object): if res_headroom < deltas[resource]: resources_over_limit.append(resource) if expired_reservations: - self._handle_expired_reservations( - context, tenant_id, resource, expired_reservations) + self._handle_expired_reservations(context, tenant_id) if resources_over_limit: raise exceptions.OverQuota(overs=sorted(resources_over_limit)) diff --git a/neutron/quota/resource.py b/neutron/quota/resource.py index 7068254c7..4bfb14c48 100644 --- a/neutron/quota/resource.py +++ b/neutron/quota/resource.py @@ -213,14 +213,13 @@ class TrackedResource(BaseResource): max_retries=db_api.MAX_RETRIES, exception_checker=lambda exc: isinstance(exc, oslo_db_exception.DBDuplicateEntry)) - def _set_quota_usage(self, context, tenant_id, in_use, reserved): - return quota_api.set_quota_usage(context, self.name, tenant_id, - in_use=in_use, reserved=reserved) + def _set_quota_usage(self, context, tenant_id, in_use): + return quota_api.set_quota_usage( + context, self.name, tenant_id, in_use=in_use) - def _resync(self, context, tenant_id, in_use, reserved): + def _resync(self, context, tenant_id, in_use): # Update quota usage - usage_info = self._set_quota_usage( - context, tenant_id, in_use, reserved) + usage_info = self._set_quota_usage(context, tenant_id, in_use) self._dirty_tenants.discard(tenant_id) self._out_of_sync_tenants.discard(tenant_id) @@ -238,14 +237,17 @@ class TrackedResource(BaseResource): in_use = context.session.query(self._model_class).filter_by( tenant_id=tenant_id).count() # Update quota usage - return self._resync(context, tenant_id, in_use, reserved=0) + return self._resync(context, tenant_id, in_use) def count(self, context, _plugin, tenant_id, resync_usage=False): """Return the current usage count for the resource. - This method will fetch the information from resource usage data, - unless usage data are marked as "dirty", in which case both used and - reserved resource are explicitly counted. + This method will fetch aggregate information for resource usage + data, unless usage data are marked as "dirty". + In the latter case resource usage will be calculated counting + rows for tenant_id in the resource's database model. + Active reserved amount are instead always calculated by summing + amounts for matching records in the 'reservations' database model. The _plugin and _resource parameters are unused but kept for compatibility with the signature of the count method for @@ -254,6 +256,11 @@ class TrackedResource(BaseResource): # Load current usage data, setting a row-level lock on the DB usage_info = quota_api.get_quota_usage_by_resource_and_tenant( context, self.name, tenant_id, lock_for_update=True) + # Always fetch reservations, as they are not tracked by usage counters + reservations = quota_api.get_reservations_for_resources( + context, tenant_id, [self.name]) + reserved = reservations.get(self.name, 0) + # If dirty or missing, calculate actual resource usage querying # the database and set/create usage info data # NOTE: this routine "trusts" usage counters at service startup. This @@ -273,23 +280,20 @@ class TrackedResource(BaseResource): # typically one counts before adding a record, and that would mark # the usage counter as dirty again) if resync_usage or not usage_info: - usage_info = self._resync(context, tenant_id, - in_use, reserved=0) + usage_info = self._resync(context, tenant_id, in_use) else: # NOTE(salv-orlando): Passing 0 for reserved amount as # reservations are currently not supported usage_info = quota_api.QuotaUsageInfo(usage_info.resource, usage_info.tenant_id, in_use, - 0, usage_info.dirty) LOG.debug(("Quota usage for %(resource)s was recalculated. " - "Used quota:%(used)d; Reserved quota:%(reserved)d"), + "Used quota:%(used)d."), {'resource': self.name, - 'used': usage_info.used, - 'reserved': usage_info.reserved}) - return usage_info.total + 'used': usage_info.used}) + return usage_info.used + reserved def register_events(self): event.listen(self._model_class, 'after_insert', self._db_event_handler) diff --git a/neutron/tests/unit/db/quota/test_api.py b/neutron/tests/unit/db/quota/test_api.py index c527a6631..e78f7943d 100644 --- a/neutron/tests/unit/db/quota/test_api.py +++ b/neutron/tests/unit/db/quota/test_api.py @@ -34,16 +34,14 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): return quota_api.create_reservation( self.context, tenant_id, resource_deltas, expiration) - def _create_quota_usage(self, resource, used, reserved, tenant_id=None): + def _create_quota_usage(self, resource, used, tenant_id=None): tenant_id = tenant_id or self.tenant_id return quota_api.set_quota_usage( - self.context, resource, tenant_id, - in_use=used, reserved=reserved) + self.context, resource, tenant_id, in_use=used) def _verify_quota_usage(self, usage_info, expected_resource=None, expected_used=None, - expected_reserved=None, expected_dirty=None): self.assertEqual(self.tenant_id, usage_info.tenant_id) if expected_resource: @@ -52,57 +50,42 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): self.assertEqual(expected_dirty, usage_info.dirty) if expected_used is not None: self.assertEqual(expected_used, usage_info.used) - if expected_reserved is not None: - self.assertEqual(expected_reserved, usage_info.reserved) - if expected_used is not None and expected_reserved is not None: - self.assertEqual(expected_used + expected_reserved, - usage_info.total) def setUp(self): super(TestQuotaDbApi, self).setUp() self._set_context() def test_create_quota_usage(self): - usage_info = self._create_quota_usage('goals', 26, 10) + usage_info = self._create_quota_usage('goals', 26) self._verify_quota_usage(usage_info, expected_resource='goals', - expected_used=26, - expected_reserved=10) + expected_used=26) def test_update_quota_usage(self): - self._create_quota_usage('goals', 26, 10) + self._create_quota_usage('goals', 26) # Higuain scores a double usage_info_1 = quota_api.set_quota_usage( self.context, 'goals', self.tenant_id, in_use=28) self._verify_quota_usage(usage_info_1, - expected_used=28, - expected_reserved=10) + expected_used=28) usage_info_2 = quota_api.set_quota_usage( self.context, 'goals', self.tenant_id, - reserved=8) + in_use=24) self._verify_quota_usage(usage_info_2, - expected_used=28, - expected_reserved=8) + expected_used=24) def test_update_quota_usage_with_deltas(self): - self._create_quota_usage('goals', 26, 10) + self._create_quota_usage('goals', 26) # Higuain scores a double usage_info_1 = quota_api.set_quota_usage( self.context, 'goals', self.tenant_id, in_use=2, delta=True) self._verify_quota_usage(usage_info_1, - expected_used=28, - expected_reserved=10) - usage_info_2 = quota_api.set_quota_usage( - self.context, 'goals', self.tenant_id, - reserved=-2, delta=True) - self._verify_quota_usage(usage_info_2, - expected_used=28, - expected_reserved=8) + expected_used=28) def test_set_quota_usage_dirty(self): - self._create_quota_usage('goals', 26, 10) + self._create_quota_usage('goals', 26) # Higuain needs a shower after the match self.assertEqual(1, quota_api.set_quota_usage_dirty( self.context, 'goals', self.tenant_id)) @@ -123,9 +106,9 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): self.context, 'meh', self.tenant_id)) def test_set_resources_quota_usage_dirty(self): - self._create_quota_usage('goals', 26, 10) - self._create_quota_usage('assists', 11, 5) - self._create_quota_usage('bookings', 3, 1) + self._create_quota_usage('goals', 26) + self._create_quota_usage('assists', 11) + self._create_quota_usage('bookings', 3) self.assertEqual(2, quota_api.set_resources_quota_usage_dirty( self.context, ['goals', 'bookings'], self.tenant_id)) usage_info_goals = quota_api.get_quota_usage_by_resource_and_tenant( @@ -139,9 +122,9 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): self._verify_quota_usage(usage_info_bookings, expected_dirty=True) def test_set_resources_quota_usage_dirty_with_empty_list(self): - self._create_quota_usage('goals', 26, 10) - self._create_quota_usage('assists', 11, 5) - self._create_quota_usage('bookings', 3, 1) + self._create_quota_usage('goals', 26) + self._create_quota_usage('assists', 11) + self._create_quota_usage('bookings', 3) # Expect all the resources for the tenant to be set dirty self.assertEqual(3, quota_api.set_resources_quota_usage_dirty( self.context, [], self.tenant_id)) @@ -164,8 +147,8 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): expected_dirty=False) def _test_set_all_quota_usage_dirty(self, expected): - self._create_quota_usage('goals', 26, 10) - self._create_quota_usage('goals', 12, 6, tenant_id='Callejon') + self._create_quota_usage('goals', 26) + self._create_quota_usage('goals', 12, tenant_id='Callejon') self.assertEqual(expected, quota_api.set_all_quota_usage_dirty( self.context, 'goals')) @@ -175,10 +158,10 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): self._test_set_all_quota_usage_dirty(expected=1) def test_get_quota_usage_by_tenant(self): - self._create_quota_usage('goals', 26, 10) - self._create_quota_usage('assists', 11, 5) + self._create_quota_usage('goals', 26) + self._create_quota_usage('assists', 11) # Create a resource for a different tenant - self._create_quota_usage('mehs', 99, 99, tenant_id='buffon') + self._create_quota_usage('mehs', 99, tenant_id='buffon') usage_infos = quota_api.get_quota_usage_by_tenant_id( self.context, self.tenant_id) @@ -188,26 +171,24 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): self.assertIn('assists', resources) def test_get_quota_usage_by_resource(self): - self._create_quota_usage('goals', 26, 10) - self._create_quota_usage('assists', 11, 5) - self._create_quota_usage('goals', 12, 6, tenant_id='Callejon') + self._create_quota_usage('goals', 26) + self._create_quota_usage('assists', 11) + self._create_quota_usage('goals', 12, tenant_id='Callejon') usage_infos = quota_api.get_quota_usage_by_resource( self.context, 'goals') # Only 1 result expected in tenant context self.assertEqual(1, len(usage_infos)) self._verify_quota_usage(usage_infos[0], expected_resource='goals', - expected_used=26, - expected_reserved=10) + expected_used=26) def test_get_quota_usage_by_tenant_and_resource(self): - self._create_quota_usage('goals', 26, 10) + self._create_quota_usage('goals', 26) usage_info = quota_api.get_quota_usage_by_resource_and_tenant( self.context, 'goals', self.tenant_id) self._verify_quota_usage(usage_info, expected_resource='goals', - expected_used=26, - expected_reserved=10) + expected_used=26) def test_get_non_existing_quota_usage_returns_none(self): self.assertIsNone(quota_api.get_quota_usage_by_resource_and_tenant( @@ -226,7 +207,7 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): self.assertEqual(self.tenant_id, resv.tenant_id) self._verify_reserved_resources(resources, resv.deltas) - def test_create_reservation_with_expirtion(self): + def test_create_reservation_with_expiration(self): resources = {'goals': 2, 'assists': 1} exp_date = datetime.datetime(2016, 3, 31, 14, 30) resv = self._create_reservation(resources, expiration=exp_date) @@ -234,22 +215,6 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): self.assertEqual(exp_date, resv.expiration) self._verify_reserved_resources(resources, resv.deltas) - def _test_remove_reservation(self, set_dirty): - resources = {'goals': 2, 'assists': 1} - resv = self._create_reservation(resources) - self.assertEqual(1, quota_api.remove_reservation( - self.context, resv.reservation_id, set_dirty=set_dirty)) - - def test_remove_reservation(self): - self._test_remove_reservation(False) - - def test_remove_reservation_and_set_dirty(self): - routine = 'neutron.db.quota.api.set_resources_quota_usage_dirty' - with mock.patch(routine) as mock_routine: - self._test_remove_reservation(False) - mock_routine.assert_called_once_with( - self.context, mock.ANY, self.tenant_id) - def test_remove_non_existent_reservation(self): self.assertIsNone(quota_api.remove_reservation(self.context, 'meh')) @@ -298,6 +263,22 @@ class TestQuotaDbApi(testlib_api.SqlTestCaseLight): self.assertIsNone(quota_api.get_reservations_for_resources( self.context, self.tenant_id, [])) + def _test_remove_reservation(self, set_dirty): + resources = {'goals': 2, 'assists': 1} + resv = self._create_reservation(resources) + self.assertEqual(1, quota_api.remove_reservation( + self.context, resv.reservation_id, set_dirty=set_dirty)) + + def test_remove_reservation(self): + self._test_remove_reservation(False) + + def test_remove_reservation_and_set_dirty(self): + routine = 'neutron.db.quota.api.set_resources_quota_usage_dirty' + with mock.patch(routine) as mock_routine: + self._test_remove_reservation(False) + mock_routine.assert_called_once_with( + self.context, mock.ANY, self.tenant_id) + def test_remove_expired_reservations(self): with mock.patch('neutron.db.quota.api.utcnow') as mock_utcnow: mock_utcnow.return_value = datetime.datetime( @@ -342,9 +323,9 @@ class TestQuotaDbApiAdminContext(TestQuotaDbApi): load_admin_roles=False) def test_get_quota_usage_by_resource(self): - self._create_quota_usage('goals', 26, 10) - self._create_quota_usage('assists', 11, 5) - self._create_quota_usage('goals', 12, 6, tenant_id='Callejon') + self._create_quota_usage('goals', 26) + self._create_quota_usage('assists', 11) + self._create_quota_usage('goals', 12, tenant_id='Callejon') usage_infos = quota_api.get_quota_usage_by_resource( self.context, 'goals') # 2 results expected in admin context diff --git a/neutron/tests/unit/quota/test_resource.py b/neutron/tests/unit/quota/test_resource.py index 88a00bbc9..7f6685398 100644 --- a/neutron/tests/unit/quota/test_resource.py +++ b/neutron/tests/unit/quota/test_resource.py @@ -165,8 +165,7 @@ class TestTrackedResource(testlib_api.SqlTestCaseLight): res.count(self.context, None, self.tenant_id, resync_usage=True) mock_set_quota_usage.assert_called_once_with( - self.context, self.resource, self.tenant_id, - reserved=0, in_use=2) + self.context, self.resource, self.tenant_id, in_use=2) def test_count_with_dirty_true_no_usage_info(self): res = self._create_resource() @@ -185,8 +184,7 @@ class TestTrackedResource(testlib_api.SqlTestCaseLight): self.tenant_id) res.count(self.context, None, self.tenant_id, resync_usage=True) mock_set_quota_usage.assert_called_once_with( - self.context, self.resource, self.tenant_id, - reserved=0, in_use=2) + self.context, self.resource, self.tenant_id, in_use=2) def test_add_delete_data_triggers_event(self): res = self._create_resource() @@ -253,5 +251,4 @@ class TestTrackedResource(testlib_api.SqlTestCaseLight): # and now it should be in sync self.assertNotIn(self.tenant_id, res._out_of_sync_tenants) mock_set_quota_usage.assert_called_once_with( - self.context, self.resource, self.tenant_id, - reserved=0, in_use=2) + self.context, self.resource, self.tenant_id, in_use=2)