diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py index 27fd4cf6..c995f177 100644 --- a/patroni/postgresql/config.py +++ b/patroni/postgresql/config.py @@ -1094,12 +1094,22 @@ class ConfigHandler(object): effective_configuration[name] = cvalue self._postgresql.set_pending_restart(True) - # If we are using custom bootstrap with PITR it could fail when values - # like max_connections are increased, therefore we disable hot_standby. - if self._postgresql.bootstrap.running_custom_bootstrap and \ - (self._postgresql.bootstrap.keep_existing_recovery_conf or self._recovery_conf): - effective_configuration['hot_standby'] = 'off' - self._postgresql.set_pending_restart(True) + # If we are using custom bootstrap with PITR it could fail when values like max_connections + # are increased, therefore we disable hot_standby if recovery_target_action == 'promote'. + if self._postgresql.bootstrap.running_custom_bootstrap: + disable_hot_standby = False + if self._postgresql.bootstrap.keep_existing_recovery_conf: + disable_hot_standby = True # trust that pgBackRest does the right thing + # `pause_at_recovery_target` has no effect if hot_standby is not enabled, therefore we consider only 9.5+ + elif self._postgresql.major_version >= 90500 and self._recovery_params: + pause_at_recovery_target = parse_bool(self._recovery_params.get('pause_at_recovery_target')) + recovery_target_action = self._recovery_params.get( + 'recovery_target_action', 'promote' if pause_at_recovery_target is False else 'pause') + disable_hot_standby = recovery_target_action == 'promote' + + if disable_hot_standby: + effective_configuration['hot_standby'] = 'off' + self._postgresql.set_pending_restart(True) return effective_configuration diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 40411208..20777a80 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -675,14 +675,20 @@ class TestPostgresql(BaseTestPostgresql): @patch.object(Postgresql, 'get_postgres_role_from_data_directory', Mock(return_value='replica')) @patch.object(Bootstrap, 'running_custom_bootstrap', PropertyMock(return_value=True)) - @patch.object(Bootstrap, 'keep_existing_recovery_conf', PropertyMock(return_value=True)) - def test__build_effective_configuration(self): - with patch.object(Postgresql, 'controldata', - Mock(return_value={'max_connections setting': '200', - 'max_worker_processes setting': '20', - 'max_locks_per_xact setting': '100', - 'max_wal_senders setting': 10})): - self.p.cancellable.cancel() + @patch.object(Postgresql, 'controldata', Mock(return_value={'max_connections setting': '200', + 'max_worker_processes setting': '20', + 'max_locks_per_xact setting': '100', + 'max_wal_senders setting': 10})) + @patch('patroni.postgresql.config.logger.warning') + def test_effective_configuration(self, mock_logger): + self.p.cancellable.cancel() + self.p.config.write_recovery_conf({'pause_at_recovery_target': 'false'}) + self.assertFalse(self.p.start()) + mock_logger.assert_called_once() + self.assertTrue('is missing from pg_controldata output' in mock_logger.call_args[0][0]) + + self.assertTrue(self.p.pending_restart) + with patch.object(Bootstrap, 'keep_existing_recovery_conf', PropertyMock(return_value=True)): self.assertFalse(self.p.start()) self.assertTrue(self.p.pending_restart)