]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Fix DB integrity issues when using postgres
authorarmando-migliaccio <amigliaccio@nicira.com>
Thu, 31 Oct 2013 23:53:31 +0000 (16:53 -0700)
committerarmando-migliaccio <amigliaccio@nicira.com>
Thu, 14 Nov 2013 16:14:31 +0000 (08:14 -0800)
The IntegrityError faced on network deletion that
spooks Tempest tests is most likely caused by the
fact that the ml2 plugin does a dirty read. By
adding a 'select for update' we should be able to
address the issue once and for all.

Also, there's a chance that the dhcp port gets
reallocated while we are deleting the network. To
this aim, catch the integrity error and attempt
to recover from it.

This patch also removes the handling of errors
on concurrent reads that can no longer occur when
deleting the network.

(Hopefully) Closes-bug: #1239637

PS: This fix may be relevant for bugs #1243726 and #1246737

Change-Id: Iefae7fc8a903cc35d8bf80fc4cee533b7f64032c

neutron/db/db_base_plugin_v2.py
neutron/plugins/ml2/plugin.py

index 58490bca656f7161a9be822a13f53dc70f900698..ae7fba648485d5ba7808807abf70b9a060b77e90 100644 (file)
@@ -1416,7 +1416,11 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2,
                 self.delete_port(context, port['id'])
 
     def _delete_port(self, context, id):
-        port = self._get_port(context, id)
+        query = (context.session.query(models_v2.Port).
+                 enable_eagerloads(False).filter_by(id=id))
+        if not context.is_admin:
+            query = query.filter_by(tenant_id=context.tenant_id)
+        port = query.with_lockmode('update').one()
 
         allocated_qry = context.session.query(
             models_v2.IPAllocation).with_lockmode('update')
index af7d6772beb1c12324149909dde76a9f94fd7d4e..4f7e556903cfabd40bbd0fbf717baeaf2e2f11da 100644 (file)
@@ -14,7 +14,7 @@
 #    under the License.
 
 from oslo.config import cfg
-from sqlalchemy import orm
+from sqlalchemy import exc as sql_exc
 
 from neutron.agent import securitygroups_rpc as sg_rpc
 from neutron.api.rpc.agentnotifiers import dhcp_rpc_agent_api
@@ -36,6 +36,7 @@ from neutron.extensions import multiprovidernet as mpnet
 from neutron.extensions import portbindings
 from neutron.extensions import providernet as provider
 from neutron import manager
+from neutron.openstack.common import db as os_db
 from neutron.openstack.common import excutils
 from neutron.openstack.common import importutils
 from neutron.openstack.common import log
@@ -378,52 +379,60 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
 
         LOG.debug(_("Deleting network %s"), id)
         session = context.session
-        filter = {'network_id': [id]}
         while True:
