diff --git a/neutron/db/quota/api.py b/neutron/db/quota/api.py index 8ff8cb971ed..6e1a0e47a5e 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 3b72ffdd4ed..1645b75bfe7 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 7068254c7dc..4bfb14c4823 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 c527a663179..e78f7943df6 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 88a00bbc924..7f668539807 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)