From 77aea03df9927acd62260cf058af516eaae98a2f Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 24 Aug 2017 07:53:32 +0200 Subject: [PATCH] Different bugfixes around pause state, mostly related to watchdog (#507) * Do not send keepalives if watchdog is not active * Avoid activating watchdog in a pause mode * Set correct postgres state in pause mode * Don't try to run queries from API if postgres is stopped --- features/watchdog.feature | 18 ++++++++++++++---- patroni/api.py | 2 ++ patroni/ha.py | 17 +++++++++-------- patroni/watchdog/base.py | 3 ++- tests/test_api.py | 3 ++- tests/test_ha.py | 3 ++- tests/test_watchdog.py | 3 ++- 7 files changed, 33 insertions(+), 16 deletions(-) diff --git a/features/watchdog.feature b/features/watchdog.feature index 00f4ddef..4f8215f6 100644 --- a/features/watchdog.feature +++ b/features/watchdog.feature @@ -1,16 +1,26 @@ Feature: watchdog Verify that watchdog gets pinged and triggered under appropriate circumstances. - Scenario: watchdog is opened, pinged and closed + Scenario: watchdog is opened and pinged Given I start postgres0 with watchdog Then postgres0 is a leader after 10 seconds And postgres0 role is the primary after 10 seconds And postgres0 watchdog has been pinged after 10 seconds - When I shut down postgres0 + + Scenario: watchdog is disabled during pause + Given I run patronictl.py pause batman + Then I receive a response returncode 0 + When I sleep for 2 seconds Then postgres0 watchdog has been closed - #TODO: test watchdog is disabled during pause - #TODO: test watchdog is disabled properly when shutting down + Scenario: watchdog is opened and pinged after resume + Given I run patronictl.py resume batman + Then I receive a response returncode 0 + And postgres0 watchdog has been pinged after 10 seconds + + Scenario: watchdog is disabled when shutting down + Given I shut down postgres0 + Then postgres0 watchdog has been closed Scenario: watchdog is triggered if patroni stops responding Given I start postgres0 with watchdog diff --git a/patroni/api.py b/patroni/api.py index 801b36ba..3dc7749a 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -376,6 +376,8 @@ class RestApiHandler(BaseHTTPRequestHandler): def get_postgresql_status(self, retry=False): try: + if self.server.patroni.postgresql.state not in ('running', 'restarting', 'starting'): + raise RetryFailedError('') row = self.query("""WITH replication_info AS ( SELECT usename, application_name, client_addr, state, sync_state, sync_priority FROM pg_stat_replication diff --git a/patroni/ha.py b/patroni/ha.py index 182bfe81..f3134a58 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -341,14 +341,13 @@ class Ha(object): self._disable_sync -= 1 def enforce_master_role(self, message, promote_message): - if not self.watchdog.is_running: - if not self.watchdog.activate(): - if self.state_handler.is_leader(): - self.demote('immediate') - return 'Demoting self because watchdog could not be activated' - else: - self.release_leader_key_voluntarily() - return 'Not promoting self because watchdog could not be actived' + if not self.is_paused() and not self.watchdog.is_running and not self.watchdog.activate(): + if self.state_handler.is_leader(): + self.demote('immediate') + return 'Demoting self because watchdog could not be activated' + else: + self.release_leader_key_voluntarily() + return 'Not promoting self because watchdog could not be activated' if self.state_handler.is_leader() or self.state_handler.role == 'master': # Inform the state handler about its master role. @@ -915,6 +914,8 @@ class Ha(object): # Check if we are in startup, when paused defer to main loop for manual failovers. if not self.state_handler.check_for_startup() or self.is_paused(): self.set_start_timeout(None) + if self.is_paused(): + self.state_handler.set_state(self.state_handler.is_running() and 'running' or 'stopped') return None # state_handler.state == 'starting' here diff --git a/patroni/watchdog/base.py b/patroni/watchdog/base.py index 9bdb638b..51fdb14a 100644 --- a/patroni/watchdog/base.py +++ b/patroni/watchdog/base.py @@ -202,7 +202,8 @@ class Watchdog(object): @synchronized def keepalive(self): try: - self.impl.keepalive() + if self.active: + self.impl.keepalive() # In case there are any pending configuration changes apply them now. if self.active and self.config != self.active_config: if self.config.mode != MODE_OFF and self.active_config.mode == MODE_OFF: diff --git a/tests/test_api.py b/tests/test_api.py index 37d05ece..f5fd0bf6 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -3,7 +3,7 @@ import json import psycopg2 import unittest -from mock import Mock, patch +from mock import Mock, PropertyMock, patch from patroni.api import RestApiHandler, RestApiServer from patroni.dcs import ClusterConfig, Member from patroni.ha import _MemberStatus @@ -152,6 +152,7 @@ class TestRestApiHandler(unittest.TestCase): def test_do_OPTIONS(self): self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'OPTIONS / HTTP/1.0')) + @patch.object(MockPostgresql, 'state', PropertyMock(return_value='stopped')) def test_do_GET_patroni(self): self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'GET /patroni')) diff --git a/tests/test_ha.py b/tests/test_ha.py index 6dc45491..49772774 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -63,6 +63,7 @@ def get_node_status(reachable=True, in_recovery=True, wal_position=10, nofailove return _MemberStatus(e, reachable, in_recovery, wal_position, tags, watchdog_failed) return fetch_node_status + future_restart_time = datetime.datetime.now(tzutc) + datetime.timedelta(days=5) postmaster_start_time = datetime.datetime.now(tzutc) @@ -246,7 +247,7 @@ class TestHa(unittest.TestCase): with patch.object(Watchdog, 'activate', Mock(return_value=False)): self.assertEquals(self.ha.run_cycle(), 'Demoting self because watchdog could not be activated') self.p.is_leader = false - self.assertEquals(self.ha.run_cycle(), 'Not promoting self because watchdog could not be actived') + self.assertEquals(self.ha.run_cycle(), 'Not promoting self because watchdog could not be activated') def test_leader_with_lock(self): self.ha.cluster.is_unlocked = false diff --git a/tests/test_watchdog.py b/tests/test_watchdog.py index d7775daf..85e45a91 100644 --- a/tests/test_watchdog.py +++ b/tests/test_watchdog.py @@ -132,8 +132,9 @@ class TestWatchdog(unittest.TestCase): def test_exceptions(self): wd = Watchdog({'ttl': 30, 'loop_wait': 10, 'watchdog': {'mode': 'bad'}}) wd.impl.close = wd.impl.keepalive = Mock(side_effect=WatchdogError('')) - self.assertIsNone(wd.disable()) + self.assertTrue(wd.activate()) self.assertIsNone(wd.keepalive()) + self.assertIsNone(wd.disable()) @patch('platform.system', Mock(return_value='Linux')) def test_config_reload(self):