Trim the fat on HostState.instances
The only in-tree filters that rely on HostState.instances are the affinity filters (and one of the weighers). And they don't even look at the values in the HostState.instances dict, just the keys (which are the instance uuids for the instances on the host). So rather than pull the full instance objects, we can just get the list of instance uuids off the host and fake out the object. Custom filters/weighers will still be able to lazy-load fields on the Instance objects in HostState.instances if needed, but it will mean a performance penalty due to the round trip to the database per instance, per host. Out of tree filters/weighers are encouraged to be contributed upstream. Related to blueprint put-host-manager-instance-info-on-a-diet Related-Bug: #1737465 Change-Id: I766bb5645e3b598468d092fb9e4f18e720617c52
This commit is contained in:
Matt Riedemann
parent
84701aca3b
commit
91f5af7ee7
|
|
@ -750,14 +750,10 @@ class HostManager(object):
|
|||
'instance info for this host.', host_name)
|
||||
return {}
|
||||
with context_module.target_cell(context, hm.cell_mapping) as cctxt:
|
||||
# NOTE(mriedem): We pass expected_attrs=[] to avoid a default
|
||||
# join on info_cache and security_groups, which at present none
|
||||
# of the in-tree filters/weighers rely on that information. Any
|
||||
# out of tree filters which rely on it will end up lazy-loading
|
||||
# the field but we don't have a contract on out of tree filters.
|
||||
inst_list = objects.InstanceList.get_by_host(
|
||||
cctxt, host_name, expected_attrs=[])
|
||||
return {inst.uuid: inst for inst in inst_list}
|
||||
uuids = objects.InstanceList.get_uuids_by_host(cctxt, host_name)
|
||||
# Putting the context in the otherwise fake Instance object at
|
||||
# least allows out of tree filters to lazy-load fields.
|
||||
return {uuid: objects.Instance(cctxt, uuid=uuid) for uuid in uuids}
|
||||
|
||||
def _get_instance_info(self, context, compute):
|
||||
"""Gets the host instance info from the compute host.
|
||||
|
|
|
|||
|
|
@ -539,10 +539,10 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
@mock.patch('nova.scheduler.host_manager.LOG')
|
||||
@mock.patch('nova.objects.ServiceList.get_by_binary')
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_all')
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host')
|
||||
@mock.patch('nova.objects.InstanceList.get_uuids_by_host')
|
||||
def test_get_all_host_states(self, mock_get_by_host, mock_get_all,
|
||||
mock_get_by_binary, mock_log):
|
||||
mock_get_by_host.return_value = objects.InstanceList()
|
||||
mock_get_by_host.return_value = []
|
||||
mock_get_all.return_value = fakes.COMPUTE_NODES
|
||||
mock_get_by_binary.return_value = fakes.SERVICES
|
||||
context = 'fake_context'
|
||||
|
|
@ -600,7 +600,7 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
self.assertEqual(host_states_map[('host4', 'node4')].free_disk_mb,
|
||||
8388608)
|
||||
|
||||
@mock.patch.object(nova.objects.InstanceList, 'get_by_host')
|
||||
@mock.patch.object(nova.objects.InstanceList, 'get_uuids_by_host')
|
||||
@mock.patch.object(host_manager.HostState, '_update_from_compute_node')
|
||||
@mock.patch.object(objects.ComputeNodeList, 'get_all')
|
||||
@mock.patch.object(objects.ServiceList, 'get_by_binary')
|
||||
|
|
@ -610,7 +610,7 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
svc_get_by_binary.return_value = [objects.Service(host='fake')]
|
||||
cn_get_all.return_value = [
|
||||
objects.ComputeNode(host='fake', hypervisor_hostname='fake')]
|
||||
mock_get_by_host.return_value = objects.InstanceList()
|
||||
mock_get_by_host.return_value = []
|
||||
self.host_manager.host_aggregates_map = collections.defaultdict(set)
|
||||
|
||||
hosts = self.host_manager.get_all_host_states('fake-context')
|
||||
|
|
@ -620,7 +620,7 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
host_state = host_states_map[('fake', 'fake')]
|
||||
self.assertEqual([], host_state.aggregates)
|
||||
|
||||
@mock.patch.object(nova.objects.InstanceList, 'get_by_host')
|
||||
@mock.patch.object(nova.objects.InstanceList, 'get_uuids_by_host')
|
||||
@mock.patch.object(host_manager.HostState, '_update_from_compute_node')
|
||||
@mock.patch.object(objects.ComputeNodeList, 'get_all')
|
||||
@mock.patch.object(objects.ServiceList, 'get_by_binary')
|
||||
|
|
@ -631,7 +631,7 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
svc_get_by_binary.return_value = [objects.Service(host='fake')]
|
||||
cn_get_all.return_value = [
|
||||
objects.ComputeNode(host='fake', hypervisor_hostname='fake')]
|
||||
mock_get_by_host.return_value = objects.InstanceList()
|
||||
mock_get_by_host.return_value = []
|
||||
fake_agg = objects.Aggregate(id=1)
|
||||
self.host_manager.host_aggregates_map = collections.defaultdict(
|
||||
set, {'fake': set([1])})
|
||||
|
|
@ -644,7 +644,7 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
host_state = host_states_map[('fake', 'fake')]
|
||||
self.assertEqual([fake_agg], host_state.aggregates)
|
||||
|
||||
@mock.patch.object(nova.objects.InstanceList, 'get_by_host')
|
||||
@mock.patch.object(nova.objects.InstanceList, 'get_uuids_by_host')
|
||||
@mock.patch.object(host_manager.HostState, '_update_from_compute_node')
|
||||
@mock.patch.object(objects.ComputeNodeList, 'get_all')
|
||||
@mock.patch.object(objects.ServiceList, 'get_by_binary')
|
||||
|
|
@ -658,7 +658,7 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
cn_get_all.return_value = [
|
||||
objects.ComputeNode(host='fake', hypervisor_hostname='fake'),
|
||||
objects.ComputeNode(host='other', hypervisor_hostname='other')]
|
||||
mock_get_by_host.return_value = objects.InstanceList()
|
||||
mock_get_by_host.return_value = []
|
||||
fake_agg = objects.Aggregate(id=1)
|
||||
self.host_manager.host_aggregates_map = collections.defaultdict(
|
||||
set, {'other': set([1])})
|
||||
|
|
@ -671,8 +671,8 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
host_state = host_states_map[('fake', 'fake')]
|
||||
self.assertEqual([], host_state.aggregates)
|
||||
|
||||
@mock.patch.object(nova.objects.InstanceList, 'get_by_host',
|
||||
return_value=objects.InstanceList())
|
||||
@mock.patch.object(nova.objects.InstanceList, 'get_uuids_by_host',
|
||||
return_value=[])
|
||||
@mock.patch.object(host_manager.HostState, '_update_from_compute_node')
|
||||
@mock.patch.object(objects.ComputeNodeList, 'get_all')
|
||||
@mock.patch.object(objects.ServiceList, 'get_by_binary')
|
||||
|
|
@ -733,7 +733,7 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
|
||||
@mock.patch('nova.objects.ServiceList.get_by_binary')
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_all')
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host')
|
||||
@mock.patch('nova.objects.InstanceList.get_uuids_by_host')
|
||||
def test_get_all_host_states_not_updated(self, mock_get_by_host,
|
||||
mock_get_all_comp,
|
||||
mock_get_svc_by_binary):
|
||||
|
|
@ -747,26 +747,25 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
'updated': False}}
|
||||
host_state = host_manager.HostState('host1', cn1, uuids.cell)
|
||||
self.assertFalse(host_state.instances)
|
||||
mock_get_by_host.return_value = objects.InstanceList(objects=[inst1])
|
||||
mock_get_by_host.return_value = [uuids.instance]
|
||||
host_state.update(
|
||||
inst_dict=hm._get_instance_info(context, cn1))
|
||||
mock_get_by_host.assert_called_once_with(
|
||||
context, cn1.host, expected_attrs=[])
|
||||
mock_get_by_host.assert_called_once_with(context, cn1.host)
|
||||
self.assertTrue(host_state.instances)
|
||||
self.assertEqual(host_state.instances[uuids.instance], inst1)
|
||||
self.assertIn(uuids.instance, host_state.instances)
|
||||
inst = host_state.instances[uuids.instance]
|
||||
self.assertEqual(uuids.instance, inst.uuid)
|
||||
self.assertIsNotNone(inst._context, 'Instance is orphaned.')
|
||||
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host')
|
||||
@mock.patch('nova.objects.InstanceList.get_uuids_by_host')
|
||||
def test_recreate_instance_info(self, mock_get_by_host):
|
||||
host_name = 'fake_host'
|
||||
inst1 = fake_instance.fake_instance_obj('fake_context',
|
||||
uuid=uuids.instance_1,
|
||||
host=host_name)
|
||||
uuid=uuids.instance_1)
|
||||
inst2 = fake_instance.fake_instance_obj('fake_context',
|
||||
uuid=uuids.instance_2,
|
||||
host=host_name)
|
||||
uuid=uuids.instance_2)
|
||||
orig_inst_dict = {inst1.uuid: inst1, inst2.uuid: inst2}
|
||||
new_inst_list = objects.InstanceList(objects=[inst1, inst2])
|
||||
mock_get_by_host.return_value = new_inst_list
|
||||
mock_get_by_host.return_value = [uuids.instance_1, uuids.instance_2]
|
||||
self.host_manager._instance_info = {
|
||||
host_name: {
|
||||
'instances': orig_inst_dict,
|
||||
|
|
@ -774,7 +773,8 @@ class HostManagerTestCase(test.NoDBTestCase):
|
|||
}}
|
||||
self.host_manager._recreate_instance_info('fake_context', host_name)
|
||||
new_info = self.host_manager._instance_info[host_name]
|
||||
self.assertEqual(len(new_info['instances']), len(new_inst_list))
|
||||
self.assertEqual(len(new_info['instances']),
|
||||
len(mock_get_by_host.return_value))
|
||||
self.assertFalse(new_info['updated'])
|
||||
|
||||
def test_update_instance_info(self):
|
||||
|
|
@ -1072,10 +1072,10 @@ class HostManagerChangedNodesTestCase(test.NoDBTestCase):
|
|||
|
||||
@mock.patch('nova.objects.ServiceList.get_by_binary')
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_all')
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host')
|
||||
@mock.patch('nova.objects.InstanceList.get_uuids_by_host')
|
||||
def test_get_all_host_states(self, mock_get_by_host, mock_get_all,
|
||||
mock_get_by_binary):
|
||||
mock_get_by_host.return_value = objects.InstanceList()
|
||||
mock_get_by_host.return_value = []
|
||||
mock_get_all.return_value = fakes.COMPUTE_NODES
|
||||
mock_get_by_binary.return_value = fakes.SERVICES
|
||||
context = 'fake_context'
|
||||
|
|
@ -1087,7 +1087,7 @@ class HostManagerChangedNodesTestCase(test.NoDBTestCase):
|
|||
|
||||
@mock.patch('nova.objects.ServiceList.get_by_binary')
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_all')
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host')
|
||||
@mock.patch('nova.objects.InstanceList.get_uuids_by_host')
|
||||
def test_get_all_host_states_after_delete_one(self, mock_get_by_host,
|
||||
mock_get_all,
|
||||
mock_get_by_binary):
|
||||
|
|
@ -1096,7 +1096,7 @@ class HostManagerChangedNodesTestCase(test.NoDBTestCase):
|
|||
running_nodes = [n for n in fakes.COMPUTE_NODES
|
||||
if getter(n) != 'node4']
|
||||
|
||||
mock_get_by_host.return_value = objects.InstanceList()
|
||||
mock_get_by_host.return_value = []
|
||||
mock_get_all.side_effect = [fakes.COMPUTE_NODES, running_nodes]
|
||||
mock_get_by_binary.side_effect = [fakes.SERVICES, fakes.SERVICES]
|
||||
context = 'fake_context'
|
||||
|
|
@ -1116,11 +1116,11 @@ class HostManagerChangedNodesTestCase(test.NoDBTestCase):
|
|||
|
||||
@mock.patch('nova.objects.ServiceList.get_by_binary')
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_all')
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host')
|
||||
@mock.patch('nova.objects.InstanceList.get_uuids_by_host')
|
||||
def test_get_all_host_states_after_delete_all(self, mock_get_by_host,
|
||||
mock_get_all,
|
||||
mock_get_by_binary):
|
||||
mock_get_by_host.return_value = objects.InstanceList()
|
||||
mock_get_by_host.return_value = []
|
||||
mock_get_all.side_effect = [fakes.COMPUTE_NODES, []]
|
||||
mock_get_by_binary.side_effect = [fakes.SERVICES, fakes.SERVICES]
|
||||
context = 'fake_context'
|
||||
|
|
@ -1140,10 +1140,10 @@ class HostManagerChangedNodesTestCase(test.NoDBTestCase):
|
|||
|
||||
@mock.patch('nova.objects.ServiceList.get_by_binary')
|
||||
@mock.patch('nova.objects.ComputeNodeList.get_all_by_uuids')
|
||||
@mock.patch('nova.objects.InstanceList.get_by_host')
|
||||
@mock.patch('nova.objects.InstanceList.get_uuids_by_host')
|
||||
def test_get_host_states_by_uuids(self, mock_get_by_host, mock_get_all,
|
||||
mock_get_by_binary):
|
||||
mock_get_by_host.return_value = objects.InstanceList()
|
||||
mock_get_by_host.return_value = []
|
||||
mock_get_all.side_effect = [fakes.COMPUTE_NODES, []]
|
||||
mock_get_by_binary.side_effect = [fakes.SERVICES, fakes.SERVICES]
|
||||
|
||||
|
|
|
|||
|
|
@ -0,0 +1,21 @@
|
|||
---
|
||||
upgrade:
|
||||
- |
|
||||
Deployments with custom scheduler filters (or weighers) that rely on
|
||||
the ``HostState.instances`` dict to contain full Instance objects will
|
||||
now hit a performance penalty because the Instance values in that dict
|
||||
are no longer fully populated objects. The in-tree filters that do rely
|
||||
on ``HostState.instances`` only care about the (1) uuids of the instances
|
||||
per host, which is the keys in the dict and (2) the number of instances
|
||||
per host, which can be determined via ``len(host_state.instances)``.
|
||||
|
||||
Custom scheduler filters and weighers should continue to function since
|
||||
the Instance objects will lazy-load any accessed fields, but this means
|
||||
a round-trip to the database to re-load the object per instance, per host.
|
||||
|
||||
If this is an issue for you, you have three options:
|
||||
|
||||
* Accept this change along with the performance penalty
|
||||
* Revert change I766bb5645e3b598468d092fb9e4f18e720617c52 and carry
|
||||
the fork in the scheduler code
|
||||
* Contribute your custom filter/weigher upstream (this is the best option)
|
||||
Loading…
Reference in New Issue