From 6865baccd3825bf891a763627693b8b299e524df Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Mon, 1 May 2017 10:32:39 -0400 Subject: [PATCH] Add compute_nodes_uuid_idx unique index Since change Ie12acb76ec5affba536c3c45fbb6de35d64aea1b in Ocata the FilterScheduler is calling the placement service to get a list of resource providers that can service a request spec, and then taking that list and querying the compute nodes table with a list of uuids. We should have a unique index on the uuid column if we're doing queries like this, especially somewhere where performance matters like the scheduler. As a result of this change, several unit tests that were blindly creating duplicate compute nodes with the same uuid have to be fixed to use unique uuids. Change-Id: I940979d85fc105c9733ae19c406e85debe8fdffb --- .../361_add_compute_nodes_uuid_index.py | 41 +++++++++++++++++++ nova/db/sqlalchemy/models.py | 1 + nova/tests/unit/db/test_db_api.py | 19 +++++++-- nova/tests/unit/db/test_migrations.py | 4 ++ 4 files changed, 61 insertions(+), 4 deletions(-) create mode 100644 nova/db/sqlalchemy/migrate_repo/versions/361_add_compute_nodes_uuid_index.py diff --git a/nova/db/sqlalchemy/migrate_repo/versions/361_add_compute_nodes_uuid_index.py b/nova/db/sqlalchemy/migrate_repo/versions/361_add_compute_nodes_uuid_index.py new file mode 100644 index 000000000000..be38d9f9a69b --- /dev/null +++ b/nova/db/sqlalchemy/migrate_repo/versions/361_add_compute_nodes_uuid_index.py @@ -0,0 +1,41 @@ +# Copyright 2017 Huawei Technologies Co.,LTD. +# +# 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. + + +from oslo_log import log as logging +from sqlalchemy import MetaData, Table, Index + +LOG = logging.getLogger(__name__) + + +def _get_table_index(migrate_engine): + meta = MetaData() + meta.bind = migrate_engine + table = Table('compute_nodes', meta, autoload=True) + for idx in table.indexes: + if idx.columns.keys() == ['uuid']: + break + else: + idx = None + return table, idx + + +def upgrade(migrate_engine): + table, index = _get_table_index(migrate_engine) + if index: + LOG.info('Skipped adding compute_nodes_uuid_idx because an ' + 'equivalent index already exists.') + return + index = Index('compute_nodes_uuid_idx', table.c.uuid, unique=True) + index.create(migrate_engine) diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py index 766aa0bad5cc..9c13618d5304 100644 --- a/nova/db/sqlalchemy/models.py +++ b/nova/db/sqlalchemy/models.py @@ -112,6 +112,7 @@ class ComputeNode(BASE, NovaBase, models.SoftDeleteMixin): __tablename__ = 'compute_nodes' __table_args__ = ( + Index('compute_nodes_uuid_idx', 'uuid', unique=True), schema.UniqueConstraint( 'host', 'hypervisor_hostname', 'deleted', name="uniq_compute_nodes0host0hypervisor_hostname0deleted"), diff --git a/nova/tests/unit/db/test_db_api.py b/nova/tests/unit/db/test_db_api.py index 251a6def79db..55e12ecd6f69 100644 --- a/nova/tests/unit/db/test_db_api.py +++ b/nova/tests/unit/db/test_db_api.py @@ -90,7 +90,7 @@ def _reservation_get(context, uuid): def _make_compute_node(host, node, hv_type, service_id): compute_node_dict = dict(vcpus=2, memory_mb=1024, local_gb=2048, - uuid=uuidsentinel.fake_compute_node, + uuid=uuidutils.generate_uuid(), vcpus_used=0, memory_mb_used=0, local_gb_used=0, free_ram_mb=1024, free_disk_gb=2048, hypervisor_type=hv_type, @@ -7787,7 +7787,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): disabled=False) self.service = db.service_create(self.ctxt, self.service_dict) self.compute_node_dict = dict(vcpus=2, memory_mb=1024, local_gb=2048, - uuid=uuidsentinel.fake_compute_node, + uuid=uuidutils.generate_uuid(), vcpus_used=0, memory_mb_used=0, local_gb_used=0, free_ram_mb=1024, free_disk_gb=2048, hypervisor_type="xen", @@ -7835,12 +7835,14 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): cn = dict(self.compute_node_dict, hostname='foo', hypervisor_hostname='foo', - mapped=None) + mapped=None, + uuid=uuidutils.generate_uuid()) db.compute_node_create(self.ctxt, cn) cn = dict(self.compute_node_dict, hostname='bar', hypervisor_hostname='nar', - mapped=3) + mapped=3, + uuid=uuidutils.generate_uuid()) db.compute_node_create(self.ctxt, cn) cns = db.compute_node_get_all_mapped_less_than(self.ctxt, 1) self.assertEqual(2, len(cns)) @@ -7909,6 +7911,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): # Create a compute node compute_node_data = self.compute_node_dict.copy() + compute_node_data['uuid'] = uuidutils.generate_uuid() compute_node_data['service_id'] = service['id'] compute_node_data['stats'] = jsonutils.dumps(self.stats.copy()) compute_node_data['hypervisor_hostname'] = 'hypervisor-%s' % x @@ -7943,6 +7946,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): for name in ['bm_node1', 'bm_node2']: compute_node_data = self.compute_node_dict.copy() + compute_node_data['uuid'] = uuidutils.generate_uuid() compute_node_data['service_id'] = service['id'] compute_node_data['stats'] = jsonutils.dumps(self.stats) compute_node_data['hypervisor_hostname'] = name @@ -7964,6 +7968,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): service2['host'] = 'host2' db.service_create(self.ctxt, service2) compute_node_another_host = self.compute_node_dict.copy() + compute_node_another_host['uuid'] = uuidutils.generate_uuid() compute_node_another_host['stats'] = jsonutils.dumps(self.stats) compute_node_another_host['hypervisor_hostname'] = 'node_2' compute_node_another_host['host'] = 'host2' @@ -7978,6 +7983,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): def test_compute_node_get_all_by_host_with_same_host(self): # Create another node on top of the same service compute_node_same_host = self.compute_node_dict.copy() + compute_node_same_host['uuid'] = uuidutils.generate_uuid() compute_node_same_host['stats'] = jsonutils.dumps(self.stats) compute_node_same_host['hypervisor_hostname'] = 'node_3' @@ -8008,6 +8014,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): def test_compute_nodes_get_by_service_id_multiple_results(self): # Create another node on top of the same service compute_node_same_host = self.compute_node_dict.copy() + compute_node_same_host['uuid'] = uuidutils.generate_uuid() compute_node_same_host['stats'] = jsonutils.dumps(self.stats) compute_node_same_host['hypervisor_hostname'] = 'node_2' @@ -8030,6 +8037,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): def test_compute_node_get_by_host_and_nodename(self): # Create another node on top of the same service compute_node_same_host = self.compute_node_dict.copy() + compute_node_same_host['uuid'] = uuidutils.generate_uuid() compute_node_same_host['stats'] = jsonutils.dumps(self.stats) compute_node_same_host['hypervisor_hostname'] = 'node_2' @@ -8089,6 +8097,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): self.compute_node_dict['service_id'] = service['id'] self.compute_node_dict['hypervisor_hostname'] = 'testhost' + str(i) self.compute_node_dict['stats'] = jsonutils.dumps(self.stats) + self.compute_node_dict['uuid'] = uuidutils.generate_uuid() node = db.compute_node_create(self.ctxt, self.compute_node_dict) nodes_created.append(node) nodes = db.compute_node_search_by_hypervisor(self.ctxt, 'host') @@ -8191,6 +8200,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): service2['host'] = 'host2' db_service2 = db.service_create(self.ctxt, service2) compute_node_old_host = self.compute_node_dict.copy() + compute_node_old_host['uuid'] = uuidutils.generate_uuid() compute_node_old_host['stats'] = jsonutils.dumps(self.stats) compute_node_old_host['hypervisor_hostname'] = 'node_2' compute_node_old_host['service_id'] = db_service2['id'] @@ -8257,6 +8267,7 @@ class ComputeNodeTestCase(test.TestCase, ModelsObjectComparatorMixin): # This test could be removed once we are sure that all compute nodes # are populating the host field thanks to the ResourceTracker compute_node_old_host_dict = self.compute_node_dict.copy() + compute_node_old_host_dict['uuid'] = uuidutils.generate_uuid() compute_node_old_host_dict.pop('host') item_old = db.compute_node_create(self.ctxt, compute_node_old_host_dict) diff --git a/nova/tests/unit/db/test_migrations.py b/nova/tests/unit/db/test_migrations.py index 3887a8d286e5..885eca13f1ba 100644 --- a/nova/tests/unit/db/test_migrations.py +++ b/nova/tests/unit/db/test_migrations.py @@ -956,6 +956,10 @@ class NovaMigrationsCheckers(test_migrations.ModelsMigrationsSync, self.assertColumnExists(engine, 'compute_nodes', 'mapped') self.assertColumnExists(engine, 'shadow_compute_nodes', 'mapped') + def _check_361(self, engine, data): + self.assertIndexMembers(engine, 'compute_nodes', + 'compute_nodes_uuid_idx', ['uuid']) + class TestNovaMigrationsSQLite(NovaMigrationsCheckers, test_base.DbTestCase,