From f1819443ef7ec55b3d2a28aab9b7f0c422e038d3 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 2 Dec 2019 12:16:18 +0100 Subject: [PATCH] Avoid spawning semaphore tracker process (#1299) We are not using semaphores, therefore we don't need to track them. --- patroni/__init__.py | 6 +----- patroni/postgresql/postmaster.py | 14 ++++++++++++-- tests/test_bootstrap.py | 4 +++- tests/test_patroni.py | 1 - tests/test_postmaster.py | 2 ++ 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index f0e06df4..e3699986 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -217,11 +217,6 @@ def check_psycopg2(): def main(): - import multiprocessing - if sys.version_info >= (3, 4): # pragma: no cover - # The default, forking, method is not a good idea in a multithreaded process: https://bugs.python.org/issue6721 - multiprocessing.set_start_method('spawn') - check_psycopg2() if os.getpid() != 1: return patroni_main() @@ -255,6 +250,7 @@ def main(): signal.signal(signal.SIGABRT, passtochild) signal.signal(signal.SIGTERM, passtochild) + import multiprocessing patroni = multiprocessing.Process(target=patroni_main) patroni.start() pid = patroni.pid diff --git a/patroni/postgresql/postmaster.py b/patroni/postgresql/postmaster.py index 61dfa419..f2f23b25 100644 --- a/patroni/postgresql/postmaster.py +++ b/patroni/postgresql/postmaster.py @@ -5,9 +5,18 @@ import psutil import re import signal import subprocess +import sys from patroni import PATRONI_ENV_PREFIX +# avoid spawning the resource tracker process +if sys.version_info >= (3, 8): # pragma: no cover + import multiprocessing.resource_tracker + multiprocessing.resource_tracker.getfd = lambda: 0 +elif sys.version_info >= (3, 4): # pragma: no cover + import multiprocessing.semaphore_tracker + multiprocessing.semaphore_tracker.getfd = lambda: 0 + logger = logging.getLogger(__name__) STOP_SIGNALS = { @@ -172,8 +181,9 @@ class PostmasterProcess(psutil.Process): pass cmdline = [pgcommand, '-D', data_dir, '--config-file={}'.format(conf)] + options logger.debug("Starting postgres: %s", " ".join(cmdline)) - parent_conn, child_conn = multiprocessing.Pipe(False) - proc = multiprocessing.Process(target=pg_ctl_start, args=(child_conn, cmdline, env)) + ctx = multiprocessing.get_context('spawn') if sys.version_info >= (3, 4) else multiprocessing + parent_conn, child_conn = ctx.Pipe(False) + proc = ctx.Process(target=pg_ctl_start, args=(child_conn, cmdline, env)) proc.start() pid = parent_conn.recv() proc.join() diff --git a/tests/test_bootstrap.py b/tests/test_bootstrap.py index 3268c617..904ec26b 100644 --- a/tests/test_bootstrap.py +++ b/tests/test_bootstrap.py @@ -112,7 +112,8 @@ class TestBootstrap(BaseTestPostgresql): config = {'users': {'replicator': {'password': 'rep-pass', 'options': ['replication']}}} with patch.object(Postgresql, 'is_running', Mock(return_value=False)),\ - patch('multiprocessing.Process', Mock(side_effect=Exception)): + patch('multiprocessing.Process', Mock(side_effect=Exception)),\ + patch('multiprocessing.get_context', Mock(side_effect=Exception), create=True): self.assertRaises(Exception, self.b.bootstrap, config) with open(os.path.join(self.p.data_dir, 'pg_hba.conf')) as f: lines = f.readlines() @@ -140,6 +141,7 @@ class TestBootstrap(BaseTestPostgresql): mock_cancellable_subprocess_call.return_value = 0 with patch('multiprocessing.Process', Mock(side_effect=Exception("42"))),\ + patch('multiprocessing.get_context', Mock(side_effect=Exception("42")), create=True),\ patch('os.path.isfile', Mock(return_value=True)),\ patch('os.unlink', Mock()),\ patch.object(ConfigHandler, 'save_configuration_files', Mock()),\ diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 9e36de14..1611d970 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -86,7 +86,6 @@ class TestPatroni(unittest.TestCase): @patch('os.getpid') @patch('multiprocessing.Process') @patch('patroni.patroni_main', Mock()) - @patch('multiprocessing.set_start_method', Mock(), create=True) def test_patroni_main(self, mock_process, mock_getpid): mock_getpid.return_value = 2 _main() diff --git a/tests/test_postmaster.py b/tests/test_postmaster.py index 75dff566..83afaeea 100644 --- a/tests/test_postmaster.py +++ b/tests/test_postmaster.py @@ -1,3 +1,4 @@ +import multiprocessing import psutil import unittest @@ -96,6 +97,7 @@ class TestPostmasterProcess(unittest.TestCase): @patch('subprocess.Popen') @patch('os.setsid', Mock(), create=True) @patch('multiprocessing.Process', MockProcess) + @patch('multiprocessing.get_context', Mock(return_value=multiprocessing), create=True) @patch.object(PostmasterProcess, 'from_pid') @patch.object(PostmasterProcess, '_from_pidfile') def test_start(self, mock_frompidfile, mock_frompid, mock_popen):