gitops/patroni
1
0
Fork 0
mirror of https://github.com/outbackdingo/patroni.git synced 2026-08-26 15:40:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

Perform pg_ctl reload regardless of config changes (#1204)

Browse Source 
It is possible that some config files are not controlled by Patroni and when somebody is doing reload via REST API or by sending SIGHUP to Patroni process the usual expectation is that postgres will also be reloaded, but it didn't happen when there were no changes in the postgresql section of Patroni config.

For example one might replace ssl_cert_file and ssl_key_file on the filesystem and starting from PostgreSQL 10 it just requires a reload, but Patroni wasn't doing it.

In addition to that fix the issue with handling of `wal_buffers`. The default value depends on `shared_buffers` and `wal_segment_size` and therefore Patroni was exposing pending_restart when the new value in the config was explicitly set to -1 (default).

Close https://github.com/zalando/patroni/issues/1198
This commit is contained in:
Alexander Kukushkin
2019-10-10 14:49:30 +02:00
committed by GitHub
parent 1572c02ced
commit 3d29cb7e50
11 changed files with 80 additions and 58 deletions
Download Patch File Download Diff File Expand all files Collapse all files
features/patroni_api.feature
+1 -4
View File
@@ -28,10 +28,7 @@ Scenario: check API requests on a stand-alone server
And I receive a response text "Failover could be performed only to a specific candidate"
Scenario: check local configuration reload
Given I issue an empty POST request to http://127.0.0.1:8008/reload
Then I receive a response code 200
And I receive a response text nothing changed
When I add tag new_tag new_value to postgres0 config
Given I add tag new_tag new_value to postgres0 config
And I issue an empty POST request to http://127.0.0.1:8008/reload
Then I receive a response code 202
patroni/__init__.py
+6 -4
View File
@@ -66,14 +66,14 @@ class Patroni(object):
def nosync(self):
return bool(self.tags.get('nosync', False))
def reload_config(self):
def reload_config(self, sighup=False):
try:
self.tags = self.get_tags()
self.logger.reload_config(self.config.get('log', {}))
self.dcs.reload_config(self.config)
self.watchdog.reload_config(self.config)
self.api.reload_config(self.config['restapi'])
self.postgresql.reload_config(self.config['postgresql'])
self.postgresql.reload_config(self.config['postgresql'], sighup)
self.dcs.reload_config(self.config)
except Exception:
logger.exception('Failed to reload config_file=%s', self.config.config_file)
@@ -121,7 +121,9 @@ class Patroni(object):
if self._received_sighup:
self._received_sighup = False
if self.config.reload_local_configuration():
self.reload_config()
self.reload_config(True)
else:
self.postgresql.config.reload_config(self.config['postgresql'], True)
logger.info(self.ha.run_cycle())
patroni/api.py
+2 -12
View File
@@ -187,18 +187,8 @@ class RestApiHandler(BaseHTTPRequestHandler):
@check_auth
def do_POST_reload(self):
try:
if self.server.patroni.config.reload_local_configuration(True):
status_code = 202
response = 'reload scheduled'
self.server.patroni.sighup_handler()
else:
status_code = 200
response = 'nothing changed'
except Exception as e:
status_code = 500
response = str(e)
self._write_response(status_code, response)
self.server.patroni.sighup_handler()
self._write_response(202, 'reload scheduled')
@staticmethod
def parse_schedule(schedule, action):
patroni/config.py
+2 -6
View File
@@ -168,23 +168,19 @@ class Config(object):
except Exception:
logger.exception('Exception when setting dynamic_configuration')
def reload_local_configuration(self, dry_run=False):
def reload_local_configuration(self):
if self.config_file:
try:
configuration = self._load_config_file()
if not deep_compare(self._local_configuration, configuration):
new_configuration = self._build_effective_configuration(self._dynamic_configuration, configuration)
if dry_run:
return not deep_compare(new_configuration, self.__effective_configuration)
self._local_configuration = configuration
self.__effective_configuration = new_configuration
return True
else:
logger.info('No configuration items changed, nothing to reload.')
logger.info('No local configuration items changed.')
except Exception:
logger.exception('Exception when reloading local configuration from %s', self.config_file)
if dry_run:
raise
@staticmethod
def _process_postgresql_parameters(parameters, is_local=False):
patroni/postgresql/__init__.py
+2 -2
View File
@@ -187,8 +187,8 @@ class Postgresql(object):
3: STATE_UNKNOWN}
return return_codes.get(ret, STATE_UNKNOWN)
def reload_config(self, config):
self.config.reload_config(config)
def reload_config(self, config, sighup=False):
self.config.reload_config(config, sighup)
self._is_leader_retry.deadline = self.retry.deadline = config['retry_timeout']/2.0
@property
patroni/postgresql/config.py
+53 -23
View File
@@ -4,6 +4,7 @@ import re
import shutil
import socket
import stat
import time
from requests.structures import CaseInsensitiveDict
from six.moves.urllib_parse import urlparse, parse_qsl, unquote
@@ -695,7 +696,23 @@ class ConfigHandler(object):
self._postgresql.set_connection_kwargs(self.local_connect_kwargs)
def reload_config(self, config):
@staticmethod
def _handle_wal_buffers(old_values, changes):
wal_block_size = parse_int(old_values['wal_block_size'][1])
wal_segment_size = old_values['wal_segment_size']
wal_segment_size = parse_int(wal_segment_size[1]) * parse_int(wal_segment_size[2], 'B') / wal_block_size
default_wal_buffers = min(max(parse_int(old_values['shared_buffers'][1]) / 32, 8), wal_segment_size)
wal_buffers = old_values['wal_buffers']
new_value = str(changes['wal_buffers'] or -1)
new_value = default_wal_buffers if new_value == '-1' else parse_int(new_value, wal_buffers[2])
old_value = default_wal_buffers if wal_buffers[1] == '-1' else parse_int(*wal_buffers[1:3])
if new_value == old_value:
del changes['wal_buffers']
def reload_config(self, config, sighup=False):
self._superuser = config['authentication'].get('superuser', {})
server_parameters = self.get_server_parameters(config)
@@ -704,42 +721,47 @@ class ConfigHandler(object):
changes = CaseInsensitiveDict({p: v for p, v in server_parameters.items() if '.' not in p})
changes.update({p: None for p in self._server_parameters.keys() if not ('.' in p or p in changes)})
if changes:
if 'wal_buffers' in changes: # we need to calculate the default value of wal_buffers
undef = [p for p in ('shared_buffers', 'wal_segment_size', 'wal_block_size') if p not in changes]
changes.update({p: None for p in undef})
# XXX: query can raise an exception
for r in self._postgresql.query(('SELECT name, setting, unit, vartype, context '
+ 'FROM pg_catalog.pg_settings ' +
' WHERE pg_catalog.lower(name) IN ('
+ ', '.join(['%s'] * len(changes)) +
')'), *(k.lower() for k in changes.keys())):
old_values = {r[0]: r for r in self._postgresql.query(('SELECT name, setting, unit, vartype, context '
+ 'FROM pg_catalog.pg_settings ' +
' WHERE pg_catalog.lower(name) = ANY(%s)'),
[k.lower() for k in changes.keys()])}
if 'wal_buffers' in changes:
self._handle_wal_buffers(old_values, changes)
for p in undef:
del changes[p]
for r in old_values.values():
if r[4] != 'internal' and r[0] in changes:
new_value = changes.pop(r[0])
if new_value is None or not compare_values(r[3], r[2], r[1], new_value):
conf_changed = True
if r[4] == 'postmaster':
pending_restart = True
logger.info('Changed %s from %s to %s (restart required)', r[0], r[1], new_value)
logger.info('Changed %s from %s to %s (restart might be required)',
r[0], r[1], new_value)
if config.get('use_unix_socket') and r[0] == 'unix_socket_directories'\
or r[0] in ('listen_addresses', 'port'):
local_connection_address_changed = True
else:
logger.info('Changed %s from %s to %s', r[0], r[1], new_value)
conf_changed = True
for param in changes:
if param in server_parameters:
logger.warning('Removing invalid parameter `%s` from postgresql.parameters', param)
server_parameters.pop(param)
# Check that user-defined-paramters have changed (parameters with period in name)
if not conf_changed:
for p, v in server_parameters.items():
if '.' in p and (p not in self._server_parameters or str(v) != str(self._server_parameters[p])):
logger.info('Changed %s from %s to %s', p, self._server_parameters.get(p), v)
conf_changed = True
break
if not conf_changed:
for p, v in self._server_parameters.items():
if '.' in p and (p not in server_parameters or str(v) != str(server_parameters[p])):
logger.info('Changed %s from %s to %s', p, v, server_parameters.get(p))
conf_changed = True
break
for p, v in server_parameters.items():
if '.' in p and (p not in self._server_parameters or str(v) != str(self._server_parameters[p])):
logger.info('Changed %s from %s to %s', p, self._server_parameters.get(p), v)
conf_changed = True
for p, v in self._server_parameters.items():
if '.' in p and (p not in server_parameters or str(v) != str(server_parameters[p])):
logger.info('Changed %s from %s to %s', p, v, server_parameters.get(p))
conf_changed = True
if not server_parameters.get('hba_file') and config.get('pg_hba'):
hba_changed = self._config.get('pg_hba', []) != config['pg_hba']
@@ -769,10 +791,18 @@ class ConfigHandler(object):
if ident_changed:
self.replace_pg_ident()
if conf_changed or hba_changed or ident_changed:
logger.info('PostgreSQL configuration items changed, reloading configuration.')
if sighup or conf_changed or hba_changed or ident_changed:
logger.info('Reloading PostgreSQL configuration.')
self._postgresql.reload()
elif not pending_restart:
if self._postgresql.major_version >= 90500:
time.sleep(1)
try:
pending_restart = self._postgresql.query('SELECT COUNT(*) FROM pg_catalog.pg_settings'
' WHERE pending_restart').fetchone()[0] > 0
self._postgresql.set_pending_restart(pending_restart)
except Exception as e:
logger.warning('Exception %r when running query', e)
else:
logger.info('No PostgreSQL configuration items changed, nothing to reload.')
def set_synchronous_standby(self, name):
tests/__init__.py
+3
View File
@@ -106,6 +106,9 @@ class MockCursor(object):
self.results = [('', 0, '', '', '', '', False, replication_info)]
elif sql.startswith('SELECT name, setting'):
self.results = [('wal_segment_size', '2048', '8kB', 'integer', 'internal'),
('wal_block_size', '8192', None, 'integer', 'internal'),
('shared_buffers', '16384', '8kB', 'integer', 'postmaster'),
('wal_buffers', '-1', '8kB', 'integer', 'postmaster'),
('search_path', 'public', None, 'string', 'user'),
('port', '5433', None, 'integer', 'postmaster'),
('listen_addresses', '*', None, 'string', 'postmaster'),
tests/test_api.py
+1 -4
View File
@@ -233,11 +233,8 @@ class TestRestApiHandler(unittest.TestCase):
mock_dcs.get_cluster.return_value.config = ClusterConfig.from_node(1, config)
MockRestApiServer(RestApiHandler, request)
@patch.object(MockPatroni, 'sighup_handler', Mock(side_effect=Exception))
@patch.object(MockPatroni, 'sighup_handler', Mock())
def test_do_POST_reload(self):
with patch.object(MockPatroni, 'config') as mock_config:
mock_config.reload_local_configuration.return_value = False
MockRestApiServer(RestApiHandler, 'POST /reload HTTP/1.0' + self._authorization)
self.assertIsNotNone(MockRestApiServer(RestApiHandler, 'POST /reload HTTP/1.0' + self._authorization))
@patch.object(MockPatroni, 'dcs')
tests/test_config.py
+1 -2
View File
@@ -70,8 +70,7 @@ class TestConfig(unittest.TestCase):
config = Config()
with patch.object(Config, '_load_config_file', Mock(return_value={'restapi': {}})):
with patch.object(Config, '_build_effective_configuration', Mock(side_effect=Exception)):
self.assertRaises(Exception, config.reload_local_configuration, True)
self.assertTrue(config.reload_local_configuration(True))
config.reload_local_configuration()
self.assertTrue(config.reload_local_configuration())
self.assertIsNone(config.reload_local_configuration())
tests/test_patroni.py
+3
View File
@@ -125,6 +125,9 @@ class TestPatroni(unittest.TestCase):
self.p.logger.start = Mock()
self.p.config._dynamic_configuration = {}
self.assertRaises(SleepException, self.p.run)
with patch('patroni.config.Config.reload_local_configuration', Mock(return_value=False)):
self.p.sighup_handler()
self.assertRaises(SleepException, self.p.run)
with patch('patroni.config.Config.set_dynamic_configuration', Mock(return_value=True)):
self.assertRaises(SleepException, self.p.run)
with patch('patroni.postgresql.Postgresql.data_directory_empty', Mock(return_value=False)):
tests/test_postgresql.py
+6 -1
View File
@@ -424,13 +424,18 @@ class TestPostgresql(BaseTestPostgresql):
self.p.config._config['foo'] = {'command': 'bar'}
self.assertFalse(self.p.replica_method_can_work_without_replication_connection('foo'))
@patch('time.sleep', Mock())
@patch.object(Postgresql, 'is_running', Mock(return_value=True))
def test_reload_config(self):
@patch.object(MockCursor, 'fetchone')
def test_reload_config(self, mock_fetchone):
mock_fetchone.return_value = (1,)
parameters = self._PARAMETERS.copy()
parameters.pop('f.oo')
parameters['wal_buffers'] = '512'
config = {'pg_hba': [''], 'pg_ident': [''], 'use_unix_socket': True, 'authentication': {},
'retry_timeout': 10, 'listen': '*', 'krbsrvname': 'postgres', 'parameters': parameters}
self.p.reload_config(config)
mock_fetchone.side_effect = Exception
parameters['b.ar'] = 'bar'
self.p.reload_config(config)
parameters['autovacuum'] = 'on'