]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
NSX: Sync do not pass around model object
authorAaron Rosen <arosen@nicira.com>
Tue, 18 Feb 2014 20:22:42 +0000 (12:22 -0800)
committerAaron Rosen <aaronorosen@gmail.com>
Mon, 10 Mar 2014 23:42:00 +0000 (16:42 -0700)
The NSX sync backend previously passed around a sqlalchemy model object
around which was nice because we did not need to query the database an
additional time to update the status of an object.

Unfortunately, this was add done within a db transaction which included a
call to NSX which could cause deadlock this needed to be removed. Now a
dict is passed around instead and proper transaction handling
is used when updating objects.

Change-Id: Id50abd8f2143b127d1ca81c360c1c6d400e1d74f
Closes-bug: #1281772

neutron/plugins/vmware/common/sync.py

index 229ff342713275365b0bcb944bff1cee8702bd25..1b9c4926bd02399e9afe078dc470763369b8e3cd 100644 (file)
@@ -15,6 +15,8 @@
 
 import random
 
+from sqlalchemy.orm import exc
+
 from neutron.common import constants
 from neutron.common import exceptions
 from neutron import context
@@ -235,17 +237,6 @@ class NsxSynchronizer():
     def _get_tag_dict(self, tags):
         return dict((tag.get('scope'), tag['tag']) for tag in tags)
 
-    def _update_neutron_object(self, context, neutron_data, status):
-        if status == neutron_data['status']:
-            # do nothing
-            return
-        with context.session.begin(subtransactions=True):
-            LOG.debug(_("Updating status for neutron resource %(q_id)s to: "
-                        "%(status)s"), {'q_id': neutron_data['id'],
-                                        'status': status})
-            neutron_data['status'] = status
-            context.session.add(neutron_data)
-
     def synchronize_network(self, context, neutron_network_data,
                             lswitches=None):
         """Synchronize a Neutron network with its NSX counterpart.
@@ -286,7 +277,22 @@ class NsxSynchronizer():
             if lswitches:
                 status = constants.NET_STATUS_ACTIVE
         # Update db object
-        self._update_neutron_object(context, neutron_network_data, status)
+        if status == neutron_network_data['status']:
+            # do nothing
+            return
+
+        with context.session.begin(subtransactions=True):
+            try:
+                network = self._plugin._get_network(context,
+                                                    neutron_network_data['id'])
+            except exc.NoResultFound:
+                pass
+            else:
+                network.status = status
+                LOG.debug(_("Updating status for neutron resource %(q_id)s to:"
+                            " %(status)s"),
+                          {'q_id': neutron_network_data['id'],
+                           'status': status})
 
     def _synchronize_lswitches(self, ctx, ls_uuids, scan_missing=False):
         if not ls_uuids and not scan_missing:
@@ -309,8 +315,11 @@ class NsxSynchronizer():
         filters = {'router:external': [False]}
         if not scan_missing:
             filters['id'] = neutron_net_ids
-        networks = self._plugin._get_collection_query(
-            ctx, models_v2.Network, filters=filters)
+
+        networks = self._plugin._get_collection(
+            ctx, models_v2.Network, self._plugin._make_network_dict,
+            filters=filters)
+
         for network in networks:
             lswitches = neutron_nsx_mappings.get(network['id'], [])
             lswitches = [lswitch.get('data') for lswitch in lswitches]
@@ -350,7 +359,22 @@ class NsxSynchronizer():
                       constants.NET_STATUS_ACTIVE
                       or constants.NET_STATUS_DOWN)
         # Update db object
-        self._update_neutron_object(context, neutron_router_data, status)
+        if status == neutron_router_data['status']:
+            # do nothing
+            return
+
+        with context.session.begin(subtransactions=True):
+            try:
+                router = self._plugin._get_router(context,
+                                                  neutron_router_data['id'])
+            except exc.NoResultFound:
+                pass
+            else:
+                router.status = status
+                LOG.debug(_("Updating status for neutron resource %(q_id)s to:"
+                            " %(status)s"),
+                          {'q_id': neutron_router_data['id'],
+                           'status': status})
 
     def _synchronize_lrouters(self, ctx, lr_uuids, scan_missing=False):
         if not lr_uuids and not scan_missing:
@@ -372,8 +396,9 @@ class NsxSynchronizer():
         # Fetch neutron routers from database
         filters = ({} if scan_missing else
                    {'id': neutron_router_mappings.keys()})
-        routers = self._plugin._get_collection_query(
-            ctx, l3_db.Router, filters=filters)
+        routers = self._plugin._get_collection(
+            ctx, l3_db.Router, self._plugin._make_router_dict,
+            filters=filters)
         for router in routers:
             lrouter = neutron_router_mappings.get(router['id'])
             self.synchronize_router(
@@ -427,8 +452,24 @@ class NsxSynchronizer():
             status = (lp_status and
                       constants.PORT_STATUS_ACTIVE
                       or constants.PORT_STATUS_DOWN)
+
         # Update db object
-        self._update_neutron_object(context, neutron_port_data, status)
+        if status == neutron_port_data['status']:
+            # do nothing
+            return
+
+        with context.session.begin(subtransactions=True):
+            try:
+                port = self._plugin._get_port(context,
+                                              neutron_port_data['id'])
+            except exc.NoResultFound:
+                pass
+            else:
+                port.status = status
+                LOG.debug(_("Updating status for neutron resource %(q_id)s to:"
+                            " %(status)s"),
+                          {'q_id': neutron_port_data['id'],
+                           'status': status})
 
     def _synchronize_lswitchports(self, ctx, lp_uuids, scan_missing=False):
         if not lp_uuids and not scan_missing:
@@ -457,8 +498,9 @@ class NsxSynchronizer():
                 external_net_db.ExternalNetwork,
                 (models_v2.Network.id ==
                  external_net_db.ExternalNetwork.network_id))]
-        ports = self._plugin._get_collection_query(
-            ctx, models_v2.Port, filters=filters)
+        ports = self._plugin._get_collection(
+            ctx, models_v2.Port, self._plugin._make_port_dict,
+            filters=filters)
         for port in ports:
             lswitchport = neutron_port_mappings.get(port['id'])
             self.synchronize_port(
@@ -599,13 +641,12 @@ class NsxSynchronizer():
         # Get an admin context
         ctx = context.get_admin_context()
         # Synchronize with database
-        with ctx.session.begin(subtransactions=True):
-            self._synchronize_lswitches(ctx, ls_uuids,
-                                        scan_missing=scan_missing)
-            self._synchronize_lrouters(ctx, lr_uuids,
+        self._synchronize_lswitches(ctx, ls_uuids,
+                                    scan_missing=scan_missing)
+        self._synchronize_lrouters(ctx, lr_uuids,
+                                   scan_missing=scan_missing)
+        self._synchronize_lswitchports(ctx, lp_uuids,
                                        scan_missing=scan_missing)
-            self._synchronize_lswitchports(ctx, lp_uuids,
-                                           scan_missing=scan_missing)
         # Increase chunk counter
         LOG.info(_("Synchronization for chunk %(chunk_num)d of "
                    "%(total_chunks)d performed"),