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 8ff8cb971ed4ba28f6a3de993fc4513db7eaefb9..6e1a0e47a5eca184b3ed1735094d08bd51a26da5 100644 (file)
--- 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 3b72ffdd4ed0b85864650c37aa2d494c3d7ea59f..1645b75bfe7ebd79820f1671f6d9162df6b33bc4 100644 (file)
--- 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 7068254c7dc4a39fea9b11308cef0a0a598f1709..4bfb14c4823ac15ca954d30b02b6053d867d7d25 100644 (file)
--- 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 c527a6631792ab0bdaabc0c9ab683e1f0c0ba317..e78f7943df689b285afe32ff692d643dc839d585 100644 (file)
--- 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 88a00bbc924eaadfc482682572da66a8a20aa90b..7f668539807395afd9d41b5f979d568c4256cdca 100644 (file)
--- 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)