From: Kevin Benton <blak111@gmail.com>
Date: Thu, 16 Oct 2014 08:49:19 +0000 (-0700)
Subject: _update_router_db: don't hold open transactions
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=a575cb7cb6c30d843c3ac824e5f2f11a5055a9fb;p=openstack-build%2Fneutron-build.git

_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.

Conflicts:

	neutron/db/l3_db.py

Closes-Bug: #1377241
Change-Id: I5fdb6b24bf2fb80ac5e36a742aa7056db72c8c7d
(cherry picked from commit f23f2ecee68ba4abd12139bbb91b77ba9410f581)
---

diff --git a/neutron/db/l3_db.py b/neutron/db/l3_db.py
index 0f8a56c0e..a8d3c217a 100644
--- 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)