From c985974ece7d73444f7e97c15ae6beb607162772 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Tue, 28 Feb 2023 10:08:42 +0100 Subject: [PATCH] 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 --- patroni/postgresql/config.py | 22 ++++++++++++++++------ tests/test_postgresql.py | 22 ++++++++++++++-------- 2 files changed, 30 insertions(+), 14 deletions(-) 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)