From 5e8ae4711462f0ec344c13bb5f21f42503b7caff Mon Sep 17 00:00:00 2001 From: Robert Li Date: Thu, 11 Sep 2014 13:46:26 -0400 Subject: [PATCH] Mitigating performance impact with getting pci requests from DB This change mitigates the performance impact in host manager that retrieves an instance's PCI requests from DB while consuming it. This is achieved by adding pci_requests into the instance dict before calling consume_from_instance() and deleting it afterwards. The complete solution will be provided when bug 1368260 is fully addressed. The patch also removes the unnecessary context argument. Change-Id: Ib0d12cde1c297a04e5d626b28d0a994c5dd4d965 Partial-Bug: 1368260 --- nova/pci/pci_stats.py | 2 +- nova/scheduler/base_baremetal_host_manager.py | 2 +- nova/scheduler/filter_scheduler.py | 12 +++++++++++- nova/scheduler/host_manager.py | 8 +++----- nova/scheduler/ironic_host_manager.py | 4 ++-- nova/tests/scheduler/test_host_manager.py | 10 +++------- nova/tests/scheduler/test_ironic_host_manager.py | 6 +++--- 7 files changed, 24 insertions(+), 20 deletions(-) diff --git a/nova/pci/pci_stats.py b/nova/pci/pci_stats.py index 19194258af..8d4b7cb3fb 100644 --- a/nova/pci/pci_stats.py +++ b/nova/pci/pci_stats.py @@ -177,7 +177,7 @@ class PciDeviceStats(object): def _apply_request(self, pools, request): count = request.count - matching_pools = self._filter_pools_for_spec(pools, request['spec']) + matching_pools = self._filter_pools_for_spec(pools, request.spec) if sum([pool['count'] for pool in matching_pools]) < count: return False else: diff --git a/nova/scheduler/base_baremetal_host_manager.py b/nova/scheduler/base_baremetal_host_manager.py index 091e7845f3..99baba117b 100644 --- a/nova/scheduler/base_baremetal_host_manager.py +++ b/nova/scheduler/base_baremetal_host_manager.py @@ -40,7 +40,7 @@ class BaseBaremetalNodeState(host_manager.HostState): stats = compute.get('stats', '{}') self.stats = jsonutils.loads(stats) - def consume_from_instance(self, context, instance): + def consume_from_instance(self, instance): """Consume nodes entire resources regardless of instance request.""" self.free_ram_mb = 0 self.free_disk_mb = 0 diff --git a/nova/scheduler/filter_scheduler.py b/nova/scheduler/filter_scheduler.py index 3876c24760..ef39cc9d0a 100644 --- a/nova/scheduler/filter_scheduler.py +++ b/nova/scheduler/filter_scheduler.py @@ -297,7 +297,17 @@ class FilterScheduler(driver.Scheduler): # Now consume the resources so the filter/weights # will change for the next instance. - chosen_host.obj.consume_from_instance(context, instance_properties) + # NOTE (baoli) adding and deleting pci_requests is a temporary + # fix to avoid DB access in consume_from_instance() while getting + # pci_requests. The change can be removed once pci_requests is + # part of the instance object that is passed into the scheduler + # APIs + pci_requests = filter_properties.get('pci_requests') + if pci_requests: + instance_properties['pci_requests'] = pci_requests + chosen_host.obj.consume_from_instance(instance_properties) + if pci_requests: + del instance_properties['pci_requests'] if update_group_hosts is True: filter_properties['group_hosts'].add(chosen_host.obj.host) return selected_hosts diff --git a/nova/scheduler/host_manager.py b/nova/scheduler/host_manager.py index 883c042768..6ec2cd87c9 100644 --- a/nova/scheduler/host_manager.py +++ b/nova/scheduler/host_manager.py @@ -27,7 +27,6 @@ from nova.compute import vm_states from nova import db from nova import exception from nova.i18n import _ -from nova import objects from nova.openstack.common import jsonutils from nova.openstack.common import log as logging from nova.openstack.common import timeutils @@ -229,7 +228,7 @@ class HostState(object): # update metrics self._update_metrics_from_compute_node(compute) - def consume_from_instance(self, context, instance): + def consume_from_instance(self, instance): """Incrementally update host state from an instance.""" disk_mb = (instance['root_gb'] + instance['ephemeral_gb']) * 1024 ram_mb = instance['memory_mb'] @@ -242,9 +241,8 @@ class HostState(object): # Track number of instances on host self.num_instances += 1 - pci_requests = objects.InstancePCIRequests.get_by_instance_uuid( - context, instance['uuid']) - if pci_requests.requests and self.pci_stats: + pci_requests = instance.get('pci_requests') + if pci_requests and pci_requests.requests and self.pci_stats: self.pci_stats.apply_requests(pci_requests.requests) # Calculate the numa usage diff --git a/nova/scheduler/ironic_host_manager.py b/nova/scheduler/ironic_host_manager.py index abcd28b864..409c6dd1cc 100644 --- a/nova/scheduler/ironic_host_manager.py +++ b/nova/scheduler/ironic_host_manager.py @@ -68,9 +68,9 @@ class IronicNodeState(bbhm.BaseBaremetalNodeState): self.total_usable_disk_gb = compute['local_gb'] self.updated = compute['updated_at'] - def consume_from_instance(self, context, instance): + def consume_from_instance(self, instance): """Consume nodes entire resources regardless of instance request.""" - super(IronicNodeState, self).consume_from_instance(context, instance) + super(IronicNodeState, self).consume_from_instance(instance) self.updated = timeutils.utcnow() diff --git a/nova/tests/scheduler/test_host_manager.py b/nova/tests/scheduler/test_host_manager.py index 750e786cab..8436bf5d58 100644 --- a/nova/tests/scheduler/test_host_manager.py +++ b/nova/tests/scheduler/test_host_manager.py @@ -485,12 +485,8 @@ class HostStateTestCase(test.NoDBTestCase): self.assertIsNone(host.pci_stats) self.assertEqual(hyper_ver_int, host.hypervisor_version) - @mock.patch.object(host_manager.objects.InstancePCIRequests, - 'get_by_instance_uuid', - return_value=host_manager.objects.InstancePCIRequests(requests=[])) @mock.patch('nova.virt.hardware.get_host_numa_usage_from_instance') - def test_stat_consumption_from_instance(self, numa_usage_mock, - mock_pci_req): + def test_stat_consumption_from_instance(self, numa_usage_mock): numa_usage_mock.return_value = 'fake-consumed-once' host = host_manager.HostState("fakehost", "fakenode") @@ -498,7 +494,7 @@ class HostStateTestCase(test.NoDBTestCase): project_id='12345', vm_state=vm_states.BUILDING, task_state=task_states.SCHEDULING, os_type='Linux', uuid='fake-uuid') - host.consume_from_instance('fake-context', instance) + host.consume_from_instance(instance) numa_usage_mock.assert_called_once_with(host, instance) self.assertEqual('fake-consumed-once', host.numa_topology) @@ -507,7 +503,7 @@ class HostStateTestCase(test.NoDBTestCase): project_id='12345', vm_state=vm_states.PAUSED, task_state=None, os_type='Linux', uuid='fake-uuid') - host.consume_from_instance('fake-context', instance) + host.consume_from_instance(instance) self.assertEqual(2, host.num_instances) self.assertEqual(1, host.num_io_ops) diff --git a/nova/tests/scheduler/test_ironic_host_manager.py b/nova/tests/scheduler/test_ironic_host_manager.py index 21bd457561..761dbf893a 100644 --- a/nova/tests/scheduler/test_ironic_host_manager.py +++ b/nova/tests/scheduler/test_ironic_host_manager.py @@ -155,7 +155,7 @@ class IronicHostManagerChangedNodesTestCase(test.NoDBTestCase): host.update_from_compute_node(self.compute_node) instance = dict(root_gb=10, ephemeral_gb=0, memory_mb=1024, vcpus=1) - host.consume_from_instance('fake-context', instance) + host.consume_from_instance(instance) self.assertEqual(1, host.vcpus_used) self.assertEqual(0, host.free_ram_mb) @@ -166,7 +166,7 @@ class IronicHostManagerChangedNodesTestCase(test.NoDBTestCase): host.update_from_compute_node(self.compute_node) instance = dict(root_gb=20, ephemeral_gb=0, memory_mb=2048, vcpus=2) - host.consume_from_instance('fake-context', instance) + host.consume_from_instance(instance) self.assertEqual(1, host.vcpus_used) self.assertEqual(0, host.free_ram_mb) @@ -177,7 +177,7 @@ class IronicHostManagerChangedNodesTestCase(test.NoDBTestCase): host.update_from_compute_node(self.compute_node) instance = dict(root_gb=5, ephemeral_gb=0, memory_mb=512, vcpus=1) - host.consume_from_instance('fake-context', instance) + host.consume_from_instance(instance) self.assertEqual(1, host.vcpus_used) self.assertEqual(0, host.free_ram_mb)