_update_router_db: don't hold open transactions

This patch prevents the L3 _update_router_db method from
starting a transaction before calling the gateway interface
removal functions. With these port changes now occuring
outside of the L3 DB transaction, a failure to update the
router DB information will not rollback the port deletion
operation.

The 'VPN in use' check had to be moved inside of the DB deletion
transaction now that there isn't an enclosing transaction to undo
the delete when an 'in use' error is raised.

===Details===

The router update db method starts a transaction and calls
the gateway update method with the transaction held open.
This becomes a problem when the update results in an
interface removal which uses a port table lock.

Because the delete_port caller is still holding open a
transaction, other sessions are blocked from getting an
SQL lock on the same tables when delete_port starts
performing RPC notifications, external controller calls,
etc. During those external calls, eventlet will
yield and another thread may try to get a lock on the
port table, causing the infamous mysql/eventlet deadlock.

This separation of L2/L3 transactions is similiar to change
I3ae7bb269df9b9dcef94f48f13f1bde1e4106a80 in nature. Even
though there is a loss in the atomic behavior of the interface
removal operation, it was arguably incorrect to begin with.
The restoration of port DB records during a rollback after some
other failure doesn't undo the backend operations (e.g. REST calls)
that happened during the original deletion. So, having a delete
rollback without corresponding 'create_port' calls to the backend
causes a loss in consistency.

Closes-Bug: #1377241
Change-Id: I5fdb6b24bf2fb80ac5e36a742aa7056db72c8c7d

diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py
index 69357afce4bd55522eea4471d019bc0e723d4b41..ee024c20578d382fb6e1a0492f140832d9b98205 100644 (file)
--- a/neutron/db/l3_db.py
+++ b/neutron/db/l3_db.py
@@ -169,10 +169,8 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
         return self._make_router_dict(router_db)
 
     def _update_router_db(self, context, router_id, data, gw_info):
-        """Update the DB object and related gw info, if available."""
+        """Update the DB object."""
         with context.session.begin(subtransactions=True):
-            if gw_info != attributes.ATTR_NOT_SPECIFIED:
-                self._update_router_gw_info(context, router_id, gw_info)
             router_db = self._get_router(context, router_id)
             if data:
                 router_db.update(data)
@@ -188,6 +186,10 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
         if gw_info != attributes.ATTR_NOT_SPECIFIED:
             candidates = self._check_router_needs_rescheduling(
                 context, id, gw_info)
+            # Update the gateway outside of the DB update since it involves L2
+            # calls that don't make sense to rollback and may cause deadlocks
+            # in a transaction.
+            self._update_router_gw_info(context, id, gw_info)
         else:
             candidates = None
         router_db = self._update_router_db(context, id, r, gw_info)
@@ -316,10 +318,10 @@ class L3_NAT_dbonly_mixin(l3.RouterPluginBase):
                 router.gw_port = None
                 context.session.add(router)
                 context.session.expire(gw_port)
-            vpnservice = manager.NeutronManager.get_service_plugins().get(
-                constants.VPN)
-            if vpnservice:
-                vpnservice.check_router_in_use(context, router_id)
+                vpnservice = manager.NeutronManager.get_service_plugins().get(
+                    constants.VPN)
+                if vpnservice:
+                    vpnservice.check_router_in_use(context, router_id)
             self._core_plugin.delete_port(
                 admin_ctx, gw_port['id'], l3_port_check=False)