Remove SELECT FOR UPDATE use in ML2 tunnel driver add_endpoint

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 add_endpoint methods
with the use of single-shot SELECT and INSERT expressions and the
correction of VxlanEndpoint primary key: indeed previously it was not
possible to create multiple endpoints with the same ip, now the model
primary key constraint ensures it.

Change-Id: Id69fbc15c8f51b4b275cd742312e6ff6802d8c0f
Partial-Bug: #1330562

neutron/db/migration/alembic_migrations/versions/4eba2f05c2f4_correct_vxlan_endpoint_primary_key.py

@@ -0,0 +1,43 @@
+# Copyright (c) 2014 Thales Services SAS
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+#
+
+"""correct Vxlan Endpoint primary key
+
+Revision ID: 4eba2f05c2f4
+Revises: 884573acbf1c
+Create Date: 2014-07-07 22:48:38.544323
+
+"""
+
+# revision identifiers, used by Alembic.
+revision = '4eba2f05c2f4'
+down_revision = '884573acbf1c'
+
+
+from alembic import op
+
+
+TABLE_NAME = 'ml2_vxlan_endpoints'
+PK_NAME = 'ml2_vxlan_endpoints_pkey'
+
+
+def upgrade(active_plugins=None, options=None):
+    op.drop_constraint(PK_NAME, TABLE_NAME, type_='primary')
+    op.create_primary_key(PK_NAME, TABLE_NAME, cols=['ip_address'])
+
+
+def downgrade(active_plugins=None, options=None):
+    op.drop_constraint(PK_NAME, TABLE_NAME, type_='primary')
+    op.create_primary_key(PK_NAME, TABLE_NAME, cols=['ip_address', 'udp_port'])

neutron/db/migration/alembic_migrations/versions/HEAD

@@ -1 +1 @@
-884573acbf1c
+4eba2f05c2f4

neutron/plugins/ml2/drivers/type_gre.py

@@ -14,9 +14,9 @@
 #    under the License.
 
 from oslo.config import cfg
+from oslo.db import exception as db_exc
 from six import moves
 import sqlalchemy as sa
-from sqlalchemy.orm import exc as sa_exc
 from sqlalchemy import sql
 
 from neutron.common import exceptions as exc
@@ -172,12 +172,11 @@ class GreTypeDriver(helpers.TypeDriverHelper, type_tunnel.TunnelTypeDriver):
     def add_endpoint(self, ip):
         LOG.debug(_("add_gre_endpoint() called for ip %s"), ip)
         session = db_api.get_session()
-        with session.begin(subtransactions=True):
-            try:
-                gre_endpoint = (session.query(GreEndpoints).
-                                filter_by(ip_address=ip).one())
-                LOG.warning(_("Gre endpoint with ip %s already exists"), ip)
-            except sa_exc.NoResultFound:
-                gre_endpoint = GreEndpoints(ip_address=ip)
-                session.add(gre_endpoint)
-            return gre_endpoint
+        try:
+            gre_endpoint = GreEndpoints(ip_address=ip)
+            gre_endpoint.save(session)
+        except db_exc.DBDuplicateEntry:
+            gre_endpoint = (session.query(GreEndpoints).
+                            filter_by(ip_address=ip).one())
+            LOG.warning(_("Gre endpoint with ip %s already exists"), ip)
+        return gre_endpoint

neutron/plugins/ml2/drivers/type_vxlan.py

@@ -15,8 +15,8 @@
 # @author: Kyle Mestery, Cisco Systems, Inc.
 
 from oslo.config import cfg
+from oslo.db import exception as db_exc
 import sqlalchemy as sa
-from sqlalchemy.orm import exc as sa_exc
 from sqlalchemy import sql
 
 from neutron.common import exceptions as exc
@@ -62,8 +62,7 @@ class VxlanEndpoints(model_base.BASEV2):
     __tablename__ = 'ml2_vxlan_endpoints'
 
     ip_address = sa.Column(sa.String(64), primary_key=True)
-    udp_port = sa.Column(sa.Integer, primary_key=True, nullable=False,
-                         autoincrement=False)
+    udp_port = sa.Column(sa.Integer, nullable=False)
 
     def __repr__(self):
         return "<VxlanTunnelEndpoint(%s)>" % self.ip_address
@@ -197,13 +196,12 @@ class VxlanTypeDriver(helpers.TypeDriverHelper, type_tunnel.TunnelTypeDriver):
     def add_endpoint(self, ip, udp_port=VXLAN_UDP_PORT):
         LOG.debug(_("add_vxlan_endpoint() called for ip %s"), ip)
         session = db_api.get_session()
-        with session.begin(subtransactions=True):
-            try:
-                vxlan_endpoint = (session.query(VxlanEndpoints).
-                                  filter_by(ip_address=ip).
-                                  with_lockmode('update').one())
-            except sa_exc.NoResultFound:
-                vxlan_endpoint = VxlanEndpoints(ip_address=ip,
-                                                udp_port=udp_port)
-                session.add(vxlan_endpoint)
-            return vxlan_endpoint
+        try:
+            vxlan_endpoint = VxlanEndpoints(ip_address=ip,
+                                            udp_port=udp_port)
+            vxlan_endpoint.save(session)
+        except db_exc.DBDuplicateEntry:
+            vxlan_endpoint = (session.query(VxlanEndpoints).
+                              filter_by(ip_address=ip).one())
+            LOG.warning(_("Vxlan endpoint with ip %s already exists"), ip)
+        return vxlan_endpoint

neutron/tests/unit/ml2/test_type_gre.py

@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import mock
 from six import moves
 import testtools
 from testtools import matchers
@@ -225,6 +226,12 @@ class GreTypeTest(base.BaseTestCase):
             self.assertIn(endpoint['ip_address'],
                           [TUNNEL_IP_ONE, TUNNEL_IP_TWO])
 
+    def test_add_same_endpoints(self):
+        self.driver.add_endpoint(TUNNEL_IP_ONE)
+        with mock.patch.object(type_gre.LOG, 'warning') as log_warn:
+            self.driver.add_endpoint(TUNNEL_IP_ONE)
+        log_warn.assert_called_once_with(mock.ANY, TUNNEL_IP_ONE)
+
 
 class GreTypeMultiRangeTest(base.BaseTestCase):
 

neutron/tests/unit/ml2/test_type_vxlan.py

@@ -14,6 +14,7 @@
 #    under the License.
 # @author: Kyle Mestery, Cisco Systems, Inc.
 
+import mock
 from oslo.config import cfg
 from six import moves
 import testtools
@@ -244,6 +245,14 @@ class VxlanTypeTest(base.BaseTestCase):
             elif endpoint['ip_address'] == TUNNEL_IP_TWO:
                 self.assertEqual(VXLAN_UDP_PORT_TWO, endpoint['udp_port'])
 
+    def test_add_same_ip_endpoints(self):
+        self.driver.add_endpoint(TUNNEL_IP_ONE, VXLAN_UDP_PORT_ONE)
+        with mock.patch.object(type_vxlan.LOG, 'warning') as log_warn:
+            observed = self.driver.add_endpoint(TUNNEL_IP_ONE,
+                                                VXLAN_UDP_PORT_TWO)
+            self.assertEqual(VXLAN_UDP_PORT_ONE, observed['udp_port'])
+            log_warn.assert_called_once_with(mock.ANY, TUNNEL_IP_ONE)
+
 
 class VxlanTypeMultiRangeTest(base.BaseTestCase):