From 9edbe7e3f701ad848624a0448f2743b0563f4d08 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 29 Mar 2021 08:06:12 +0200 Subject: [PATCH] Fix little issues with custom bootstrap (#1891) 1. Set hot_standby=off only when we do PITR 2. Restart postgres after PITR is done to avoid warnings 3. Address invalid config issue https://github.com/zalando/patroni/issues/1870#issuecomment-800088643 --- patroni/postgresql/bootstrap.py | 4 ++-- patroni/postgresql/config.py | 22 ++++++++++++---------- tests/test_postgresql.py | 3 +++ 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/patroni/postgresql/bootstrap.py b/patroni/postgresql/bootstrap.py index ca72fc59..2c328718 100644 --- a/patroni/postgresql/bootstrap.py +++ b/patroni/postgresql/bootstrap.py @@ -90,8 +90,8 @@ class Bootstrap(object): self._postgresql.configure_server_parameters() # make sure there is no trigger file or postgres will be automatically promoted - trigger_file = 'promote_trigger_file' if self._postgresql.major_version >= 120000 else 'trigger_file' - trigger_file = self._postgresql.config.get('recovery_conf', {}).get(trigger_file) or 'promote' + trigger_file = self._postgresql.config.triggerfile_good_name + trigger_file = (self._postgresql.config.get('recovery_conf') or {}).get(trigger_file) or 'promote' trigger_file = os.path.abspath(os.path.join(self._postgresql.data_dir, trigger_file)) if os.path.exists(trigger_file): os.unlink(trigger_file) diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py index 7ad794ef..b1f2e69c 100644 --- a/patroni/postgresql/config.py +++ b/patroni/postgresql/config.py @@ -388,12 +388,6 @@ class ConfigHandler(object): os.rename(self._postgresql_conf, self._postgresql_base_conf) configuration = configuration or self._server_parameters.copy() - - # In case we are using custom bootstrap from spilo image with PITR it fails if it contains increasing - # values like Max_connections. We disable hot_standby so it will accept increasing values. - if self._postgresql.bootstrap.running_custom_bootstrap: - configuration['hot_standby'] = 'off' - # Due to the permanent logical replication slots configured we have to enable hot_standby_feedback if self._postgresql.enforce_hot_standby_feedback: configuration['hot_standby_feedback'] = 'on' @@ -559,7 +553,7 @@ class ConfigHandler(object): return os.path.exists(self._recovery_conf) @property - def _triggerfile_good_name(self): + def triggerfile_good_name(self): return 'trigger_file' if self._postgresql.major_version < 120000 else 'promote_trigger_file' @property @@ -762,13 +756,13 @@ class ConfigHandler(object): return env def write_recovery_conf(self, recovery_params): + self._recovery_params = recovery_params if self._postgresql.major_version >= 120000: if parse_bool(recovery_params.pop('standby_mode', None)): open(self._standby_signal, 'w').close() else: self._remove_file_if_exists(self._standby_signal) open(self._recovery_signal, 'w').close() - self._recovery_params = recovery_params else: with ConfigWriter(self._recovery_conf) as f: os.chmod(self._recovery_conf, stat.S_IWRITE | stat.S_IREAD) @@ -812,8 +806,8 @@ class ConfigHandler(object): if self.get('recovery_conf'): value = self._config['recovery_conf'].pop(self._triggerfile_wrong_name, None) - if self._triggerfile_good_name not in self._config['recovery_conf'] and value: - self._config['recovery_conf'][self._triggerfile_good_name] = value + if self.triggerfile_good_name not in self._config['recovery_conf'] and value: + self._config['recovery_conf'][self.triggerfile_good_name] = value def get_server_parameters(self, config): parameters = config['parameters'].copy() @@ -1073,6 +1067,14 @@ class ConfigHandler(object): if cvalue > value: 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) + return effective_configuration @property diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 479554fe..2aa28383 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -11,6 +11,7 @@ from patroni.async_executor import CriticalTask from patroni.dcs import Cluster, RemoteMember, SyncState from patroni.exceptions import PostgresConnectionException, PatroniException from patroni.postgresql import Postgresql, STATE_REJECT, STATE_NO_RESPONSE +from patroni.postgresql.bootstrap import Bootstrap from patroni.postgresql.postmaster import PostmasterProcess from patroni.utils import RetryFailedError from six.moves import builtins @@ -684,6 +685,8 @@ class TestPostgresql(BaseTestPostgresql): self.assertEqual(self.p.get_master_timeline(), 1) @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',