From 691211b107e63ef1d7c046c4289fadbc23e623cc Mon Sep 17 00:00:00 2001 From: Arne Wiebalck Date: Mon, 22 Feb 2021 13:38:01 +0100 Subject: [PATCH] Lazy-load node details from the DB In order to reduce the load on the database backend, only lazy-load a node's ports, portgroups, volume_connectors, and volume_targets. With the power-sync as the main user, this change should reduce the number of DB operations by two thirds roughly. Change-Id: Id9a9a53156f7fd866d93569347a81e27c6f0673c (cherry picked from commit 82cab603bbb21d81769cdc4f50d4fa445d026e5c) --- ironic/conductor/task_manager.py | 72 +++++++++- ironic/tests/unit/conductor/test_manager.py | 2 +- .../tests/unit/conductor/test_task_manager.py | 128 +++++++++++------- .../unit/drivers/modules/irmc/test_inspect.py | 17 --- ...askmanager-lazy-load-32a14526c647c2f0.yaml | 9 ++ 5 files changed, 153 insertions(+), 75 deletions(-) create mode 100644 releasenotes/notes/taskmanager-lazy-load-32a14526c647c2f0.yaml diff --git a/ironic/conductor/task_manager.py b/ironic/conductor/task_manager.py index df68c27aa4..a4910e8e8d 100644 --- a/ironic/conductor/task_manager.py +++ b/ironic/conductor/task_manager.py @@ -200,6 +200,10 @@ class TaskManager(object): self.context = context self._node = None + self._ports = None + self._portgroups = None + self._volume_connectors = None + self._volume_targets = None self.node_id = node_id self.shared = shared self._retry = retry @@ -226,13 +230,6 @@ class TaskManager(object): self._debug_timer.restart() self.node = node - self.ports = objects.Port.list_by_node_id(context, self.node.id) - self.portgroups = objects.Portgroup.list_by_node_id(context, - self.node.id) - self.volume_connectors = objects.VolumeConnector.list_by_node_id( - context, self.node.id) - self.volume_targets = objects.VolumeTarget.list_by_node_id( - context, self.node.id) if load_driver: self.driver = driver_factory.build_driver_for_task(self) else: @@ -253,6 +250,67 @@ class TaskManager(object): self.fsm.initialize(start_state=self.node.provision_state, target_state=self.node.target_provision_state) + @property + def ports(self): + try: + if self._ports is None: + self._ports = objects.Port.list_by_node_id(self.context, + self.node.id) + except Exception: + with excutils.save_and_reraise_exception(): + self.release_resources() + return self._ports + + @ports.setter + def ports(self, ports): + self._ports = ports + + @property + def portgroups(self): + try: + if self._portgroups is None: + self._portgroups = objects.Portgroup.list_by_node_id( + self.context, self.node.id) + except Exception: + with excutils.save_and_reraise_exception(): + self.release_resources() + return self._portgroups + + @portgroups.setter + def portgroups(self, portgroups): + self._portgroups = portgroups + + @property + def volume_connectors(self): + try: + if self._volume_connectors is None: + self._volume_connectors = \ + objects.VolumeConnector.list_by_node_id( + self.context, self.node.id) + except Exception: + with excutils.save_and_reraise_exception(): + self.release_resources() + return self._volume_connectors + + @volume_connectors.setter + def volume_connectors(self, volume_connectors): + self._volume_connectors = volume_connectors + + @property + def volume_targets(self): + try: + if self._volume_targets is None: + self._volume_targets = objects.VolumeTarget.list_by_node_id( + self.context, self.node.id) + except Exception: + with excutils.save_and_reraise_exception(): + self.release_resources() + return self._volume_targets + + @volume_targets.setter + def volume_targets(self, volume_targets): + self._volume_targets = volume_targets + def load_driver(self): if self.driver is None: self.driver = driver_factory.build_driver_for_task(self) diff --git a/ironic/tests/unit/conductor/test_manager.py b/ironic/tests/unit/conductor/test_manager.py index a532979110..e7b8121c21 100644 --- a/ironic/tests/unit/conductor/test_manager.py +++ b/ironic/tests/unit/conductor/test_manager.py @@ -3388,9 +3388,9 @@ class ConsoleTestCase(mgr_utils.ServiceSetUpMixin, db_base.DbTestCase): console_enabled=True) self._start_service() self.service.set_console_mode(self.context, node.uuid, True) - self._stop_service() self.assertFalse(mock_sc.called) self.assertFalse(mock_notify.called) + self._stop_service() @mock.patch.object(fake.FakeConsole, 'stop_console', autospec=True) @mock.patch.object(notification_utils, 'emit_console_notification') diff --git a/ironic/tests/unit/conductor/test_task_manager.py b/ironic/tests/unit/conductor/test_task_manager.py index ced512dfd5..13de0a2414 100644 --- a/ironic/tests/unit/conductor/test_task_manager.py +++ b/ironic/tests/unit/conductor/test_task_manager.py @@ -112,7 +112,13 @@ class TaskManagerTestCase(db_base.DbTestCase): get_voltgt_mock.return_value = mock.sentinel.voltgt1 build_driver_mock.return_value = mock.sentinel.driver1 + # Note(arne_wiebalck): Force loading of lazy-loaded properties. + def _eval_all(task): + return task.ports, task.portgroups, task.volume_targets, \ + task.volume_connectors + with task_manager.TaskManager(self.context, 'node-id1') as task: + _eval_all(task) reserve_mock.return_value = node2 get_ports_mock.return_value = mock.sentinel.ports2 get_portgroups_mock.return_value = mock.sentinel.portgroups2 @@ -120,6 +126,7 @@ class TaskManagerTestCase(db_base.DbTestCase): get_voltgt_mock.return_value = mock.sentinel.voltgt2 build_driver_mock.return_value = mock.sentinel.driver2 with task_manager.TaskManager(self.context, 'node-id2') as task2: + _eval_all(task2) self.assertEqual(self.context, task.context) self.assertEqual(self.node, task.node) self.assertEqual(mock.sentinel.ports1, task.ports) @@ -231,16 +238,18 @@ class TaskManagerTestCase(db_base.DbTestCase): reserve_mock.return_value = self.node get_ports_mock.side_effect = exception.IronicException('foo') - self.assertRaises(exception.IronicException, - task_manager.TaskManager, - self.context, - 'fake-node-id') + # Note(arne_wiebalck): Force loading of lazy-loaded properties. + def _eval_ports(task): + return task.ports + + with task_manager.TaskManager(self.context, 'fake-node-id') as task: + self.assertRaises(exception.IronicException, _eval_ports, task) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') reserve_mock.assert_called_once_with(self.context, self.host, 'fake-node-id') get_ports_mock.assert_called_once_with(self.context, self.node.id) - self.assertFalse(build_driver_mock.called) + self.assertTrue(build_driver_mock.called) release_mock.assert_called_once_with(self.context, self.host, self.node.id) @@ -251,16 +260,19 @@ class TaskManagerTestCase(db_base.DbTestCase): reserve_mock.return_value = self.node get_portgroups_mock.side_effect = exception.IronicException('foo') - self.assertRaises(exception.IronicException, - task_manager.TaskManager, - self.context, - 'fake-node-id') + # Note(arne_wiebalck): Force loading of lazy-loaded properties. + def _eval_portgroups(task): + return task.portgroups + + with task_manager.TaskManager(self.context, 'fake-node-id') as task: + self.assertRaises(exception.IronicException, _eval_portgroups, + task) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') reserve_mock.assert_called_once_with(self.context, self.host, 'fake-node-id') get_portgroups_mock.assert_called_once_with(self.context, self.node.id) - self.assertFalse(build_driver_mock.called) + self.assertTrue(build_driver_mock.called) release_mock.assert_called_once_with(self.context, self.host, self.node.id) @@ -271,15 +283,18 @@ class TaskManagerTestCase(db_base.DbTestCase): reserve_mock.return_value = self.node get_volconn_mock.side_effect = exception.IronicException('foo') - self.assertRaises(exception.IronicException, - task_manager.TaskManager, - self.context, - 'fake-node-id') + # Note(arne_wiebalck): Force loading of lazy-loaded properties. + def _eval_volconn(task): + return task.volume_connectors + + with task_manager.TaskManager(self.context, 'fake-node-id') as task: + self.assertRaises(exception.IronicException, _eval_volconn, + task) reserve_mock.assert_called_once_with(self.context, self.host, 'fake-node-id') get_volconn_mock.assert_called_once_with(self.context, self.node.id) - self.assertFalse(build_driver_mock.called) + self.assertTrue(build_driver_mock.called) release_mock.assert_called_once_with(self.context, self.host, self.node.id) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') @@ -291,15 +306,17 @@ class TaskManagerTestCase(db_base.DbTestCase): reserve_mock.return_value = self.node get_voltgt_mock.side_effect = exception.IronicException('foo') - self.assertRaises(exception.IronicException, - task_manager.TaskManager, - self.context, - 'fake-node-id') + # Note(arne_wiebalck): Force loading of lazy-loaded properties. + def _eval_voltgt(task): + return task.volume_targets + + with task_manager.TaskManager(self.context, 'fake-node-id') as task: + self.assertRaises(exception.IronicException, _eval_voltgt, task) reserve_mock.assert_called_once_with(self.context, self.host, 'fake-node-id') get_voltgt_mock.assert_called_once_with(self.context, self.node.id) - self.assertFalse(build_driver_mock.called) + self.assertTrue(build_driver_mock.called) release_mock.assert_called_once_with(self.context, self.host, self.node.id) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') @@ -320,8 +337,10 @@ class TaskManagerTestCase(db_base.DbTestCase): node_get_mock.assert_called_once_with(self.context, 'fake-node-id') reserve_mock.assert_called_once_with(self.context, self.host, 'fake-node-id') - get_ports_mock.assert_called_once_with(self.context, self.node.id) - get_portgroups_mock.assert_called_once_with(self.context, self.node.id) + self.assertFalse(get_ports_mock.called) + self.assertFalse(get_portgroups_mock.called) + self.assertFalse(get_volconn_mock.called) + self.assertFalse(get_voltgt_mock.called) build_driver_mock.assert_called_once_with(mock.ANY) release_mock.assert_called_once_with(self.context, self.host, self.node.id) @@ -381,17 +400,19 @@ class TaskManagerTestCase(db_base.DbTestCase): node_get_mock.return_value = self.node get_ports_mock.side_effect = exception.IronicException('foo') - self.assertRaises(exception.IronicException, - task_manager.TaskManager, - self.context, - 'fake-node-id', - shared=True) + # Note(arne_wiebalck): Force loading of lazy-loaded properties. + def _eval_ports(task): + return task.ports + + with task_manager.TaskManager(self.context, 'fake-node-id', + shared=True) as task: + self.assertRaises(exception.IronicException, _eval_ports, task) self.assertFalse(reserve_mock.called) self.assertFalse(release_mock.called) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') get_ports_mock.assert_called_once_with(self.context, self.node.id) - self.assertFalse(build_driver_mock.called) + self.assertTrue(build_driver_mock.called) def test_shared_lock_get_portgroups_exception( self, get_voltgt_mock, get_volconn_mock, get_portgroups_mock, @@ -400,17 +421,20 @@ class TaskManagerTestCase(db_base.DbTestCase): node_get_mock.return_value = self.node get_portgroups_mock.side_effect = exception.IronicException('foo') - self.assertRaises(exception.IronicException, - task_manager.TaskManager, - self.context, - 'fake-node-id', - shared=True) + # Note(arne_wiebalck): Force loading of lazy-loaded properties. + def _eval_portgroups(task): + return task.portgroups + + with task_manager.TaskManager(self.context, 'fake-node-id', + shared=True) as task: + self.assertRaises(exception.IronicException, _eval_portgroups, + task) self.assertFalse(reserve_mock.called) self.assertFalse(release_mock.called) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') get_portgroups_mock.assert_called_once_with(self.context, self.node.id) - self.assertFalse(build_driver_mock.called) + self.assertTrue(build_driver_mock.called) def test_shared_lock_get_volconn_exception( self, get_voltgt_mock, get_volconn_mock, get_portgroups_mock, @@ -419,17 +443,19 @@ class TaskManagerTestCase(db_base.DbTestCase): node_get_mock.return_value = self.node get_volconn_mock.side_effect = exception.IronicException('foo') - self.assertRaises(exception.IronicException, - task_manager.TaskManager, - self.context, - 'fake-node-id', - shared=True) + # Note(arne_wiebalck): Force loading of lazy-loaded properties. + def _eval_volconn(task): + return task.volume_connectors + + with task_manager.TaskManager(self.context, 'fake-node-id', + shared=True) as task: + self.assertRaises(exception.IronicException, _eval_volconn, task) self.assertFalse(reserve_mock.called) self.assertFalse(release_mock.called) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') get_volconn_mock.assert_called_once_with(self.context, self.node.id) - self.assertFalse(get_voltgt_mock.called) + self.assertTrue(build_driver_mock.called) def test_shared_lock_get_voltgt_exception( self, get_voltgt_mock, get_volconn_mock, get_portgroups_mock, @@ -438,17 +464,19 @@ class TaskManagerTestCase(db_base.DbTestCase): node_get_mock.return_value = self.node get_voltgt_mock.side_effect = exception.IronicException('foo') - self.assertRaises(exception.IronicException, - task_manager.TaskManager, - self.context, - 'fake-node-id', - shared=True) + # Note(arne_wiebalck): Force loading of lazy-loaded properties. + def _eval_voltgt(task): + return task.volume_targets + + with task_manager.TaskManager(self.context, 'fake-node-id', + shared=True) as task: + self.assertRaises(exception.IronicException, _eval_voltgt, task) self.assertFalse(reserve_mock.called) self.assertFalse(release_mock.called) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') get_voltgt_mock.assert_called_once_with(self.context, self.node.id) - self.assertFalse(build_driver_mock.called) + self.assertTrue(build_driver_mock.called) def test_shared_lock_build_driver_exception( self, get_voltgt_mock, get_volconn_mock, get_portgroups_mock, @@ -467,10 +495,10 @@ class TaskManagerTestCase(db_base.DbTestCase): self.assertFalse(reserve_mock.called) self.assertFalse(release_mock.called) node_get_mock.assert_called_once_with(self.context, 'fake-node-id') - get_ports_mock.assert_called_once_with(self.context, self.node.id) - get_portgroups_mock.assert_called_once_with(self.context, self.node.id) - get_volconn_mock.assert_called_once_with(self.context, self.node.id) - get_voltgt_mock.assert_called_once_with(self.context, self.node.id) + self.assertFalse(get_ports_mock.called) + self.assertFalse(get_portgroups_mock.called) + self.assertFalse(get_voltgt_mock.called) + self.assertFalse(get_volconn_mock.called) build_driver_mock.assert_called_once_with(mock.ANY) def test_upgrade_lock( diff --git a/ironic/tests/unit/drivers/modules/irmc/test_inspect.py b/ironic/tests/unit/drivers/modules/irmc/test_inspect.py index b0bd206a37..f4e67ddcd7 100644 --- a/ironic/tests/unit/drivers/modules/irmc/test_inspect.py +++ b/ironic/tests/unit/drivers/modules/irmc/test_inspect.py @@ -205,24 +205,7 @@ class IRMCInspectTestCase(test_common.BaseIRMCTest): _inspect_hardware_mock.assert_called_once_with(task.node, existing_traits) - # note (naohirot): - # as of mock 1.2, assert_has_calls has a bug which returns - # "AssertionError: Calls not found." if mock_calls has class - # method call such as below: - - # AssertionError: Calls not found. - # Expected: [call.list_by_node_id( - # , - # 1)] - # Actual: [call.list_by_node_id( - # , - # 1)] - # - # workaround, remove class method call from mock_calls list - del port_mock.mock_calls[0] port_mock.assert_has_calls([ - # workaround, comment out class method call from expected list - # mock.call.list_by_node_id(task.context, node_id), mock.call(task.context, address=inspected_macs[0], node_id=node_id), mock.call(task.context, address=inspected_macs[1], diff --git a/releasenotes/notes/taskmanager-lazy-load-32a14526c647c2f0.yaml b/releasenotes/notes/taskmanager-lazy-load-32a14526c647c2f0.yaml new file mode 100644 index 0000000000..47aca790b6 --- /dev/null +++ b/releasenotes/notes/taskmanager-lazy-load-32a14526c647c2f0.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + Introduces lazy-loading of ports, portgroups, volume connections + and volume targets in task manager to fix performance issues. + For periodic tasks which create a task manager object but don't + require the aforementioned data (e.g. power sync), this change + should reduce the number of database interactions by around two + thirds, speeding up overall execution.