From bc4965080ce83db50b71255d6a0f972b9be67b1b Mon Sep 17 00:00:00 2001 From: Cedric Brandily Date: Tue, 8 Jul 2014 00:05:21 +0200 Subject: [PATCH] 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 --- ...c2f4_correct_vxlan_endpoint_primary_key.py | 43 +++++++++++++++++++ .../alembic_migrations/versions/HEAD | 2 +- neutron/plugins/ml2/drivers/type_gre.py | 19 ++++---- neutron/plugins/ml2/drivers/type_vxlan.py | 24 +++++------ neutron/tests/unit/ml2/test_type_gre.py | 7 +++ neutron/tests/unit/ml2/test_type_vxlan.py | 9 ++++ 6 files changed, 80 insertions(+), 24 deletions(-) create mode 100644 neutron/db/migration/alembic_migrations/versions/4eba2f05c2f4_correct_vxlan_endpoint_primary_key.py diff --git a/neutron/db/migration/alembic_migrations/versions/4eba2f05c2f4_correct_vxlan_endpoint_primary_key.py b/neutron/db/migration/alembic_migrations/versions/4eba2f05c2f4_correct_vxlan_endpoint_primary_key.py new file mode 100644 index 000000000..c6fc2832c --- /dev/null +++ b/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']) diff --git a/neutron/db/migration/alembic_migrations/versions/HEAD b/neutron/db/migration/alembic_migrations/versions/HEAD index f39590d47..6ea58e91e 100644 --- a/neutron/db/migration/alembic_migrations/versions/HEAD +++ b/neutron/db/migration/alembic_migrations/versions/HEAD @@ -1 +1 @@ -884573acbf1c +4eba2f05c2f4 diff --git a/neutron/plugins/ml2/drivers/type_gre.py b/neutron/plugins/ml2/drivers/type_gre.py index 9ade7db7b..1e2eb12ec 100644 --- a/neutron/plugins/ml2/drivers/type_gre.py +++ b/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 diff --git a/neutron/plugins/ml2/drivers/type_vxlan.py b/neutron/plugins/ml2/drivers/type_vxlan.py index 9ddcf735d..9652d6067 100644 --- a/neutron/plugins/ml2/drivers/type_vxlan.py +++ b/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 "" % 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 diff --git a/neutron/tests/unit/ml2/test_type_gre.py b/neutron/tests/unit/ml2/test_type_gre.py index 110786f11..d695c4f00 100644 --- a/neutron/tests/unit/ml2/test_type_gre.py +++ b/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): diff --git a/neutron/tests/unit/ml2/test_type_vxlan.py b/neutron/tests/unit/ml2/test_type_vxlan.py index 4512832de..1f7fed2b4 100644 --- a/neutron/tests/unit/ml2/test_type_vxlan.py +++ b/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): -- 2.45.2