From c64170ef335025f4fd74efe6d7d6d6c7edb1cc69 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 13 Jun 2016 10:33:14 +0200 Subject: [PATCH] Extend list of postgres parameters controlled by Patroni These parameters usually must be the same across all cluster nodes and therefore must be set only via global configuration and always passed as a list of postgres arguments (via pg_ctl) to make it not possible accidentally change them by 'ALTER SYSTEM' --- patroni/config.py | 8 ++------ patroni/postgresql.py | 40 ++++++++++++++++++++++++++-------------- tests/test_postgresql.py | 6 ++++-- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/patroni/config.py b/patroni/config.py index 947eee41..377337c9 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -37,7 +37,7 @@ class Config(object): 'ttl': 30, 'loop_wait': 10, 'retry_timeout': 10, 'maximum_lag_on_failover': 1048576, 'postgresql': { - 'parameters': Postgresql.CMDLINE_OPTIONS + 'parameters': {p: v[0] for p, v in Postgresql.CMDLINE_OPTIONS.items()} } } @@ -133,11 +133,7 @@ class Config(object): def _process_postgresql_parameters(self, parameters, is_local=False): ret = {} for name, value in (parameters or {}).items(): - if (is_local and name not in self.__DEFAULT_CONFIG['postgresql']['parameters']) \ - or not ((name == 'wal_level' and value not in ('hot_standby', 'logical')) or - (name in ('max_replication_slots', 'max_wal_senders', 'wal_keep_segments') and - int(value) < self.__DEFAULT_CONFIG['postgresql']['parameters'][name]) or - name in ('hot_standby', 'wal_log_hints')): + if name not in Postgresql.CMDLINE_OPTIONS or not is_local and Postgresql.CMDLINE_OPTIONS[name][1](value): ret[name] = value return ret diff --git a/patroni/postgresql.py b/patroni/postgresql.py index f7e2e79c..50d57c3a 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, parse_int, Retry, RetryFailedError +from patroni.utils import compare_values, parse_bool, parse_int, Retry, RetryFailedError from six import string_types from six.moves.urllib_parse import urlparse from threading import Lock @@ -48,15 +48,28 @@ class Postgresql(object): # These parameters could be changed only globally, i.e. via DCS. # P.S. 'listen_addresses' and 'port' are added here just for convenience, to mark them # as a parameters which should always be passed through command line. + # + # Format: + # key - parameter name + # value - tuple(default_value, check_function, min_version) + # default_value -- some sane default value + # check_function -- if the new value is not correct must return `!False` + # min_version -- major version of PostgreSQL when parameter was introduced CMDLINE_OPTIONS = { - 'listen_addresses': None, - 'port': None, - 'wal_level': 'hot_standby', - 'hot_standby': 'on', - 'max_wal_senders': 5, - 'wal_keep_segments': 8, - 'max_replication_slots': 5, - 'wal_log_hints': 'on' + 'listen_addresses': (None, lambda _: False, 9.1), + 'port': (None, lambda _: False, 9.1), + 'cluster_name': (None, lambda _: False, 9.5), + 'wal_level': ('hot_standby', lambda v: v.lower() in ('hot_standby', 'logical'), 9.1), + 'hot_standby': ('on', lambda _: False, 9.1), + 'max_connections': (100, lambda v: int(v) >= 100, 9.1), + 'max_wal_senders': (5, lambda v: int(v) >= 5, 9.1), + 'wal_keep_segments': (8, lambda v: int(v) >= 8, 9.1), + 'max_prepared_transactions': (0, lambda v: int(v) >= 0, 9.1), + 'max_locks_per_transaction': (64, lambda v: int(v) >= 64, 9.1), + 'track_commit_timestamp': ('off', lambda v: parse_bool(v) is not None, 9.5), + 'max_replication_slots': (5, lambda v: int(v) >= 5, 9.4), + 'max_worker_processes': (8, lambda v: int(v) >= 8, 9.4), + 'wal_log_hints': ('on', lambda _: False, 9.4) } def __init__(self, config): @@ -127,11 +140,10 @@ class Postgresql(object): logger.exception('Failed to read PG_VERSION from %s', self._data_dir) return 0.0 - @staticmethod - def get_server_parameters(config): + def get_server_parameters(self, config): parameters = config['parameters'].copy() listen_addresses, port = (config['listen'] + ':5432').split(':')[:2] - parameters.update({'listen_addresses': listen_addresses, 'port': port}) + parameters.update({'cluster_name': self.scope, 'listen_addresses': listen_addresses, 'port': port}) return parameters def resolve_connection_addresses(self): @@ -496,8 +508,8 @@ class Postgresql(object): self._write_postgresql_conf() self.resolve_connection_addresses() - options = ' '.join("--{0}='{1}'".format(p, self._server_parameters[p]) for p in self.CMDLINE_OPTIONS - if not (self._major_version < 9.4 and p in ('max_replication_slots', 'wal_log_hints'))) + options = ' '.join("--{0}='{1}'".format(p, self._server_parameters[p]) for p, v in self.CMDLINE_OPTIONS.items() + if self._major_version >= v[2]) ret = subprocess.call(self._pg_ctl + ['start', '-o', options], env=env, preexec_fn=os.setsid) == 0 self._pending_restart = False diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index badee349..7fa7279a 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -153,8 +153,10 @@ 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, 'f.oo': 'bar', 'search_path': 'public', - 'hot_standby': 'on', 'max_wal_senders': 5, 'wal_keep_segments': 8, 'wal_log_hints': 'on'} + _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', 'max_locks_per_transaction': 64, + 'max_worker_processes': 8, 'max_connections': 100, 'max_prepared_transactions': 0} @patch('subprocess.call', Mock(return_value=0)) @patch('psycopg2.connect', psycopg2_connect)