From 39cbd5f1d69d34eb870cd8d365ff6f269c0dd954 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 11 Dec 2015 18:54:38 +0100 Subject: [PATCH] Unify all follow the leader calls from eventloop. Call normal follow the leader method from HA even during recovery. This provides a single place that changes recovery.conf, making it easier to plug in a cascading replica in the future. Remove an obsolete demote function from PostreSQL module, modified the tests. --- patroni/ha.py | 45 ++++++++++++++++++++++------------------ patroni/postgresql.py | 3 --- tests/test_ha.py | 8 ++++++- tests/test_postgresql.py | 2 -- 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 3a8d46d7..0a2072aa 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -19,6 +19,7 @@ class Ha: self.dcs = patroni.dcs self.cluster = None self.old_cluster = None + self.recovering = False self._async_executor = AsyncExecutor() def load_cluster_from_dcs(self): @@ -94,33 +95,21 @@ class Ha: return 'waiting for leader to bootstrap' def recover(self): - has_lock = self.has_lock() - # 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. pg_controldata = self.state_handler.controldata() - if not has_lock and pg_controldata and\ + if (self.state_handler.role == 'master') and pg_controldata and\ pg_controldata.get('Database cluster state', '') == 'in production': # crashed master self.state_handler.require_rewind() + self.recovering = True + return self.follow_the_leader("started as readonly because i had the session lock", + "started as a secondary", + refresh=True, recovery=True) - # XXX: follow the leader calls stop, which might take quite some time. - # perhaps we should run sync asynchronously - # (we still need the exit code from follow_the_leader) - ret = self.state_handler.follow_the_leader(None if has_lock else self.cluster.leader, recovery=True) - if not ret: - if not has_lock: - return 'failed to start postgres' - self.dcs.delete_leader() - self.dcs.reset_cluster() - return 'removed leader key after trying and failing to start postgres' - if not has_lock: - return 'started as a secondary' - logger.info('started as readonly because i had the session lock') - self.load_cluster_from_dcs() - - def follow_the_leader(self, demote_reason, follow_reason, refresh=True): + def follow_the_leader(self, demote_reason, follow_reason, refresh=True, recovery=False): refresh and self.load_cluster_from_dcs() - ret = demote_reason if self.state_handler.is_leader() else follow_reason + ret = demote_reason if (not recovery and self.state_handler.is_leader() + or recovery and self.state_handler.role == 'master') else follow_reason leader = self.cluster.leader leader = None if (leader and leader.name) == self.state_handler.name else leader if not self.state_handler.check_recovery_conf(leader): @@ -382,6 +371,15 @@ class Ha: # so even 1 << 32 would have 10 digits. return str(sysid) and len(str(sysid)) >= 10 and str(sysid).isdigit() + def post_recover(self): + if not self.state_handler.is_running(): + if self.has_lock(): + self.dcs.delete_leader() + self.dcs.reset_cluster() + return 'removed leader key after trying and failing to start postgres' + return 'failed to start postgres' + return None + def _run_cycle(self): try: self.load_cluster_from_dcs() @@ -395,6 +393,13 @@ class Ha: if self._async_executor.busy: return self.handle_long_action_in_progress() + # we've go here, so async action has finished. Check if we tried to recover and failed + if self.recovering: + self.recovering = False + msg = self.post_recover() + if msg is not None: + return msg + # currently it can trigger only reinitialize msg = self.process_scheduled_action() if msg is not None: diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 45b871c2..84223675 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -596,9 +596,6 @@ recovery_target_timeline = 'latest' self.call_nowait(ACTION_ON_ROLE_CHANGE) return ret - def demote(self): - self.follow_the_leader(None) - def create_or_update_role(self, name, password, options): self.query("""DO $$ BEGIN diff --git a/tests/test_ha.py b/tests/test_ha.py index d9a408a4..bcc5c93d 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -127,13 +127,18 @@ class TestHa(unittest.TestCase): def test_recover_replica_failed(self): self.p.controldata = lambda: {'Database cluster state': 'in production'} self.p.is_healthy = false + self.p.is_running = false self.p.follow_the_leader = false + self.assertEquals(self.ha.run_cycle(), 'started as a secondary') self.assertEquals(self.ha.run_cycle(), 'failed to start postgres') def test_recover_master_failed(self): self.p.follow_the_leader = false self.p.is_healthy = false + self.p.is_running = false self.ha.has_lock = true + self.p.role = 'master' + self.assertEquals(self.ha.run_cycle(), 'started as readonly because i had the session lock') self.assertEquals(self.ha.run_cycle(), 'removed leader key after trying and failing to start postgres') @patch('sys.exit', return_value=1) @@ -144,7 +149,8 @@ class TestHa(unittest.TestCase): @patch.object(Cluster, 'is_unlocked', Mock(return_value=False)) def test_start_as_readonly(self): - self.p.is_leader = self.p.is_healthy = false + self.p.is_leader = false + self.p.is_healthy = true self.ha.has_lock = true self.assertEquals(self.ha.run_cycle(), 'promoted self to leader because i had the session lock') diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 2097bcb7..a4dae962 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -243,9 +243,7 @@ class TestPostgresql(unittest.TestCase): @patch('patroni.postgresql.Postgresql.single_user_mode', MagicMock(return_value=1)) @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict())) def test_follow_the_leader(self, mock_pg_rewind): - self.p.demote() self.p.follow_the_leader(None) - self.p.demote() self.p.follow_the_leader(self.leader) self.p.follow_the_leader(Leader(-1, 28, self.other)) self.p.rewind = mock_pg_rewind