BSN: Remove db lock and add missing contexts

Adds context tracking decorators that were missing
from router interface methods. Without them, new
sessions were being created instead of using the
existing context which was causing transaction
issues.

Modifies the servermanager to store context references
as weakrefs so if multiple functions are called before
the rest functions are called, the first one doesn't steal
the only context reference with a pop() call.

Removes a DB lock for update in the server manager that occured
during rest calls that was triggering deadlocks due to the
file lock synchronization for the rest calls.

Closes-Bug: #1336251
Change-Id: Iad3d61e2c23832b3ad760a999fbab7feaa13f805

neutron/plugins/bigswitch/db/consistency_db.py

@@ -55,13 +55,13 @@ class HashHandler(object):
         if self.transaction:
             raise MultipleReadForUpdateCalls()
         self.transaction = self.session.begin(subtransactions=True)
-        # Lock for update here to prevent another server from reading the hash
-        # while this one is in the middle of a transaction.
-        # This may not lock the SQL table in MySQL Galera deployments
-        # but that's okay because the worst case is a double-sync
+        # REVISIT(kevinbenton): locking here with the DB is prone to deadlocks
+        # in various multi-REST-call scenarios (router intfs, flips, etc).
+        # Since it doesn't work in Galera deployments anyway, another sync
+        # mechanism will have to be introduced to prevent inefficient double
+        # syncs in HA deployments.
         res = (self.session.query(ConsistencyHash).
-               filter_by(hash_id=self.hash_id).
-               with_lockmode('update').first())
+               filter_by(hash_id=self.hash_id).first())
         if not res:
             return ''
         self.hash_db_obj = res

neutron/plugins/bigswitch/plugin.py

@@ -968,6 +968,7 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
             self.servers.rest_delete_router(tenant_id, router_id)
             return ret_val
 
+    @put_context_in_serverpool
     def add_router_interface(self, context, router_id, interface_info):
 
         LOG.debug(_("NeutronRestProxyV2: add_router_interface() called"))
@@ -996,6 +997,7 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
                                                    intf_details)
             return new_intf_info
 
+    @put_context_in_serverpool
     def remove_router_interface(self, context, router_id, interface_info):
 
         LOG.debug(_("NeutronRestProxyV2: remove_router_interface() called"))
@@ -1087,6 +1089,7 @@ class NeutronRestProxyV2(NeutronRestProxyV2Base,
             else:
                 self._send_floatingip_update(context)
 
+    @put_context_in_serverpool
     def disassociate_floatingips(self, context, port_id):
         LOG.debug(_("NeutronRestProxyV2: diassociate_floatingips() called"))
         super(NeutronRestProxyV2, self).disassociate_floatingips(context,

neutron/plugins/bigswitch/servermanager.py

@@ -35,6 +35,7 @@ import httplib
 import os
 import socket
 import ssl
+import weakref
 
 import eventlet
 import eventlet.corolocal
@@ -266,14 +267,18 @@ class ServerPool(object):
 
     def set_context(self, context):
         # this context needs to be local to the greenthread
-        # so concurrent requests don't use the wrong context
-        self.contexts[eventlet.corolocal.get_ident()] = context
+        # so concurrent requests don't use the wrong context.
+        # Use a weakref so the context is garbage collected
+        # after the plugin is done with it.
+        ref = weakref.ref(context)
+        self.contexts[eventlet.corolocal.get_ident()] = ref
 
-    def pop_context(self):
-        # Don't store these contexts after use. They should only
-        # last for one request.
+    def get_context_ref(self):
+        # Try to get the context cached for this thread. If one
+        # doesn't exist or if it's been garbage collected, this will
+        # just return None.
         try:
-            return self.contexts.pop(eventlet.corolocal.get_ident())
+            return self.contexts[eventlet.corolocal.get_ident()]()
         except KeyError:
             return None
 
@@ -403,7 +408,7 @@ class ServerPool(object):
     @utils.synchronized('bsn-rest-call')
     def rest_call(self, action, resource, data, headers, ignore_codes,
                   timeout=False):
-        hash_handler = cdb.HashHandler(context=self.pop_context())
+        hash_handler = cdb.HashHandler(context=self.get_context_ref())
         good_first = sorted(self.servers, key=lambda x: x.failed)
         first_response = None
         for active_server in good_first: