From 35e595f7b3d85eb6053a50ab08044bd89eb554ab Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Thu, 31 Oct 2013 16:53:31 -0700 Subject: [PATCH] Fix DB integrity issues when using postgres 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 (cherry picked from commit 54d79acaab50993f08b50f21019f82f17d38caa3) --- neutron/db/db_base_plugin_v2.py | 6 ++- neutron/plugins/ml2/plugin.py | 94 ++++++++++++++++++--------------- 2 files changed, 55 insertions(+), 45 deletions(-) diff --git a/neutron/db/db_base_plugin_v2.py b/neutron/db/db_base_plugin_v2.py index 52fc45389aa..2afbac544de 100644 --- a/neutron/db/db_base_plugin_v2.py +++ b/neutron/db/db_base_plugin_v2.py @@ -1403,7 +1403,11 @@ class NeutronDbPluginV2(neutron_plugin_base_v2.NeutronPluginBaseV2, self._delete_port(context, 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') diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py index af7d6772beb..4f7e556903c 100644 --- a/neutron/plugins/ml2/plugin.py +++ b/neutron/plugins/ml2/plugin.py @@ -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) + 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). - filter_by(network_id=id). - all()) - LOG.debug(_("Subnets to auto-delete: %s"), subnets) + # 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) + 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) + 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) + 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 + # 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)