diff --git a/patroni/postgresql/config.py b/patroni/postgresql/config.py index 9c43b4c6..beabdebd 100644 --- a/patroni/postgresql/config.py +++ b/patroni/postgresql/config.py @@ -1076,13 +1076,14 @@ class ConfigHandler(object): def reload_config(self, config: Dict[str, Any], sighup: bool = False) -> None: self._superuser = config['authentication'].get('superuser', {}) server_parameters = self.get_server_parameters(config) + params_skip_changes = CaseInsensitiveSet((*self._RECOVERY_PARAMETERS, 'hot_standby', 'wal_log_hints')) conf_changed = hba_changed = ident_changed = local_connection_address_changed = pending_restart = False if self._postgresql.state == 'running': changes = CaseInsensitiveDict({p: v for p, v in server_parameters.items() - if p.lower() not in self._RECOVERY_PARAMETERS}) + if p not in params_skip_changes}) changes.update({p: None for p in self._server_parameters.keys() - if not (p in changes or p.lower() in self._RECOVERY_PARAMETERS)}) + if not (p in changes or p in params_skip_changes)}) if changes: undef = [] if 'wal_buffers' in changes: # we need to calculate the default value of wal_buffers @@ -1168,7 +1169,7 @@ class ConfigHandler(object): pending_restart = self._postgresql.query( 'SELECT COUNT(*) FROM pg_catalog.pg_settings' ' WHERE pg_catalog.lower(name) != ALL(%s) AND pending_restart', - [n.lower() for n in self._RECOVERY_PARAMETERS])[0][0] > 0 + [n.lower() for n in params_skip_changes])[0][0] > 0 self._postgresql.set_pending_restart(pending_restart) except Exception as e: logger.warning('Exception %r when running query', e) @@ -1243,7 +1244,6 @@ class ConfigHandler(object): if disable_hot_standby: effective_configuration['hot_standby'] = 'off' - self._postgresql.set_pending_restart(True) return effective_configuration diff --git a/tests/__init__.py b/tests/__init__.py index 33f3fa6c..0b83a4df 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -54,10 +54,10 @@ GET_PG_SETTINGS_RESULT = [ ('zero_damaged_pages', 'off', None, 'bool', 'superuser'), ('stats_temp_directory', '/tmp', None, 'string', 'sighup'), ('track_commit_timestamp', 'off', None, 'bool', 'postmaster'), - ('wal_log_hints', 'on', None, 'bool', 'superuser'), - ('hot_standby', 'on', None, 'bool', 'superuser'), - ('max_replication_slots', '5', None, 'integer', 'superuser'), - ('wal_level', 'logical', None, 'enum', 'superuser'), + ('wal_log_hints', 'on', None, 'bool', 'postmaster'), + ('hot_standby', 'on', None, 'bool', 'postmaster'), + ('max_replication_slots', '5', None, 'integer', 'postmaster'), + ('wal_level', 'logical', None, 'enum', 'postmaster'), ] diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index ddb57b4d..bdabd93c 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -575,6 +575,14 @@ class TestPostgresql(BaseTestPostgresql): mock_info.reset_mock() + # Ignored params changed + config['parameters']['archive_cleanup_command'] = 'blabla' + self.p.reload_config(config) + mock_info.assert_called_once_with('No PostgreSQL configuration items changed, nothing to reload.') + self.assertEqual(self.p.pending_restart, False) + + mock_info.reset_mock() + # Handle wal_buffers self.p.config._config['parameters']['wal_buffers'] = '512' self.p.reload_config(config) @@ -801,21 +809,28 @@ class TestPostgresql(BaseTestPostgresql): @patch.object(Postgresql, 'get_postgres_role_from_data_directory', Mock(return_value='replica')) @patch.object(Postgresql, 'is_running', Mock(return_value=False)) @patch.object(Bootstrap, 'running_custom_bootstrap', PropertyMock(return_value=True)) - @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') + @patch('patroni.postgresql.config.logger') 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]) + controldata = {'max_connections setting': '100', 'max_worker_processes setting': '8', + 'max_locks_per_xact setting': '64', 'max_wal_senders setting': 5} - self.assertTrue(self.p.pending_restart) - with patch.object(Bootstrap, 'keep_existing_recovery_conf', PropertyMock(return_value=True)): + with patch.object(Postgresql, 'controldata', Mock(return_value=controldata)), \ + patch.object(Bootstrap, 'keep_existing_recovery_conf', PropertyMock(return_value=True)): + self.p.cancellable.cancel() self.assertFalse(self.p.start()) + self.assertFalse(self.p.pending_restart) + mock_logger.warning.assert_called_once() + self.assertEqual(mock_logger.warning.call_args[0], + ('%s is missing from pg_controldata output', 'max_prepared_xacts setting')) + + mock_logger.reset_mock() + controldata['max_prepared_xacts setting'] = 0 + controldata['max_wal_senders setting'] *= 2 + + with patch.object(Postgresql, 'controldata', Mock(return_value=controldata)): + self.p.config.write_recovery_conf({'pause_at_recovery_target': 'false'}) + self.assertFalse(self.p.start()) + mock_logger.warning.assert_not_called() self.assertTrue(self.p.pending_restart) @patch('os.path.exists', Mock(return_value=True))