From 91852a7f529d276052e898480307c727be20fab3 Mon Sep 17 00:00:00 2001 From: salvatore Date: Fri, 21 Aug 2015 10:10:56 +0200 Subject: [PATCH] Quota enforcement: remove locks on _dirty_tenants This lock was used to avoid errors due to list contents changing during iteration, but is causing issues with pymysql. This patch proposes an alternative approach which makes the use of a lock unnecessary. With this change a copy of the dirty_tenants set is made before setting the dirty bit on resources, and then the mark_dirty routine operates on this copy. This still guaranteses operations correctness, as all the tenants that should be marked dirty are marked dirty before the completion of the relevant API request. Related-Blueprint: better-quotas Change-Id: Ib39e7089889d3f906bdc025c843128a1fa3e8797 --- neutron/quota/resource.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/neutron/quota/resource.py b/neutron/quota/resource.py index 900013d80..7068254c7 100644 --- a/neutron/quota/resource.py +++ b/neutron/quota/resource.py @@ -12,7 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -from oslo_concurrency import lockutils from oslo_config import cfg from oslo_db import api as oslo_db_api from oslo_db import exception as oslo_db_exception @@ -177,20 +176,26 @@ class TrackedResource(BaseResource): def dirty(self): return self._dirty_tenants - @lockutils.synchronized('dirty_tenants') def mark_dirty(self, context, nested=False): if not self._dirty_tenants: return with context.session.begin(nested=nested, subtransactions=True): - for tenant_id in self._dirty_tenants: + # It is not necessary to protect this operation with a lock. + # Indeed when this method is called the request has been processed + # and therefore all resources created or deleted. + # dirty_tenants will contain all the tenants for which the + # resource count is changed. The list might contain also tenants + # for which resource count was altered in other requests, but this + # won't be harmful. + dirty_tenants_snap = self._dirty_tenants.copy() + for tenant_id in dirty_tenants_snap: quota_api.set_quota_usage_dirty(context, self.name, tenant_id) LOG.debug(("Persisted dirty status for tenant:%(tenant_id)s " "on resource:%(resource)s"), {'tenant_id': tenant_id, 'resource': self.name}) - self._out_of_sync_tenants |= self._dirty_tenants - self._dirty_tenants.clear() + self._out_of_sync_tenants |= dirty_tenants_snap + self._dirty_tenants = self._dirty_tenants - dirty_tenants_snap - @lockutils.synchronized('dirty_tenants') def _db_event_handler(self, mapper, _conn, target): try: tenant_id = target['tenant_id'] @@ -224,7 +229,6 @@ class TrackedResource(BaseResource): {'tenant_id': tenant_id, 'resource': self.name}) return usage_info - @lockutils.synchronized('dirty_tenants') def resync(self, context, tenant_id): if tenant_id not in self._out_of_sync_tenants: return @@ -236,7 +240,6 @@ class TrackedResource(BaseResource): # Update quota usage return self._resync(context, tenant_id, in_use, reserved=0) - @lockutils.synchronized('dirty_tenants') def count(self, context, _plugin, tenant_id, resync_usage=False): """Return the current usage count for the resource. -- 2.45.2