diff --git a/nova/scheduler/host_manager.py b/nova/scheduler/host_manager.py index aab4b90ac53f..bf3abbf508bb 100644 --- a/nova/scheduler/host_manager.py +++ b/nova/scheduler/host_manager.py @@ -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. diff --git a/nova/tests/unit/scheduler/test_host_manager.py b/nova/tests/unit/scheduler/test_host_manager.py index bfddb0ce6ec4..fbfb66336477 100644 --- a/nova/tests/unit/scheduler/test_host_manager.py +++ b/nova/tests/unit/scheduler/test_host_manager.py @@ -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] diff --git a/releasenotes/notes/full_host_state_instances-6fbc828564a000ec.yaml b/releasenotes/notes/full_host_state_instances-6fbc828564a000ec.yaml new file mode 100644 index 000000000000..7ab5a115b2f2 --- /dev/null +++ b/releasenotes/notes/full_host_state_instances-6fbc828564a000ec.yaml @@ -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) \ No newline at end of file