diff --git a/features/basic_replication.feature b/features/basic_replication.feature index 4a0eda68..0e2e8c4b 100644 --- a/features/basic_replication.feature +++ b/features/basic_replication.feature @@ -72,16 +72,11 @@ Feature: basic replication Then table bar is present on postgres1 after 20 seconds And Response on GET http://127.0.0.1:8010/config contains master_start_timeout after 10 seconds - Scenario: check immediate failover when master_start_timeout=0 - Given I kill postmaster on postgres2 - Then postgres1 is a leader after 10 seconds - And postgres1 role is the primary after 10 seconds - Scenario: check rejoin of the former primary with pg_rewind Given I add the table splitbrain to postgres0 And I start postgres0 Then postgres0 role is the secondary after 20 seconds - When I add the table buz to postgres1 + When I add the table buz to postgres2 Then table buz is present on postgres0 after 20 seconds Scenario: check graceful rejection when two nodes have the same name diff --git a/features/recovery.feature b/features/recovery.feature new file mode 100644 index 00000000..809f7fb9 --- /dev/null +++ b/features/recovery.feature @@ -0,0 +1,24 @@ +Feature: recovery + We want to check that crashed postgres is started back + + Scenario: check that timeline is not incremented when primary is started after crash + Given I start postgres0 + Then postgres0 is a leader after 10 seconds + And there is a non empty initialize key in DCS after 15 seconds + When I start postgres1 + And I add the table foo to postgres0 + Then table foo is present on postgres1 after 20 seconds + When I kill postmaster on postgres0 + Then postgres0 role is the primary after 10 seconds + When I issue a GET request to http://127.0.0.1:8008/ + Then I receive a response code 200 + And I receive a response role master + And I receive a response timeline 1 + + Scenario: check immediate failover when master_start_timeout=0 + Given I issue a PATCH request to http://127.0.0.1:8008/config with {"master_start_timeout": 0} + Then I receive a response code 200 + And Response on GET http://127.0.0.1:8008/config contains master_start_timeout after 10 seconds + When I kill postmaster on postgres0 + Then postgres1 is a leader after 10 seconds + And postgres1 role is the primary after 10 seconds diff --git a/patroni/ha.py b/patroni/ha.py index 3108f787..a85dd690 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -149,7 +149,6 @@ class Ha(object): self._leader_timeline = None self.recovering = False self._async_response = CriticalTask() - self._crash_recovery_executed = False self._crash_recovery_started = 0 self._start_timeout = None self._async_executor = AsyncExecutor(self.state_handler.cancellable, self.wakeup) @@ -411,8 +410,7 @@ class Ha(object): return result def _handle_crash_recovery(self) -> Optional[str]: - if not self._crash_recovery_executed and (self.cluster.is_unlocked() or self._rewind.can_rewind): - self._crash_recovery_executed = True + if self._crash_recovery_started == 0 and (self.cluster.is_unlocked() or self._rewind.can_rewind): 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 @@ -438,15 +436,29 @@ class Ha(object): return self._async_executor.try_run_async(msg, self._do_reinitialize, args=(self.cluster,)) or msg def recover(self) -> str: - # Postgres is not running and we will restart in standby mode. Watchdog is not needed until we promote. - self.watchdog.disable() + """Handle the case when postgres isn't running. + Depending on the state of Patroni, DCS cluster view, and pg_controldata the following could happen: + - if ``primary_start_timeout`` is 0 and this node owns the leader lock, the lock + will be voluntarily released if there are healthy replicas to take it over. + - if postgres was running as a ``primary`` and this node owns the leader lock, postgres is started as primary. + - crash recover in a single-user mode is executed in the following cases: + - postgres was running as ``primary`` wasn't ``shut down`` cleanly and there is no leader in DCS + - postgres was running as ``replica`` wasn't ``shut down in recovery`` (cleanly) + and we need to run ``pg_rewind`` to join back to the cluster. + - ``pg_rewind`` is executed if it is necessary, or optinally, the data directory could + be removed if it is allowed by configuration. + - after ``crash recovery`` and/or ``pg_rewind`` are executed, postgres is started in recovery. + + :returns: action message, describing what was performed. + """ if self.has_lock() and self.update_lock(): timeout = self.global_config.primary_start_timeout if timeout == 0: # We are requested to prefer failing over to restarting primary. But see first if there # is anyone to fail over to. if self.is_failover_possible(self.cluster.members): + self.watchdog.disable() logger.info("Primary crashed. Failing over.") self.demote('immediate') return 'stopped PostgreSQL to fail over after a crash' @@ -455,6 +467,23 @@ 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())) + + # timeout > 0 indicates that we still have the leader lock, and it was just updated + if timeout\ + and data.get('Database cluster state') in ('in production', 'shutting down', 'shut down')\ + and self.state_handler.state == 'crashed'\ + and self.state_handler.role in ('primary', 'master')\ + and not self.state_handler.config.recovery_conf_exists(): + # We know 100% that we were running as a primary a few moments ago, therefore could just start postgres + msg = 'starting primary after failure' + if self._async_executor.try_run_async(msg, self.state_handler.start, + args=(timeout, self._async_executor.critical_task)) is None: + self.recovering = True + return msg + + # Postgres is not running, and we will restart in standby mode. Watchdog is not needed until we promote. + self.watchdog.disable() + if data.get('Database cluster state') in ('in production', 'shutting down', 'in crash recovery'): msg = self._handle_crash_recovery() if msg: @@ -965,9 +994,6 @@ class Ha(object): if ret is not None: # continue if we just deleted the stale failover key as a leader return ret - if self.state_handler.is_starting(): # postgresql still starting up is unhealthy - return False - if self.state_handler.is_leader(): # in pause leader is the healthiest only when no initialize or sysid matches with initialize! return not self.is_paused() or not self.cluster.initialize\ @@ -1451,7 +1477,7 @@ class Ha(object): if self.state_handler.role in ('master', 'primary'): logger.info('Demoting primary during %s', self._async_executor.scheduled_action) - if self._async_executor.scheduled_action == 'restart': + if self._async_executor.scheduled_action in ('restart', 'starting primary after failure'): # Restart needs a special interlocking cancel because postmaster may be just started in a # background thread and has not even written a pid file yet. with self._async_executor.critical_task as task: @@ -1616,7 +1642,7 @@ class Ha(object): return msg # Reset some states after postgres successfully started up - self._crash_recovery_executed = False + self._crash_recovery_started = 0 if self._rewind.executed and not self._rewind.failed: self._rewind.reset_state() diff --git a/tests/test_ha.py b/tests/test_ha.py index 8495a1f0..cfcb51c4 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -282,11 +282,20 @@ class TestHa(PostgresInit): self.p.follow = false self.p.is_running = false self.p.name = 'leader' - self.p.set_role('primary') + self.p.set_role('demoted') self.p.controldata = lambda: {'Database cluster state': 'shut down', 'Database system identifier': SYSID} self.ha.cluster = get_cluster_initialized_with_leader() self.assertEqual(self.ha.run_cycle(), 'starting as readonly because i had the session lock') + def test_start_primary_after_failure(self): + self.p.start = false + self.p.is_running = false + self.p.name = 'leader' + self.p.set_role('primary') + self.p.controldata = lambda: {'Database cluster state': 'in production', 'Database system identifier': SYSID} + self.ha.cluster = get_cluster_initialized_with_leader() + self.assertEqual(self.ha.run_cycle(), 'starting primary after failure') + @patch.object(Rewind, 'ensure_clean_shutdown', Mock()) def test_crash_recovery(self): self.ha.has_lock = true @@ -837,8 +846,6 @@ class TestHa(PostgresInit): self.ha.dcs._last_failsafe = None with patch.object(Watchdog, 'is_healthy', PropertyMock(return_value=False)): self.assertFalse(self.ha.is_healthiest_node()) - with patch('patroni.postgresql.Postgresql.is_starting', return_value=True): - self.assertFalse(self.ha.is_healthiest_node()) self.ha.is_paused = true self.assertFalse(self.ha.is_healthiest_node())