-            with session.begin(subtransactions=True):
-                # Get ports to auto-delete.
-                ports = (self._get_ports_query(context, filters=filter).
-                         all())
-                LOG.debug(_("Ports to auto-delete: %s"), ports)
-                only_auto_del = all(p.device_owner
-                                    in db_base_plugin_v2.
-                                    AUTO_DELETE_PORT_OWNERS
-                                    for p in ports)
-                if not only_auto_del:
-                    LOG.debug(_("Tenant-owned ports exist"))
-                    raise exc.NetworkInUse(net_id=id)
-
-                # Get subnets to auto-delete.
-                subnets = (session.query(models_v2.Subnet).
-                           filter_by(network_id=id).
-                           all())
-                LOG.debug(_("Subnets to auto-delete: %s"), subnets)
-
-                if not (ports or subnets):
-                    network = self.get_network(context, id)
-                    mech_context = driver_context.NetworkContext(self,
-                                                                 context,
-                                                                 network)
-                    self.mechanism_manager.delete_network_precommit(
-                        mech_context)
-
-                    LOG.debug(_("Deleting network record"))
-                    record = self._get_network(context, id)
-                    session.delete(record)
-
-                    for segment in mech_context.network_segments:
-                        self.type_manager.release_segment(session, segment)
-
-                    # The segment records are deleted via cascade from the
-                    # network record, so explicit removal is not necessary.
-                    LOG.debug(_("Committing transaction"))
-                    break
+            try:
+                with 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())
+                    LOG.debug(_("Ports to auto-delete: %s"), ports)
+                    only_auto_del = all(p.device_owner
+                                        in db_base_plugin_v2.
+                                        AUTO_DELETE_PORT_OWNERS
+                                        for p in ports)
+                    if not only_auto_del:
+                        LOG.debug(_("Tenant-owned ports exist"))
+                        raise exc.NetworkInUse(net_id=id)
+
+                    # Get subnets to auto-delete.
+                    subnets = (session.query(models_v2.Subnet).
+                               enable_eagerloads(False).
+                               filter_by(network_id=id).
+                               with_lockmode('update').all())
+                    LOG.debug(_("Subnets to auto-delete: %s"), subnets)
+
+                    if not (ports or subnets):
+                        network = self.get_network(context, id)
+                        mech_context = driver_context.NetworkContext(self,
+                                                                     context,
+                                                                     network)
+                        self.mechanism_manager.delete_network_precommit(
+                            mech_context)
+
+                        record = self._get_network(context, id)
+                        LOG.debug(_("Deleting network record %s"), record)
+                        session.delete(record)
+
+                        for segment in mech_context.network_segments:
+                            self.type_manager.release_segment(session, segment)
+
+                        # The segment records are deleted via cascade from the
+                        # network record, so explicit removal is not necessary.
+                        LOG.debug(_("Committing transaction"))
+                        break
+            except os_db.exception.DBError as e:
+                if isinstance(e.inner_exception, sql_exc.IntegrityError):
+                    msg = _("A concurrent port creation has occurred")
+                    LOG.exception(msg)
+                    continue
+                else:
+                    raise
 
             for port in ports:
                 try:
                     self.delete_port(context, port.id)
-                except exc.PortNotFound:
-                    LOG.debug(_("Port %s concurrently deleted"), port.id)
                 except Exception:
                     LOG.exception(_("Exception auto-deleting port %s"),
                                   port.id)
@@ -432,8 +441,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             for subnet in subnets:
                 try:
                     self.delete_subnet(context, subnet.id)
-                except exc.SubnetNotFound:
-                    LOG.debug(_("Subnet %s concurrently deleted"), subnet.id)
                 except Exception:
                     LOG.exception(_("Exception auto-deleting subnet %s"),
                                   subnet.id)
@@ -493,11 +500,13 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
         session = context.session
         while True:
             with session.begin(subtransactions=True):
+                subnet = self.get_subnet(context, id)
                 # Get ports to auto-delete.
                 allocated = (session.query(models_v2.IPAllocation).
-                             options(orm.joinedload('ports')).
                              filter_by(subnet_id=id).
-                             all())
+                             join(models_v2.Port).
+                             filter_by(network_id=subnet['network_id']).
+                             with_lockmode('update').all())
                 LOG.debug(_("Ports to auto-delete: %s"), allocated)
                 only_auto_del = all(not a.port_id or
                                     a.ports.device_owner in db_base_plugin_v2.
@@ -508,7 +517,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                     raise exc.SubnetInUse(subnet_id=id)
 
                 if not allocated:
-                    subnet = self.get_subnet(context, id)
                     mech_context = driver_context.SubnetContext(self, context,
                                                                 subnet)
                     self.mechanism_manager.delete_subnet_precommit(
@@ -524,8 +532,6 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
             for a in allocated:
                 try:
                     self.delete_port(context, a.port_id)
-                except exc.PortNotFound:
-                    LOG.debug(_("Port %s concurrently deleted"), a.port_id)
                 except Exception:
                     LOG.exception(_("Exception auto-deleting port %s"),
                                   a.port_id)