Merge "Remove the expired reservations in a separate DB transaction"

neutron/db/quota/api.py

@@ -23,7 +23,7 @@ from neutron.common import utils
 from neutron.objects import quota as quota_obj
 
 
-RESERVATION_EXPIRATION_TIMEOUT = 20  # seconds
+RESERVATION_EXPIRATION_TIMEOUT = 120  # seconds
 UNLIMITED_QUOTA = -1
 
 
@@ -175,7 +175,8 @@ def set_all_quota_usage_dirty(context, resource, dirty=True):
 def create_reservation(context, project_id, deltas, expiration=None):
     # This method is usually called from within another transaction.
     # Consider using begin_nested
-    expiration = expiration or (utcnow() + datetime.timedelta(0, 120))
+    expiration = expiration or (
+            utcnow() + datetime.timedelta(0, RESERVATION_EXPIRATION_TIMEOUT))
     delta_objs = []
     for (resource, delta) in deltas.items():
         delta_objs.append(quota_obj.ResourceDelta(
@@ -241,7 +242,6 @@ def get_reservations_for_resources(context, project_id, resources,
         context, utcnow(), project_id, resources, expired)
 
 
-@db_api.retry_if_session_inactive()
 @db_api.CONTEXT_WRITER
 def remove_expired_reservations(context, project_id=None, timeout=None):
     expiring_time = utcnow()

neutron/db/quota/driver_nolock.py

@@ -15,8 +15,10 @@
 
 from neutron_lib.db import api as db_api
 from neutron_lib import exceptions
+from oslo_db import exception as db_exc
 from oslo_log import log
 
+from neutron.common import utils
 from neutron.db.quota import api as quota_api
 from neutron.db.quota import driver as quota_driver
 
@@ -40,8 +42,26 @@ class DbQuotaNoLockDriver(quota_driver.DbQuotaDriver):
     method is to be fast enough to avoid the concurrency when counting the
     resources while not blocking concurrent API operations.
     """
+    @utils.skip_exceptions(db_exc.DBError)
+    def _remove_expired_reservations(self, context, project_id, timeout):
+        """Remove expired reservations
+
+        Any DB exception will be catch and dismissed. This operation can have
+        been successfully executed by another concurrent request. There is no
+        need to fail or retry it.
+        """
+        quota_api.remove_expired_reservations(context, project_id=project_id,
+                                              timeout=timeout)
+
     @db_api.retry_if_session_inactive()
     def make_reservation(self, context, project_id, resources, deltas, plugin):
+        # Delete expired reservations before counting valid ones. This
+        # operation is fast and by calling it before making any
+        # reservation, we ensure the freshness of the reservations.
+        self._remove_expired_reservations(
+            context, project_id=project_id,
+            timeout=quota_api.RESERVATION_EXPIRATION_TIMEOUT)
+
         resources_over_limit = []
         with db_api.CONTEXT_WRITER.using(context):
             # Filter out unlimited resources.
@@ -50,13 +70,6 @@ class DbQuotaNoLockDriver(quota_driver.DbQuotaDriver):
                                        limits.items() if limit < 0])
             requested_resources = (set(deltas.keys()) - unlimited_resources)
 
-            # Delete expired reservations before counting valid ones. This
-            # operation is fast and by calling it before making any
-            # reservation, we ensure the freshness of the reservations.
-            quota_api.remove_expired_reservations(
-                context, project_id=project_id,
-                timeout=quota_api.RESERVATION_EXPIRATION_TIMEOUT)
-
             # Count the number of (1) used and (2) reserved resources for this
             # project_id. If any resource limit is exceeded, raise exception.
             for resource_name in requested_resources:

neutron/tests/unit/db/quota/test_driver.py

@@ -95,6 +95,8 @@ class TestDbQuotaDriver(testlib_api.SqlTestCase,
         self.quota_driver = driver.DbQuotaDriver()
         self.project_1, self.project_2 = 'prj_test_1', 'prj_test_2'
         self.resource_1, self.resource_2 = 'res_test_1', 'res_test_2'
+        self.projects = (self.project_1, self.project_2)
+        self.resources = (self.resource_1, self.resource_2)
 
     def test_create_quota_limit(self):
         defaults = {RESOURCE: TestResource(RESOURCE, 4)}

neutron/tests/unit/db/quota/test_driver_nolock.py

@@ -13,7 +13,13 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import itertools
+
+from neutron_lib.db import api as db_api
+
+from neutron.db.quota import api as quota_api
 from neutron.db.quota import driver_nolock
+from neutron.objects import quota as quota_obj
 from neutron.tests.unit.db.quota import test_driver
 
 
@@ -22,3 +28,32 @@ class TestDbQuotaDriverNoLock(test_driver.TestDbQuotaDriver):
     def setUp(self):
         super(TestDbQuotaDriverNoLock, self).setUp()
         self.quota_driver = driver_nolock.DbQuotaNoLockDriver()
+
+    def test__remove_expired_reservations(self):
+        for project, resource in itertools.product(self.projects,
+                                                   self.resources):
+            deltas = {resource: 1}
+            with db_api.CONTEXT_WRITER.using(self.context):
+                quota_api.create_reservation(self.context, project, deltas)
+
+        # Initial check: the reservations are correctly created.
+        for project in self.projects:
+            for res in quota_obj.Reservation.get_objects(self.context,
+                                                         project_id=project):
+                self.assertEqual(1, len(res.resource_deltas))
+                delta = res.resource_deltas[0]
+                self.assertEqual(1, delta.amount)
+                self.assertIn(delta.resource, self.resources)
+
+        # Delete the expired reservations and check.
+        for project in self.projects:
+            # NOTE(ralonsoh): the timeout is set to -121 to force the deletion
+            # of all created reservations, including those ones created in this
+            # test. The value of 121 overcomes the 120 seconds of default
+            # expiration time a reservation has.
+            time_delta = quota_api.RESERVATION_EXPIRATION_TIMEOUT + 1
+            self.quota_driver._remove_expired_reservations(
+                self.context, project, -time_delta)
+            res = quota_obj.Reservation.get_objects(self.context,
+                                                    project_id=project)
+            self.assertEqual([], res)