Merge "Trim the fat on HostState.instances"
This commit is contained in:
commit
c2ef1c0863
|
@ -753,14 +753,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.
|
||||
|
|
|
@ -549,10 +549,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'
|
||||
|
@ -605,7 +605,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')
|
||||
|
@ -615,7 +615,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')
|
||||
|
@ -625,7 +625,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')
|
||||
|
@ -636,7 +636,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])})
|
||||
|
@ -649,7 +649,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')
|
||||
|
@ -663,7 +663,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])})
|
||||
|
@ -676,8 +676,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')
|
||||
|
@ -738,7 +738,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):
|
||||
|
@ -752,26 +752,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,
|
||||
|
@ -779,7 +778,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):
|
||||
|
@ -1077,10 +1077,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'
|
||||
|
@ -1092,7 +1092,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):
|
||||
|
@ -1101,7 +1101,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'
|
||||
|
@ -1121,11 +1121,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'
|
||||
|
@ -1145,10 +1145,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