From 27857c337378472205c37db6ab79fe8404406129 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Fri, 3 Aug 2018 17:26:00 -0400 Subject: [PATCH] Optimize AZ lookup during schedule_and_build_instances If we're creating multiple servers, there is a chance the scheduler returned the same host for more than one of them, which means we could be redundantly looking up the AZ for the same host multiple times. This could happen when creating multiple servers in the same strict affinity group, or simply if the scheduler is configured with a pack-first strategy for filling up hosts. The get_host_availability_zone() method does not use its own internal cache, so this change adds a simple cache to the schedule_and_build_instances method itself so that we only lookup the AZ per unique host once. Note that build_instances suffers from the same issue but that is only called for scheduling with cells v1 which is deprecated so we shouldn't need to care about optimizing that method. Change-Id: I2ae5ae7240e5183acca7492ddad017c0c878835b Closes-Bug: #1785327 --- nova/conductor/manager.py | 9 ++++++--- nova/tests/unit/conductor/test_conductor.py | 9 ++++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py index 1409720c914c..0240e62fafb9 100644 --- a/nova/conductor/manager.py +++ b/nova/conductor/manager.py @@ -1214,6 +1214,7 @@ class ComputeTaskManager(base.Base): host_mapping_cache = {} cell_mapping_cache = {} instances = [] + host_az = {} # host=az cache to optimize multi-create for (build_request, request_spec, host_list) in six.moves.zip( build_requests, request_specs, host_lists): @@ -1259,9 +1260,11 @@ class ComputeTaskManager(base.Base): rc.delete_allocation_for_instance(context, instance.uuid) continue else: - instance.availability_zone = ( - availability_zones.get_host_availability_zone( - context, host.service_host)) + if host.service_host not in host_az: + host_az[host.service_host] = ( + availability_zones.get_host_availability_zone( + context, host.service_host)) + instance.availability_zone = host_az[host.service_host] with obj_target_cell(instance, cell): instance.create() instances.append(instance) diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py index 47f8b3a3d108..b26a2e4636e5 100644 --- a/nova/tests/unit/conductor/test_conductor.py +++ b/nova/tests/unit/conductor/test_conductor.py @@ -1625,7 +1625,9 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations') @mock.patch('nova.objects.HostMapping.get_by_host') - def test_schedule_and_build_multiple_instances(self, + @mock.patch('nova.availability_zones.get_host_availability_zone', + return_value='nova') + def test_schedule_and_build_multiple_instances(self, mock_get_az, get_hostmapping, select_destinations, build_and_run_instance): @@ -1678,6 +1680,11 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase): build_and_run_instance.side_effect = _build_and_run_instance self.conductor.schedule_and_build_instances(**params) self.assertEqual(3, build_and_run_instance.call_count) + # We're processing 4 instances over 2 hosts, so we should only lookup + # the AZ per host once. + mock_get_az.assert_has_calls([ + mock.call(self.ctxt, 'host1'), mock.call(self.ctxt, 'host2')], + any_order=True) @mock.patch('nova.compute.rpcapi.ComputeAPI.build_and_run_instance') @mock.patch('nova.scheduler.rpcapi.SchedulerAPI.select_destinations')