From a8bddee4f43c2772e4ca96acdee9b95feec733a9 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Tue, 18 Aug 2015 10:01:50 -0700 Subject: [PATCH] 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 --- doc/source/devref/quota.rst | 6 +++ neutron/api/v2/base.py | 85 +++++++++++++------------------------ 2 files changed, 35 insertions(+), 56 deletions(-) diff --git a/doc/source/devref/quota.rst b/doc/source/devref/quota.rst index ae0f99279..53bd6ce51 100644 --- 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 5f808a2a9..69a88d230 100644 --- 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, -- 2.45.2