From b6f057850a8b22ef42295fc38d9709d1510621aa Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 17 Aug 2022 09:23:07 +0200 Subject: [PATCH] Apply timeout when waiting for user backends to close (#2382) Close #2365 --- patroni/postgresql/__init__.py | 2 +- patroni/postgresql/postmaster.py | 18 +++++++++++------- tests/test_postmaster.py | 12 +++++++++--- 3 files changed, 21 insertions(+), 11 deletions(-) diff --git a/patroni/postgresql/__init__.py b/patroni/postgresql/__init__.py index a5b3d592..3e2e1d38 100644 --- a/patroni/postgresql/__init__.py +++ b/patroni/postgresql/__init__.py @@ -659,7 +659,7 @@ class Postgresql(object): if on_safepoint: # Wait for our connection to terminate so we can be sure that no new connections are being initiated self._wait_for_connection_close(postmaster) - postmaster.wait_for_user_backends_to_close() + postmaster.wait_for_user_backends_to_close(stop_timeout) on_safepoint() if on_shutdown and mode in ('fast', 'smart'): diff --git a/patroni/postgresql/postmaster.py b/patroni/postgresql/postmaster.py index f7b39622..e2a88856 100644 --- a/patroni/postgresql/postmaster.py +++ b/patroni/postgresql/postmaster.py @@ -171,8 +171,8 @@ class PostmasterProcess(psutil.Process): else: return not self.is_running() - def wait_for_user_backends_to_close(self): - # These regexps are cross checked against versions PostgreSQL 9.1 .. 11 + def wait_for_user_backends_to_close(self, stop_timeout): + # These regexps are cross checked against versions PostgreSQL 9.1 .. 15 aux_proc_re = re.compile("(?:postgres:)( .*:)? (?:(?:archiver|startup|autovacuum launcher|autovacuum worker|" "checkpointer|logger|stats collector|wal receiver|wal writer|writer)(?: process )?|" "walreceiver|wal sender process|walsender|walwriter|background writer|" @@ -184,19 +184,23 @@ class PostmasterProcess(psutil.Process): return logger.debug('Failed to get list of postmaster children') user_backends = [] - user_backends_cmdlines = [] + user_backends_cmdlines = {} for child in children: try: cmdline = child.cmdline() if cmdline and not aux_proc_re.match(cmdline[0]): user_backends.append(child) - user_backends_cmdlines.append(cmdline[0]) + user_backends_cmdlines[child.pid] = cmdline[0] except psutil.NoSuchProcess: pass if user_backends: - logger.debug('Waiting for user backends %s to close', ', '.join(user_backends_cmdlines)) - psutil.wait_procs(user_backends) - logger.debug("Backends closed") + logger.debug('Waiting for user backends %s to close', ', '.join(user_backends_cmdlines.values())) + gone, live = psutil.wait_procs(user_backends, stop_timeout) + if stop_timeout and live: + live = [user_backends_cmdlines[b.pid] for b in live] + logger.warning('Backends still alive after %s: %s', stop_timeout, ', '.join(live)) + else: + logger.debug("Backends closed") @staticmethod def start(pgcommand, data_dir, conf, options): diff --git a/tests/test_postmaster.py b/tests/test_postmaster.py index 07f09de2..61a45049 100644 --- a/tests/test_postmaster.py +++ b/tests/test_postmaster.py @@ -133,14 +133,20 @@ class TestPostmasterProcess(unittest.TestCase): c2.cmdline = Mock(return_value=["postgres: postgres postgres [local] idle"]) c3 = Mock() c3.cmdline = Mock(side_effect=psutil.NoSuchProcess(123)) + mock_wait.return_value = ([], [c2]) with patch('psutil.Process.children', Mock(return_value=[c1, c2, c3])): proc = PostmasterProcess(123) - self.assertIsNone(proc.wait_for_user_backends_to_close()) - mock_wait.assert_called_with([c2]) + self.assertIsNone(proc.wait_for_user_backends_to_close(1)) + mock_wait.assert_called_with([c2], 1) + + mock_wait.return_value = ([c2], []) + with patch('psutil.Process.children', Mock(return_value=[c1, c2, c3])): + proc = PostmasterProcess(123) + proc.wait_for_user_backends_to_close(1) with patch('psutil.Process.children', Mock(side_effect=psutil.NoSuchProcess(123))): proc = PostmasterProcess(123) - self.assertIsNone(proc.wait_for_user_backends_to_close()) + self.assertIsNone(proc.wait_for_user_backends_to_close(None)) @patch('subprocess.Popen') @patch('os.setsid', Mock(), create=True)