]> review.fuel-infra Code Review - openstack-build/neutron-build.git/commitdiff
Remove SELECT FOR UPDATE use in ML2 type driver release_segment
authorCedric Brandily <zzelle@gmail.com>
Mon, 16 Jun 2014 20:56:10 +0000 (22:56 +0200)
committerCedric Brandily <zzelle@gmail.com>
Tue, 17 Jun 2014 23:15:56 +0000 (23:15 +0000)
SELECT FOR UPDATE expression, which is triggered with the use of the
SQLAlchemy Query object's with_lockmode('update') method, is
detrimental to performance and scalability of the database
performance code in Neutron due to the lock contention it produces.

SELECT FOR UPDATE can be entirely avoided in release_segment methods
with the use of single-shot UPDATE and DELETE expressions, and this
patch clears a number of uses of SELECT FOR UPDATE by consolidating
multiple SQL expressions into one.

Partial-Bug: #1330562
Change-Id: I29ffcafc8d4d73ac1cb50c94df5da85514d47a3f

neutron/plugins/ml2/drivers/type_flat.py
neutron/plugins/ml2/drivers/type_gre.py
neutron/plugins/ml2/drivers/type_vlan.py
neutron/plugins/ml2/drivers/type_vxlan.py

index 3e736eabc8bc689097ec4564757deb93535d95cc..2cd4104229eff67d9e115d24b64acc895a795e32 100644 (file)
@@ -118,14 +118,12 @@ class FlatTypeDriver(api.TypeDriver):
     def release_segment(self, session, segment):
         physical_network = segment[api.PHYSICAL_NETWORK]
         with session.begin(subtransactions=True):
-            try:
-                alloc = (session.query(FlatAllocation).
-                         filter_by(physical_network=physical_network).
-                         with_lockmode('update').
-                         one())
-                session.delete(alloc)
-                LOG.debug(_("Releasing flat network on physical "
-                            "network %s"), physical_network)
-            except sa.orm.exc.NoResultFound:
-                LOG.warning(_("No flat network found on physical network %s"),
-                            physical_network)
+            count = (session.query(FlatAllocation).
+                     filter_by(physical_network=physical_network).
+                     delete())
+        if count:
+            LOG.debug("Releasing flat network on physical network %s",
+                      physical_network)
+        else:
+            LOG.warning(_("No flat network found on physical network %s"),
+                        physical_network)
index abd894bfe5058f70301275370dadb252fd21aa59..b9a00c0cb86b3a1741b1f8bafff2eab27b661323 100644 (file)
@@ -108,24 +108,22 @@ class GreTypeDriver(type_tunnel.TunnelTypeDriver):
 
     def release_segment(self, session, segment):
         gre_id = segment[api.SEGMENTATION_ID]
+
+        inside = any(lo <= gre_id <= hi for lo, hi in self.gre_id_ranges)
+
         with session.begin(subtransactions=True):
-            try:
-                alloc = (session.query(GreAllocation).
-                         filter_by(gre_id=gre_id).
-                         with_lockmode('update').
-                         one())
-                alloc.allocated = False
-                for lo, hi in self.gre_id_ranges:
-                    if lo <= gre_id <= hi:
-                        LOG.debug(_("Releasing gre tunnel %s to pool"),
-                                  gre_id)
-                        break
-                else:
-                    session.delete(alloc)
-                    LOG.debug(_("Releasing gre tunnel %s outside pool"),
-                              gre_id)
-            except sa_exc.NoResultFound:
-                LOG.warning(_("gre_id %s not found"), gre_id)
+            query = session.query(GreAllocation).filter_by(gre_id=gre_id)
+            if inside:
+                count = query.update({"allocated": False})
+                if count:
+                    LOG.debug("Releasing gre tunnel %s to pool", gre_id)
+            else:
+                count = query.delete()
+                if count:
+                    LOG.debug("Releasing gre tunnel %s outside pool", gre_id)
+
+        if not count:
+            LOG.warning(_("gre_id %s not found"), gre_id)
 
     def _sync_gre_allocations(self):
         """Synchronize gre_allocations table with configured tunnel ranges."""
index 0159d5713bd1ba424275c300e2fc35f63357d93b..c35ba3ce4be59a09f81722a6062bcc774e23990a 100644 (file)
@@ -235,33 +235,31 @@ class VlanTypeDriver(api.TypeDriver):
     def release_segment(self, session, segment):
         physical_network = segment[api.PHYSICAL_NETWORK]
         vlan_id = segment[api.SEGMENTATION_ID]
+
+        ranges = self.network_vlan_ranges.get(physical_network, [])
+        inside = any(lo <= vlan_id <= hi for lo, hi in ranges)
+
         with session.begin(subtransactions=True):
-            try:
-                alloc = (session.query(VlanAllocation).
-                         filter_by(physical_network=physical_network,
-                                   vlan_id=vlan_id).
-                         with_lockmode('update').
-                         one())
-                alloc.allocated = False
-                inside = False
-                for vlan_min, vlan_max in self.network_vlan_ranges.get(
-                    physical_network, []):
-                    if vlan_min <= vlan_id <= vlan_max:
-                        inside = True
-                        break
-                if not inside:
-                    session.delete(alloc)
-                    LOG.debug(_("Releasing vlan %(vlan_id)s on physical "
-                                "network %(physical_network)s outside pool"),
+            query = (session.query(VlanAllocation).
+                     filter_by(physical_network=physical_network,
+                               vlan_id=vlan_id))
+            if inside:
+                count = query.update({"allocated": False})
+                if count:
+                    LOG.debug("Releasing vlan %(vlan_id)s on physical "
+                              "network %(physical_network)s to pool",
                               {'vlan_id': vlan_id,
                                'physical_network': physical_network})
-                else:
-                    LOG.debug(_("Releasing vlan %(vlan_id)s on physical "
-                                "network %(physical_network)s to pool"),
+            else:
+                count = query.delete()
+                if count:
+                    LOG.debug("Releasing vlan %(vlan_id)s on physical "
+                              "network %(physical_network)s outside pool",
                               {'vlan_id': vlan_id,
                                'physical_network': physical_network})
-            except sa.orm.exc.NoResultFound:
-                LOG.warning(_("No vlan_id %(vlan_id)s found on physical "
-                              "network %(physical_network)s"),
-                            {'vlan_id': vlan_id,
-                             'physical_network': physical_network})
+
+        if not count:
+            LOG.warning(_("No vlan_id %(vlan_id)s found on physical "
+                          "network %(physical_network)s"),
+                        {'vlan_id': vlan_id,
+                         'physical_network': physical_network})
index 3e5d4756791f875b8e98588c99d5f5fafa6ea3f1..8a6e29ad47c62e6a4bd4006883c53550699478f9 100644 (file)
@@ -116,24 +116,25 @@ class VxlanTypeDriver(type_tunnel.TunnelTypeDriver):
 
     def release_segment(self, session, segment):
         vxlan_vni = segment[api.SEGMENTATION_ID]
+
+        inside = any(lo <= vxlan_vni <= hi for lo, hi in self.vxlan_vni_ranges)
+
         with session.begin(subtransactions=True):
-            try:
-                alloc = (session.query(VxlanAllocation).
-                         filter_by(vxlan_vni=vxlan_vni).
-                         with_lockmode('update').
-                         one())
-                alloc.allocated = False
-                for low, high in self.vxlan_vni_ranges:
-                    if low <= vxlan_vni <= high:
-                        LOG.debug(_("Releasing vxlan tunnel %s to pool"),
-                                  vxlan_vni)
-                        break
-                else:
-                    session.delete(alloc)
-                    LOG.debug(_("Releasing vxlan tunnel %s outside pool"),
+            query = (session.query(VxlanAllocation).
+                     filter_by(vxlan_vni=vxlan_vni))
+            if inside:
+                count = query.update({"allocated": False})
+                if count:
+                    LOG.debug("Releasing vxlan tunnel %s to pool",
                               vxlan_vni)
-            except sa_exc.NoResultFound:
-                LOG.warning(_("vxlan_vni %s not found"), vxlan_vni)
+            else:
+                count = query.delete()
+                if count:
+                    LOG.debug("Releasing vxlan tunnel %s outside pool",
+                              vxlan_vni)
+
+        if not count:
+            LOG.warning(_("vxlan_vni %s not found"), vxlan_vni)
 
     def _sync_vxlan_allocations(self):
         """