Avoid DB errors when deleting network's ports and subnets

DB errors may occur when accessing query results
after the transaction was closed (like ObjectDeletedError).
Hence it's better to avoid DB object access especially
when it's not needed.
This patch changes _delete_ports() and _delete_subnets() to accept
only ids. Indeed, there is no need to pass db objects to these methods.

Closes-Bug: #1484135
Related-Bug: #1454408
Change-Id: I7507cb1c85defb2e6d5144e5832aea713d6251ae

diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index 8035e6a21fe23854717e40e17a65125e6c079886..0366b8291a44937a59baf0047885962ee3825219 100644 (file)
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -699,32 +699,30 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
 
         return [self._fields(net, fields) for net in nets]
 
-    def _delete_ports(self, context, ports):
-        for port in ports:
+    def _delete_ports(self, context, port_ids):
+        for port_id in port_ids:
             try:
-                self.delete_port(context, port.id)
-            except (exc.PortNotFound, sa_exc.ObjectDeletedError):
-                context.session.expunge(port)
+                self.delete_port(context, port_id)
+            except exc.PortNotFound:
                 # concurrent port deletion can be performed by
                 # release_dhcp_port caused by concurrent subnet_delete
-                LOG.info(_LI("Port %s was deleted concurrently"), port.id)
+                LOG.info(_LI("Port %s was deleted concurrently"), port_id)
             except Exception:
                 with excutils.save_and_reraise_exception():
                     LOG.exception(_LE("Exception auto-deleting port %s"),
-                                  port.id)
+                                  port_id)
 
-    def _delete_subnets(self, context, subnets):
-        for subnet in subnets:
+    def _delete_subnets(self, context, subnet_ids):
+        for subnet_id in subnet_ids:
             try:
-                self.delete_subnet(context, subnet.id)
-            except (exc.SubnetNotFound, sa_exc.ObjectDeletedError):
-                context.session.expunge(subnet)
+                self.delete_subnet(context, subnet_id)
+            except exc.SubnetNotFound:
                 LOG.info(_LI("Subnet %s was deleted concurrently"),
-                         subnet.id)
+                         subnet_id)
             except Exception:
                 with excutils.save_and_reraise_exception():
                     LOG.exception(_LE("Exception auto-deleting subnet %s"),
-                                  subnet.id)
+                                  subnet_id)
 
     def delete_network(self, context, id):
         # REVISIT(rkukura) The super(Ml2Plugin, self).delete_network()
@@ -788,6 +786,9 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                         # network record, so explicit removal is not necessary.
                         LOG.debug("Committing transaction")
                         break
+
+                    port_ids = [port.id for port in ports]
+                    subnet_ids = [subnet.id for subnet in subnets]
             except os_db_exception.DBError as e:
                 with excutils.save_and_reraise_exception() as ctxt:
                     if isinstance(e.inner_exception, sql_exc.IntegrityError):
@@ -795,8 +796,8 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                         LOG.warning(_LW("A concurrent port creation has "
                                         "occurred"))
                         continue
-            self._delete_ports(context, ports)
-            self._delete_subnets(context, subnets)
+            self._delete_ports(context, port_ids)
+            self._delete_subnets(context, subnet_ids)
 
         try:
             self.mechanism_manager.delete_network_postcommit(mech_context)

diff --git a/neutron/tests/unit/plugins/ml2/test_plugin.py b/neutron/tests/unit/plugins/ml2/test_plugin.py
index 56c0a3d270be1a1d20c1fd0fa4e74685326774e3..7dc84b50c412f563a667098578f503ccf8b3564f 100644 (file)
--- a/neutron/tests/unit/plugins/ml2/test_plugin.py
+++ b/neutron/tests/unit/plugins/ml2/test_plugin.py
@@ -24,7 +24,6 @@ import webob
 
 from oslo_db import exception as db_exc
 from oslo_utils import uuidutils
-from sqlalchemy.orm import exc as sqla_exc
 
 from neutron.callbacks import registry
 from neutron.common import constants
@@ -220,18 +219,12 @@ class TestMl2NetworksV2(test_plugin.TestNetworksV2,
         with mock.patch.object(plugin, "delete_port",
                                side_effect=exc.PortNotFound(port_id="123")):
             plugin._delete_ports(mock.MagicMock(), [mock.MagicMock()])
-        with mock.patch.object(plugin, "delete_port",
-                               side_effect=sqla_exc.ObjectDeletedError(None)):
-            plugin._delete_ports(mock.MagicMock(), [mock.MagicMock()])
 
     def test_subnet_delete_helper_tolerates_failure(self):
         plugin = manager.NeutronManager.get_plugin()
         with mock.patch.object(plugin, "delete_subnet",
                                side_effect=exc.SubnetNotFound(subnet_id="1")):
             plugin._delete_subnets(mock.MagicMock(), [mock.MagicMock()])
-        with mock.patch.object(plugin, "delete_subnet",
-                               side_effect=sqla_exc.ObjectDeletedError(None)):
-            plugin._delete_subnets(mock.MagicMock(), [mock.MagicMock()])
 
     def _create_and_verify_networks(self, networks):
         for net_idx, net in enumerate(networks):