diff --git a/patroni/postgresql.py b/patroni/postgresql.py index dd0409e8..7d06de11 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -8,7 +8,7 @@ import tempfile import time from patroni.exceptions import PostgresConnectionException, PostgresException -from patroni.utils import compare_values, Retry, RetryFailedError +from patroni.utils import compare_values, parse_int, Retry, RetryFailedError from six import string_types from six.moves.urllib_parse import urlparse from threading import Lock @@ -138,21 +138,49 @@ class Postgresql(object): listen_address_changed = pending_reload = False if self.is_healthy(): - changes = server_parameters.copy() - changes.update({p: None for p, v in self._server_parameters.items() if p not in server_parameters}) + changes = {p: v for p, v in server_parameters.items() if '.' not in p} + changes.update({p: None for p, v in self._server_parameters.items() if not ('.' in p or p in changes)}) if changes: + if 'wal_segment_size' not in changes: + changes['wal_segment_size'] = '16384kB' + # XXX: query can raise an exception for r in self.query("""SELECT name, setting, unit, vartype, context FROM pg_settings - WHERE name IN (""" + ', '.join('%s' for _ in changes.keys()) + ')', - *(list(changes.keys()))): - unit = '16384kB' if r[0] in ('min_wal_size', 'max_wal_size') else r[2] - if server_parameters[r[0]] is None or not compare_values(r[3], unit, r[1], server_parameters[r[0]]): - if r[4] == 'postmaster': - self._pending_restart = True - if r[0] in ('listen_addresses', 'port'): - listen_address_changed = True - elif r[4] != 'internal': + WHERE name IN (""" + ', '.join(['%s'] * len(changes)) + """) + ORDER BY 1 DESC""", *(list(changes.keys()))): + if r[4] == 'internal': + if r[0] == 'wal_segment_size': + server_parameters.pop(r[0], None) + wal_segment_size = parse_int(r[2], 'kB') + if wal_segment_size is not None: + changes['wal_segment_size'] = '{0}kB'.format(int(r[1]) * wal_segment_size) + elif r[0] in changes: + unit = changes['wal_segment_size'] if r[0] in ('min_wal_size', 'max_wal_size') else r[2] + new_value = changes.pop(r[0]) + if new_value is None or not compare_values(r[3], unit, r[1], new_value): + if r[4] == 'postmaster': + self._pending_restart = True + if r[0] in ('listen_addresses', 'port'): + listen_address_changed = True + else: + pending_reload = True + for param, value in changes.items(): + 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 pending_reload: + 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])): + pending_reload = True + break + if not pending_reload: + for p, v in self._server_parameters.items(): + if '.' in p and (p not in server_parameters or str(v) != str(server_parameters[p])): pending_reload = True + break + self.config = config self._server_parameters = server_parameters self._connect_address = config.get('connect_address') diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 3107ed9b..92d2a3e8 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -35,7 +35,10 @@ class MockCursor(object): elif sql.startswith('SELECT to_char(pg_postmaster_start_time'): self.results = [('', True, '', '', '', '', False)] elif sql.startswith('SELECT name, setting'): - self.results = [('port', '5433', None, 'integer', 'postmaster'), + self.results = [('wal_segment_size', '2048', '8kB', 'integer', 'internal'), + ('search_path', 'public', None, 'string', 'user'), + ('port', '5433', None, 'integer', 'postmaster'), + ('listen_addresses', '*', None, 'string', 'postmaster'), ('autovacuum', 'on', None, 'bool', 'sighup')] else: self.results = [(None, None, None, None, None, None, None, None, None, None)] @@ -150,7 +153,7 @@ def fake_listdir(path): @patch('subprocess.call', Mock(return_value=0)) @patch('psycopg2.connect', psycopg2_connect) class TestPostgresql(unittest.TestCase): - _PARAMETERS = {'wal_level': 'hot_standby', 'max_replication_slots': 5, 'foo': 'bar', 'config_file': None, + _PARAMETERS = {'wal_level': 'hot_standby', 'max_replication_slots': 5, 'f.oo': 'bar', 'search_path': 'public', 'hot_standby': 'on', 'max_wal_senders': 5, 'wal_keep_segments': 8, 'wal_log_hints': 'on'} @patch('subprocess.call', Mock(return_value=0)) @@ -203,7 +206,7 @@ class TestPostgresql(unittest.TestCase): self.assertTrue(self.p.start()) with open(pg_conf) as f: lines = f.readlines() - self.assertTrue("foo = 'bar'\n" in lines) + self.assertTrue("f.oo = 'bar'\n" in lines) def test_stop(self): self.assertTrue(self.p.stop()) @@ -477,9 +480,14 @@ class TestPostgresql(unittest.TestCase): def test_reload_config(self): parameters = self._PARAMETERS.copy() + parameters.pop('f.oo') + self.p.reload_config({'retry_timeout': 10, 'listen': '*', 'parameters': parameters}) + parameters['b.ar'] = 'bar' + self.p.reload_config({'retry_timeout': 10, 'listen': '*', 'parameters': parameters}) parameters['autovacuum'] = 'on' self.p.reload_config({'retry_timeout': 10, 'listen': '*', 'parameters': parameters}) parameters['autovacuum'] = 'off' + parameters.pop('search_path') self.p.reload_config({'retry_timeout': 10, 'listen': '*:5433', 'parameters': parameters}) @patch.object(builtins, 'open', mock_open(read_data='9.4'))