Stop using quota reservations on base controller

The reservation engine is subject to failures due to concurrency;
the switch to pymysql is likely to also have a part in observed
failures. While no gate failures have been observed so far, this
is a time bomb waiting to explode and must be addressed.

For this reason this patch acts conservatively by ensuring the
API controllers do not use anymore reservation. The code for
reservation management is preserved, and will wired again on the
controller when these issues are sorted.

The devref for neutron quotas is updated accordingly as a part
of this patch.

Related bug: #1486134

Change-Id: I2a95fef0fdf64ef8781bef99be0fdc743346c17a

diff --git a/doc/source/devref/quota.rst b/doc/source/devref/quota.rst
index ae0f9927910c3fc93eaef565bc3fffcd93eceb7c..53bd6ce515b439b92e2d603bc321911a04dffbfe 100644 (file)
--- a/doc/source/devref/quota.rst
+++ b/doc/source/devref/quota.rst
@@ -164,6 +164,12 @@ difference between CountableResource and TrackedResource.
 Quota Enforcement
 -----------------
 
+**NOTE: The reservation engine is currently not wired into the API controller
+as issues have been discovered with multiple workers. For more information
+see _bug1468134**
+
+.. _bug1468134: https://bugs.launchpad.net/neutron/+bug/1486134
+
 Before dispatching a request to the plugin, the Neutron 'base' controller [#]_
 attempts to make a reservation for requested resource(s).
 Reservations are made by calling the make_reservation method in

diff --git a/neutron/api/v2/base.py b/neutron/api/v2/base.py
index 5f808a2a98085deea27a87c0be3614549e4d4f82..69a88d230b2b496824805f0b20025e799c496494 100644 (file)
--- a/neutron/api/v2/base.py
+++ b/neutron/api/v2/base.py
@@ -416,15 +416,13 @@ class Controller(object):
         if self._collection in body:
             # Have to account for bulk create
             items = body[self._collection]
+            deltas = {}
+            bulk = True
         else:
             items = [body]
+            bulk = False
         # Ensure policy engine is initialized
         policy.init()
-        # Store requested resource amounts grouping them by tenant
-        # This won't work with multiple resources. However because of the
-        # current structure of this controller there will hardly be more than
-        # one resource for which reservations are being made
-        request_deltas = {}
         for item in items:
             self._validate_network_tenant_ownership(request,
                                                     item[self._resource])
@@ -435,34 +433,30 @@ class Controller(object):
             if 'tenant_id' not in item[self._resource]:
                 # no tenant_id - no quota check
                 continue
-            tenant_id = item[self._resource]['tenant_id']
-            delta = request_deltas.get(tenant_id, 0)
-            delta = delta + 1
-            request_deltas[tenant_id] = delta
-        # Quota enforcement
-        reservations = []
-        try:
-            for tenant in request_deltas:
-                reservation = quota.QUOTAS.make_reservation(
-                    request.context,
-                    tenant,
-                    {self._resource:
-                     request_deltas[tenant]},
-                    self._plugin)
-                reservations.append(reservation)
-        except exceptions.QuotaResourceUnknown as e:
+            try:
+                tenant_id = item[self._resource]['tenant_id']
+                count = quota.QUOTAS.count(request.context, self._resource,
+                                           self._plugin, tenant_id)
+                if bulk:
+                    delta = deltas.get(tenant_id, 0) + 1
+                    deltas[tenant_id] = delta
+                else:
+                    delta = 1
+                kwargs = {self._resource: count + delta}
+            except exceptions.QuotaResourceUnknown as e:
                 # We don't want to quota this resource
                 LOG.debug(e)
+            else:
+                quota.QUOTAS.limit_check(request.context,
+                                         item[self._resource]['tenant_id'],
+                                         **kwargs)
 
         def notify(create_result):
             # Ensure usage trackers for all resources affected by this API
             # operation are marked as dirty
-            with request.context.session.begin():
-                # Commit the reservation(s)
-                for reservation in reservations:
-                    quota.QUOTAS.commit_reservation(
-                        request.context, reservation.reservation_id)
-                resource_registry.set_resources_dirty(request.context)
+            # TODO(salv-orlando): This operation will happen in a single
+            # transaction with reservation commit once that is implemented
+            resource_registry.set_resources_dirty(request.context)
 
             notifier_method = self._resource + '.create.end'
             self._notifier.info(request.context,
@@ -473,35 +467,11 @@ class Controller(object):
                                          notifier_method)
             return create_result
 
-        def do_create(body, bulk=False, emulated=False):
-            kwargs = {self._parent_id_name: parent_id} if parent_id else {}
-            if bulk and not emulated:
-                obj_creator = getattr(self._plugin, "%s_bulk" % action)
-            else:
-                obj_creator = getattr(self._plugin, action)
-            try:
-                if emulated:
-                    return self._emulate_bulk_create(obj_creator, request,
-                                                     body, parent_id)
-                else:
-                    if self._collection in body:
-                        # This is weird but fixing it requires changes to the
-                        # plugin interface
-                        kwargs.update({self._collection: body})
-                    else:
-                        kwargs.update({self._resource: body})
-                    return obj_creator(request.context, **kwargs)
-            except Exception:
-                # In case of failure the plugin will always raise an
-                # exception. Cancel the reservation
-                with excutils.save_and_reraise_exception():
-                    for reservation in reservations:
-                        quota.QUOTAS.cancel_reservation(
-                            request.context, reservation.reservation_id)
-
+        kwargs = {self._parent_id_name: parent_id} if parent_id else {}
         if self._collection in body and self._native_bulk:
             # plugin does atomic bulk create operations
-            objs = do_create(body, bulk=True)
+            obj_creator = getattr(self._plugin, "%s_bulk" % action)
+            objs = obj_creator(request.context, body, **kwargs)
             # Use first element of list to discriminate attributes which
             # should be removed because of authZ policies
             fields_to_strip = self._exclude_attributes_by_policy(
@@ -510,12 +480,15 @@ class Controller(object):
                 request.context, obj, fields_to_strip=fields_to_strip)
                 for obj in objs]})
         else:
+            obj_creator = getattr(self._plugin, action)
             if self._collection in body:
                 # Emulate atomic bulk behavior
-                objs = do_create(body, bulk=True, emulated=True)
+                objs = self._emulate_bulk_create(obj_creator, request,
+                                                 body, parent_id)
                 return notify({self._collection: objs})
             else:
-                obj = do_create(body)
+                kwargs.update({self._resource: body})
+                obj = obj_creator(request.context, **kwargs)
                 self._send_nova_notification(action, {},
                                              {self._resource: obj})
                 return notify({self._resource: self._view(request.context,