From 81912c9cae71436e92902469234fd9ba764c0041 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 24 Mar 2022 13:51:59 +0100 Subject: [PATCH] Handle rewind when demoted node was shut down (#2252) In case of DCS unavailability Patroni restarts Postgres in read-only. It will cause pg_control to be updated with the `Database cluster state: in archive recovery` and also could set the `MinRecoveryPoint`. When the next time Patroni is started it will assume that Postgres was running as a replica and rewind isn't required and will try to start the Postgres up. In this situation there is the chance that the start will be aborted with the FATAL error message that looks like `requested timeline 2 does not contain minimum recovery point 0/501E8B8 on timeline 1`. On the next heart-beat Patroni will again notice that Postgres isn't running which would lead to another start-fail attempt. This loop is endless. In order to mitigate the problem we do the following: 1. While figuring out whether the rewind is required we consider `in archive recovery` along with `shut down in recovery`. 2. If pg_rewind is required and the cluster state is `in archive recovery` we also perform recovery in a single-user mode. Close https://github.com/zalando/patroni/issues/2242 --- patroni/ha.py | 24 +++++++++++++++++------- patroni/postgresql/rewind.py | 2 +- tests/test_ha.py | 12 ++++++++++++ 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 9e0f9b84..d5f47bd6 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -293,12 +293,25 @@ class Ha(object): return result + def _handle_crash_recovery(self): + if not self._crash_recovery_executed and (self.cluster.is_unlocked() or self._rewind.can_rewind): + self._crash_recovery_executed = True + self._crash_recovery_started = time.time() + msg = 'doing crash recovery in a single user mode' + return self._async_executor.try_run_async(msg, self._rewind.ensure_clean_shutdown) or msg + def _handle_rewind_or_reinitialize(self): leader = self.get_remote_master() if self.is_standby_cluster() else self.cluster.leader if not self._rewind.rewind_or_reinitialize_needed_and_possible(leader): return None if self._rewind.can_rewind: + # rewind is required, but postgres wasn't shut down cleanly. + if self.state_handler.controldata().get('Database cluster state') == 'in archive recovery': + msg = self._handle_crash_recovery() + if msg: + return msg + msg = 'running pg_rewind from ' + leader.name return self._async_executor.try_run_async(msg, self._rewind.execute, args=(leader,)) or msg @@ -325,13 +338,10 @@ class Ha(object): data = self.state_handler.controldata() logger.info('pg_controldata:\n%s\n', '\n'.join(' {0}: {1}'.format(k, v) for k, v in data.items())) - if data.get('Database cluster state') in ('in production', 'shutting down', 'in crash recovery') \ - and not self._crash_recovery_executed and \ - (self.cluster.is_unlocked() or self._rewind.can_rewind): - self._crash_recovery_executed = True - self._crash_recovery_started = time.time() - msg = 'doing crash recovery in a single user mode' - return self._async_executor.try_run_async(msg, self._rewind.ensure_clean_shutdown) or msg + if data.get('Database cluster state') in ('in production', 'shutting down', 'in crash recovery'): + msg = self._handle_crash_recovery() + if msg: + return msg self.load_cluster_from_dcs() diff --git a/patroni/postgresql/rewind.py b/patroni/postgresql/rewind.py index ba6e4532..fb754f58 100644 --- a/patroni/postgresql/rewind.py +++ b/patroni/postgresql/rewind.py @@ -112,7 +112,7 @@ class Rewind(object): in_recovery = timeline = lsn = None data = self._postgresql.controldata() try: - if data.get('Database cluster state') == 'shut down in recovery': + if data.get('Database cluster state') in ('shut down in recovery', 'in archive recovery'): in_recovery = True lsn = data.get('Minimum recovery ending location') timeline = int(data.get("Min recovery ending loc's timeline")) diff --git a/tests/test_ha.py b/tests/test_ha.py index 90467b20..e2ef7b6f 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -284,6 +284,18 @@ class TestHa(PostgresInit): self.ha.patroni.config.set_dynamic_configuration({'maximum_lag_on_failover': 10}) self.assertEqual(self.ha.run_cycle(), 'terminated crash recovery because of startup timeout') + @patch.object(Rewind, 'ensure_clean_shutdown', Mock()) + @patch.object(Rewind, 'rewind_or_reinitialize_needed_and_possible', Mock(return_value=True)) + @patch.object(Rewind, 'can_rewind', PropertyMock(return_value=True)) + def test_crash_recovery_before_rewind(self): + self.p.is_leader = false + self.p.is_running = false + self.p.controldata = lambda: {'Database cluster state': 'in archive recovery', + 'Database system identifier': SYSID} + self.ha._rewind.trigger_check_diverged_lsn() + self.ha.cluster = get_cluster_initialized_with_leader() + self.assertEqual(self.ha.run_cycle(), 'doing crash recovery in a single user mode') + @patch.object(Rewind, 'rewind_or_reinitialize_needed_and_possible', Mock(return_value=True)) @patch.object(Rewind, 'can_rewind', PropertyMock(return_value=True)) def test_recover_with_rewind(self):