mirror of
https://github.com/outbackdingo/patroni.git
synced 2026-08-25 14:53:37 +00:00
Set hot_standby=off only if recovery_target_action=promote (#2570)
During custom bootstrap the `hot_standby` is set to off to protect postgres from panicking and shutting down when some parameters like `max_connections` are increased on the primary. According to the [documentation](https://www.postgresql.org/docs/current/runtime-config-wal.html#GUC-RECOVERY-TARGET-ACTION), `hot_standby` set to `off` affects behavior of the `recovery_target_action`, and `pause` starts acting as the `shutdown`: > If [hot_standby](https://www.postgresql.org/docs/current/runtime-config-replication.html#GUC-HOT-STANDBY) is not enabled, a setting of pause will act the same as shutdown This is not what users expect/need, because normally they resolve pause state on their own. To solve the problem we will set `hot_standby` to `off` during custom bootstrap only if `recovery_target_action` is set to 'promote'. Close https://github.com/zalando/patroni/issues/2569
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user