From ee5677bd61de3a044502013bf097bb3622d7e934 Mon Sep 17 00:00:00 2001 From: jufeng <jesse@easystack.cn> Date: Fri, 10 Feb 2017 12:19:56 +0800 Subject: [PATCH] Optimize pid property in AsyncProcess class The pid property in the the AsyncProcess process class executes command 'ps --ppid <pid> -o pid=' every time the associated asynchronous process's pid is needed. This can be very heavy in high load network and compute nodes. Please see related bug. This change optimizes the pid property to execute 'ps --ppid <pid> -o pid=' only the first time it is called, memoizing the retrieved pid for future calls. Change-Id: Idd347b0b22320cbfdb5d6a738e218e5788dd7d6b Closes-Bug:1663465 --- neutron/agent/linux/async_process.py | 17 ++++++++++------ .../unit/agent/linux/test_async_process.py | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/neutron/agent/linux/async_process.py b/neutron/agent/linux/async_process.py index a29011ee6e7..21af640f095 100644 --- a/neutron/agent/linux/async_process.py +++ b/neutron/agent/linux/async_process.py @@ -78,6 +78,7 @@ class AsyncProcess(object): raise ValueError(_('respawn_interval must be >= 0 if provided.')) self.respawn_interval = respawn_interval self._process = None + self._pid = None self._is_running = False self._kill_event = None self._reset_queues() @@ -95,8 +96,8 @@ class AsyncProcess(object): def is_active(self): # If using sudo rootwrap as a root_helper, we have to wait until sudo - # spawns rootwrap and rootwrap spawns the process. - + # spawns rootwrap and rootwrap spawns the process. self.pid will make + # sure to get the correct pid. return utils.pid_invoked_with_cmdline( self.pid, self.cmd_without_namespace) @@ -137,6 +138,7 @@ class AsyncProcess(object): def _spawn(self): """Spawn a process and its watchers.""" self._is_running = True + self._pid = None self._kill_event = eventlet.event.Event() self._process, cmd = utils.create_process(self._cmd, run_as_root=self.run_as_root) @@ -154,16 +156,19 @@ class AsyncProcess(object): @property def pid(self): if self._process: - return utils.get_root_helper_child_pid( - self._process.pid, - self.cmd_without_namespace, - run_as_root=self.run_as_root) + if not self._pid: + self._pid = utils.get_root_helper_child_pid( + self._process.pid, + self.cmd_without_namespace, + run_as_root=self.run_as_root) + return self._pid def _kill(self, kill_signal): """Kill the process and the associated watcher greenthreads.""" pid = self.pid if pid: self._is_running = False + self._pid = None self._kill_process(pid, kill_signal) # Halt the greenthreads if they weren't already. diff --git a/neutron/tests/unit/agent/linux/test_async_process.py b/neutron/tests/unit/agent/linux/test_async_process.py index 73d54f1235d..dc19b5b3334 100644 --- a/neutron/tests/unit/agent/linux/test_async_process.py +++ b/neutron/tests/unit/agent/linux/test_async_process.py @@ -56,6 +56,25 @@ class TestAsyncProcess(base.BaseTestCase): ]) self.assertEqual(len(proc._watchers), 2) + def test__pid_none(self): + pid = 1 + self.proc._pid = None + with mock.patch.object(self.proc, '_process') as _process: + with mock.patch.object(utils, + 'get_root_helper_child_pid') as func: + func.return_value = pid + self.assertEqual(self.proc.pid, pid) + func.assert_called_once_with(_process.pid, ['fake'], + run_as_root=False) + self.assertEqual(self.proc._pid, pid) + + def test__pid_not_none(self): + self.proc._pid = 1 + with mock.patch.object(self.proc, '_process'),\ + mock.patch.object(utils, 'get_root_helper_child_pid') as func: + self.assertEqual(self.proc.pid, 1) + func.assert_not_called() + def test__handle_process_error_kills_with_respawn(self): with mock.patch.object(self.proc, '_kill') as kill: self.proc._handle_process_error() @@ -185,6 +204,7 @@ class TestAsyncProcess(base.BaseTestCase): self.assertIsNone(self.proc._kill_event) self.assertFalse(self.proc._is_running) + self.assertIsNone(self.proc._pid) mock_kill_event.send.assert_called_once_with() if pid: