diff --git a/patroni/ha.py b/patroni/ha.py index c0821bc2..9f84613f 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -316,8 +316,7 @@ class Ha(object): msg = 'running pg_rewind from ' + leader.name return self._async_executor.try_run_async(msg, self._rewind.execute, args=(leader,)) or msg - # remove_data_directory_on_diverged_timelines is set - if not self.is_standby_cluster(): + if self._rewind.should_remove_data_directory_on_diverged_timelines and not self.is_standby_cluster(): msg = 'reinitializing due to diverged timelines' return self._async_executor.try_run_async(msg, self._do_reinitialize, args=(self.cluster,)) or msg diff --git a/patroni/postgresql/rewind.py b/patroni/postgresql/rewind.py index 5a2e4a39..db7055df 100644 --- a/patroni/postgresql/rewind.py +++ b/patroni/postgresql/rewind.py @@ -46,9 +46,13 @@ class Rewind(object): return False return self.configuration_allows_rewind(self._postgresql.controldata()) + @property + def should_remove_data_directory_on_diverged_timelines(self): + return self._postgresql.config.get('remove_data_directory_on_diverged_timelines') + @property def can_rewind_or_reinitialize_allowed(self): - return self._postgresql.config.get('remove_data_directory_on_diverged_timelines') or self.can_rewind + return self.should_remove_data_directory_on_diverged_timelines or self.can_rewind def trigger_check_diverged_lsn(self): if self.can_rewind_or_reinitialize_allowed and self._state != REWIND_STATUS.NEED: @@ -374,19 +378,22 @@ class Rewind(object): if self.pg_rewind(r): self._state = REWIND_STATUS.SUCCESS - elif not self.check_leader_is_not_in_recovery(r): - logger.warning('Failed to rewind because master %s become unreachable', leader.name) else: - logger.error('Failed to rewind from healty master: %s', leader.name) - - for name in ('remove_data_directory_on_rewind_failure', 'remove_data_directory_on_diverged_timelines'): - if self._postgresql.config.get(name): - logger.warning('%s is set. removing...', name) - self._postgresql.remove_data_directory() - self._state = REWIND_STATUS.INITIAL - break + if not self.check_leader_is_not_in_recovery(r): + logger.warning('Failed to rewind because master %s become unreachable', leader.name) + if not self.can_rewind: # It is possible that the previous attempt damaged pg_control file! + self._state = REWIND_STATUS.FAILED else: + logger.error('Failed to rewind from healty master: %s', leader.name) self._state = REWIND_STATUS.FAILED + + if self.failed: + for name in ('remove_data_directory_on_rewind_failure', 'remove_data_directory_on_diverged_timelines'): + if self._postgresql.config.get(name): + logger.warning('%s is set. removing...', name) + self._postgresql.remove_data_directory() + self._state = REWIND_STATUS.INITIAL + break return False def reset_state(self): diff --git a/tests/test_ha.py b/tests/test_ha.py index 56277f97..5ed4503d 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -314,6 +314,7 @@ class TestHa(PostgresInit): self.assertEqual(self.ha.run_cycle(), 'fake') @patch.object(Rewind, 'rewind_or_reinitialize_needed_and_possible', Mock(return_value=True)) + @patch.object(Rewind, 'should_remove_data_directory_on_diverged_timelines', PropertyMock(return_value=True)) @patch.object(Bootstrap, 'create_replica', Mock(return_value=1)) def test_recover_with_reinitialize(self): self.p.is_running = false