]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Remove locking from network and subnet delete op
authorrossella <rsblendido@suse.com>
Tue, 19 Aug 2014 17:41:16 +0000 (19:41 +0200)
committerRossella Sblendido <rsblendido@suse.com>
Tue, 16 Dec 2014 13:51:02 +0000 (13:51 +0000)
delete_subnet in Ml2 plugin instead of using SELECT FOR
UPDATE deletes the IPAllocations that can be auto-deleted
straight away.
An exception is raised if there are ports that cannot be
autodeleted.

delete_network in ML2 plugin tries to delete all ports
and subnets before performing the network deletion.
No lock is needed here - if some other process modifies
the Port or Subnet table, adding new items, the network
deletion will fail because of a violation of a foreign
key contraint.
In that case the operation will be retried.

Change-Id: Ib4e9441a95d6c80b92a43d55fdeb18d7b51a1cf3
Closes-bug: #1332917

neutron/plugins/ml2/plugin.py

index 380141d2e5f5b5045e143f0f5f037dc475c6894c..18b044ea429240ed170141595c1ffcd264653972 100644 (file)
@@ -576,11 +576,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         session = context.session
         while True:
             try:
-                # REVISIT(rkukura): Its not clear that
-                # with_lockmode('update') is really needed in this
-                # transaction, and if not, the semaphore can also be
-                # removed.
-                #
                 # REVISIT: Serialize this operation with a semaphore
                 # to prevent deadlock waiting to acquire a DB lock
                 # held by another thread in the same process, leading
@@ -592,13 +587,14 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                 # Additionally, a rollback may not be enough to undo the
                 # deletion of a floating IP with certain L3 backends.
                 self._process_l3_delete(context, id)
+                # Using query().with_lockmode isn't necessary. Foreign-key
+                # constraints prevent deletion if concurrent creation happens.
                 with contextlib.nested(lockutils.lock('db-access'),
                                        session.begin(subtransactions=True)):
                     # Get ports to auto-delete.
                     ports = (session.query(models_v2.Port).
                              enable_eagerloads(False).
-                             filter_by(network_id=id).
-                             with_lockmode('update').all())
+                             filter_by(network_id=id).all())
                     LOG.debug("Ports to auto-delete: %s", ports)
                     only_auto_del = all(p.device_owner
                                         in db_base_plugin_v2.
@@ -611,8 +607,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                     # Get subnets to auto-delete.
                     subnets = (session.query(models_v2.Subnet).
                                enable_eagerloads(False).
-                               filter_by(network_id=id).
-                               with_lockmode('update').all())
+                               filter_by(network_id=id).all())
                     LOG.debug("Subnets to auto-delete: %s", subnets)
 
                     if not (ports or subnets):
@@ -722,20 +717,33 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                                    session.begin(subtransactions=True)):
                 record = self._get_subnet(context, id)
                 subnet = self._make_subnet_dict(record, None)
-                # Get ports to auto-deallocate
-                allocated = (session.query(models_v2.IPAllocation).
-                             filter_by(subnet_id=id).
-                             join(models_v2.Port).
-                             filter_by(network_id=subnet['network_id']).
-                             with_lockmode('update').all())
+                qry_allocated = (session.query(models_v2.IPAllocation).
+                                 filter_by(subnet_id=id).
+                                 join(models_v2.Port))
+                is_ipv6_slaac_subnet = ipv6_utils.is_slaac_subnet(subnet)
+                # Remove network owned ports, and delete IP allocations
+                # for IPv6 addresses which were automatically generated
+                # via SLAAC
+                if not is_ipv6_slaac_subnet:
+                    qry_allocated = (
+                        qry_allocated.filter(models_v2.Port.device_owner.
+                        in_(db_base_plugin_v2.AUTO_DELETE_PORT_OWNERS)))
+                allocated = qry_allocated.all()
+                # Delete all the IPAllocation that can be auto-deleted
+                if allocated:
+                    map(session.delete, allocated)
                 LOG.debug("Ports to auto-deallocate: %s", allocated)
-                only_auto_del = ipv6_utils.is_slaac_subnet(subnet) or all(
-                    not a.port_id or a.ports.device_owner in db_base_plugin_v2.
-                    AUTO_DELETE_PORT_OWNERS for a in allocated)
-                if not only_auto_del:
+                # Check if there are tenant owned ports
+                tenant_port = (session.query(models_v2.IPAllocation).
+                               filter_by(subnet_id=id).
+                               join(models_v2.Port).
+                               first())
+                if tenant_port:
                     LOG.debug("Tenant-owned ports exist")
                     raise exc.SubnetInUse(subnet_id=id)
 
+                # If allocated is None, then all the IPAllocation were
+                # correctly deleted during the previous pass.
                 if not allocated:
                     mech_context = driver_context.SubnetContext(self, context,
                                                                 subnet)
@@ -763,7 +771,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                         with excutils.save_and_reraise_exception():
                             LOG.exception(_LE("Exception deleting fixed_ip "
                                               "from port %s"), a.port_id)
-                session.delete(a)
 
         try:
             self.mechanism_manager.delete_subnet_postcommit(mech_context)