From 69099b060e0df340ebd8b9a9896c4930d300cbf2 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 16 Jun 2016 14:59:13 +0200 Subject: [PATCH] SystemExit exception was swallowed in in thread It was causing patroni failing to stop after receiving SIGTERM. Acceptance tests was killing it with SIGKILL which was causing further tests fail because postgres was still running: 2016-06-16 14:36:24,444 INFO: no action. i am the leader with the lock 2016-06-16 14:36:25,448 INFO: Lock owner: postgres0; I am postgres0 2016-06-16 14:36:25,452 ERROR: Failed to update /service/batman/optime/leader Traceback (most recent call last): File "/home/akukushkin/git/patroni/patroni/dcs/zookeeper.py", line 208, in write_leader_optime self._client.retry(self._client.set, path, last_operation) File "/home/akukushkin/git/patroni/py2/local/lib/python2.7/site-packages/kazoo/client.py", line 273, in _retry return self._retry.copy()(*args, **kwargs) File "/home/akukushkin/git/patroni/py2/local/lib/python2.7/site-packages/kazoo/retry.py", line 123, in __call__ return func(*args, **kwargs) File "/home/akukushkin/git/patroni/py2/local/lib/python2.7/site-packages/kazoo/client.py", line 1219, in set return self.set_async(path, value, version).get() File "/home/akukushkin/git/patroni/py2/local/lib/python2.7/site-packages/kazoo/handlers/utils.py", line 74, in get self._condition.wait(timeout) File "/usr/lib/python2.7/threading.py", line 340, in wait waiter.acquire() File "/home/akukushkin/git/patroni/patroni/utils.py", line 219, in sigterm_handler sys.exit() SystemExit 2016-06-16 14:36:25,453 INFO: no action. i am the leader with the lock 2016-06-16 14:36:26,443 INFO: Lock owner: postgres0; I am postgres0 2016-06-16 14:36:26,444 INFO: no action. i am the leader with the lock --- patroni/__init__.py | 18 +++++++++++++----- patroni/utils.py | 19 ------------------- tests/test_patroni.py | 3 +++ tests/test_utils.py | 7 +------ 4 files changed, 17 insertions(+), 30 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index c48eab40..ef1f29c5 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -8,7 +8,7 @@ from patroni.dcs import get_dcs from patroni.exceptions import DCSError from patroni.ha import Ha from patroni.postgresql import Postgresql -from patroni.utils import reap_children, set_ignore_sigterm, setup_signal_handlers +from patroni.utils import reap_children, sigchld_handler from patroni.version import __version__ logger = logging.getLogger(__name__) @@ -32,6 +32,7 @@ class Patroni(object): self._reload_config_scheduled = False self._received_sighup = False + self._received_sigterm = False def load_dynamic_configuration(self): while True: @@ -63,6 +64,9 @@ class Patroni(object): def sighup_handler(self, *args): self._received_sighup = True + def sigterm_handler(self, *args): + self._received_sigterm = True + @property def noloadbalance(self): return self.tags.get('noloadbalance', False) @@ -86,10 +90,9 @@ class Patroni(object): def run(self): self.api.start() - signal.signal(signal.SIGHUP, self.sighup_handler) self.next_run = time.time() - while True: + while not self._received_sigterm: if self._received_sighup: self._received_sighup = False if self.config.reload_local_configuration(): @@ -107,17 +110,22 @@ class Patroni(object): reap_children() self.schedule_next_run() + def setup_signal_handlers(self): + signal.signal(signal.SIGHUP, self.sighup_handler) + signal.signal(signal.SIGHUP, self.sigterm_handler) + signal.signal(signal.SIGCHLD, sigchld_handler) + def main(): logging.basicConfig(format='%(asctime)s %(levelname)s: %(message)s', level=logging.INFO) logging.getLogger('requests').setLevel(logging.WARNING) - setup_signal_handlers() patroni = Patroni() + patroni.setup_signal_handlers() try: patroni.run() except KeyboardInterrupt: - set_ignore_sigterm() + pass finally: patroni.api.shutdown() patroni.postgresql.stop(checkpoint=False) diff --git a/patroni/utils.py b/patroni/utils.py index 1b604406..a200292e 100644 --- a/patroni/utils.py +++ b/patroni/utils.py @@ -1,16 +1,13 @@ import datetime import os import random -import signal import six -import sys import time import pytz import dateutil.parser from patroni.exceptions import PatroniException -__ignore_sigterm = False __interrupted_sleep = False __reap_children = False @@ -208,17 +205,6 @@ def compare_values(vartype, unit, old_value, new_value): return old_value is not None and new_value is not None and old_value == new_value -def set_ignore_sigterm(value=True): - global __ignore_sigterm - __ignore_sigterm = value - - -def sigterm_handler(signo, stack_frame): - if not __ignore_sigterm: - set_ignore_sigterm() - sys.exit() - - def sigchld_handler(signo, stack_frame): global __interrupted_sleep, __reap_children __reap_children = __interrupted_sleep = True @@ -237,11 +223,6 @@ def sleep(interval): __interrupted_sleep = False -def setup_signal_handlers(): - signal.signal(signal.SIGTERM, sigterm_handler) - signal.signal(signal.SIGCHLD, sigchld_handler) - - def reap_children(): global __reap_children if __reap_children: diff --git a/tests/test_patroni.py b/tests/test_patroni.py index f9246cac..ef2a5fc0 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -68,6 +68,9 @@ class TestPatroni(unittest.TestCase): with patch('patroni.postgresql.Postgresql.data_directory_empty', Mock(return_value=False)): self.assertRaises(SleepException, self.p.run) + def test_sigterm_handler(self): + self.p.sigterm_handler() + def test_schedule_next_run(self): self.p.ha.dcs.watch = Mock(return_value=True) self.p.schedule_next_run() diff --git a/tests/test_utils.py b/tests/test_utils.py index af4cd861..b32d6b35 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,8 +2,7 @@ import unittest from mock import Mock, patch from patroni.exceptions import PatroniException -from patroni.utils import reap_children, Retry, RetryFailedError, set_ignore_sigterm,\ - sigchld_handler, sigterm_handler, sleep +from patroni.utils import reap_children, Retry, RetryFailedError, sigchld_handler, sleep def time_sleep(_): @@ -12,10 +11,6 @@ def time_sleep(_): class TestUtils(unittest.TestCase): - def test_sigterm_handler(self): - set_ignore_sigterm(False) - self.assertRaises(SystemExit, sigterm_handler, None, None) - @patch('time.sleep', Mock()) def test_reap_children(self): self.assertIsNone(reap_children())