From 309b5d4803d0c88ae93d1fee70151d169daacccb Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 11 Apr 2016 17:53:51 +0200 Subject: [PATCH] Remove restrictions on running pg_rewind. Previously, pg_rewind was called only if a crashed master tried to rejoin the cluster. It didn't cover the important case of a master shut down cleanly, but with a combination of a smart shutdown and subsequently a fast shutdown. Since out pg_rewind code does not depend on the "uncleanness" of the master's shutdown, we can call it unconditionally in all cases where the former master tries to rejoin as a replica. This resolves #167. --- patroni/ha.py | 6 ------ patroni/postgresql.py | 13 +++---------- tests/test_postgresql.py | 2 -- 3 files changed, 3 insertions(+), 18 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 21999304..a8c6760a 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -106,12 +106,6 @@ class Ha(object): return 'waiting for leader to bootstrap' def recover(self): - # try to see if we are the former master that crashed. If so - we likely need to run pg_rewind - # in order to join the former standby being promoted. - if self.state_handler.role == 'master': - pg_controldata = self.state_handler.controldata() - if pg_controldata and pg_controldata.get('Database cluster state', '') == 'in production': # crashed master - self.state_handler.require_rewind() self.recovering = True return self.follow("starting as readonly because i had the session lock", "starting as a secondary", True, True) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index cf2fddfa..529f5e99 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -73,7 +73,6 @@ class Postgresql(object): self._connection = None self._cursor_holder = None - self._need_rewind = False self._sysid = None self.replication_slots = [] # list of already existing replication slots self.retry = Retry(max_tries=-1, deadline=5, max_delay=1, retry_exceptions=PostgresConnectionException) @@ -115,9 +114,6 @@ class Postgresql(object): self._sysid = data.get('Database system identifier', "") return self._sysid - def require_rewind(self): - self._need_rewind = True - def get_local_address(self): listen_addresses = self.listen_addresses.split(',') local_address = listen_addresses[0].strip() # take first address from listen_addresses @@ -564,13 +560,12 @@ recovery_target_timeline = 'latest' def follow(self, leader, recovery=False): if self.check_recovery_conf(leader) and not recovery: return True - change_role = self.role == 'master' - self._need_rewind = (self._need_rewind or change_role) and self.can_rewind - if self._need_rewind: + need_rewind = change_role and self.can_rewind + if need_rewind: logger.info("set the rewind flag after demote") self.write_recovery_conf(leader) - if leader and self._need_rewind: # we have a leader and need to rewind + if leader and need_rewind: # we have a leader and need to rewind if self.is_running(): self.stop() # at present, pg_rewind only runs when the cluster is shut down cleanly @@ -594,7 +589,6 @@ recovery_target_timeline = 'latest' logger.error("unable to rewind the former master") self.remove_data_directory() ret = True - self._need_rewind = False else: # do not rewind until the leader becomes available ret = self.restart() if change_role and ret: @@ -630,7 +624,6 @@ recovery_target_timeline = 'latest' if ret: self.set_role('master') logger.info("cleared rewind flag after becoming the leader") - self._need_rewind = False self.call_nowait(ACTION_ON_ROLE_CHANGE) return ret diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 932e860d..8bd563b9 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -257,12 +257,10 @@ class TestPostgresql(unittest.TestCase): self.p.follow(Leader(-1, 28, self.other)) self.p.rewind = mock_pg_rewind self.p.follow(self.leader) - self.p.require_rewind() with mock.patch('os.path.islink', MagicMock(return_value=True)): with mock.patch('patroni.postgresql.Postgresql.can_rewind', new_callable=PropertyMock(return_value=True)): with mock.patch('os.unlink', MagicMock(return_value=True)): self.p.follow(self.leader, recovery=True) - self.p.require_rewind() with mock.patch('patroni.postgresql.Postgresql.can_rewind', new_callable=PropertyMock(return_value=True)): self.p.rewind.return_value = True self.p.follow(self.leader, recovery=True)