From 60f7759c5e3354eab501a15a7748fe172a140636 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 1 Jun 2016 09:21:42 +0200 Subject: [PATCH] Small optimization Don't compare values of configuration if modify_index didn't changed --- patroni/__init__.py | 4 ++-- patroni/config.py | 9 +++++++++ patroni/dcs/__init__.py | 6 +++--- patroni/dcs/consul.py | 2 +- patroni/dcs/zookeeper.py | 6 +++--- tests/test_patroni.py | 2 ++ 6 files changed, 20 insertions(+), 9 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 9fc15f50..d8494c23 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -41,7 +41,7 @@ class Patroni(object): try: cluster = self.dcs.get_cluster() if cluster and cluster.config: - self.config.set_dynamic_configuration(cluster.config.data) + self.config.set_dynamic_configuration(cluster.config) elif not self.config.dynamic_configuration and 'bootstrap' in self.config: self.config.set_dynamic_configuration(self.config['bootstrap']['dcs']) break @@ -101,7 +101,7 @@ class Patroni(object): logger.info(self.ha.run_cycle()) cluster = self.dcs.cluster - if cluster and cluster.config and self.config.set_dynamic_configuration(cluster.config.data): + if cluster and cluster.config and self.config.set_dynamic_configuration(cluster.config): self.reload_config() if not self.postgresql.data_directory_empty(): diff --git a/patroni/config.py b/patroni/config.py index d4a186b9..541905a0 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -5,6 +5,7 @@ import tempfile import yaml from copy import deepcopy +from patroni.dcs import ClusterConfig from patroni.postgresql import Postgresql from patroni.utils import deep_compare @@ -42,6 +43,7 @@ class Config(object): def __init__(self, config_file=None, config_env=None): self._config_file = None if config_env else config_file + self._modify_index = -1 self._dynamic_configuration = {} self._local_configuration = yaml.safe_load(config_env) if config_env else self._load_config_file() self.__effective_configuration = self._build_effective_configuration(self._dynamic_configuration, @@ -94,7 +96,14 @@ class Config(object): except Exception: logger.error('Can not remove temporary file %s', tmpfile) + # configuration could be either ClusterConfig or dict def set_dynamic_configuration(self, configuration): + if isinstance(configuration, ClusterConfig): + if self._modify_index == configuration.modify_index: + return False # If the index didn't changed there is nothing to do + self._modify_index = configuration.modify_index + configuration = configuration.data + if not deep_compare(self._dynamic_configuration, configuration): try: self.__effective_configuration = self._build_effective_configuration(configuration, diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index d4da8e6b..c068ef8e 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -164,10 +164,10 @@ class Failover(namedtuple('Failover', 'index,leader,candidate,scheduled_at')): return Failover(index, data.get('leader'), data.get('member'), data.get('scheduled_at')) -class ClusterConfig(namedtuple('ClusterConfig', 'index,data')): +class ClusterConfig(namedtuple('ClusterConfig', 'index,data,modify_index')): @staticmethod - def from_node(index, data): + def from_node(index, data, modify_index=None): """ >>> ClusterConfig.from_node(1, '{') is None True @@ -177,7 +177,7 @@ class ClusterConfig(namedtuple('ClusterConfig', 'index,data')): data = json.loads(data) except (TypeError, ValueError): return None - return ClusterConfig(index, data) + return ClusterConfig(index, data, modify_index or index) class Cluster(namedtuple('Cluster', 'initialize,config,leader,last_leader_operation,members,failover')): diff --git a/patroni/dcs/consul.py b/patroni/dcs/consul.py index 4c5a4bc1..f7736dfb 100644 --- a/patroni/dcs/consul.py +++ b/patroni/dcs/consul.py @@ -34,7 +34,7 @@ class HTTPClient(std.HTTPClient): defaults_attr_name = '__defaults__' if six.PY3 else 'func_defaults' defaults = list(getattr(request_func, defaults_attr_name)) code = request_func.__code__ if six.PY3 else request_func.func_code - defaults[code.co_varnames[code.co_argcount - len(defaults):code.co_argcount].index('timeout')] = 5 + defaults[code.co_varnames[code.co_argcount - len(defaults):code.co_argcount].index('timeout')] = timeout setattr(request_func, defaults_attr_name, tuple(defaults)) # monkeypatching def get(self, callback, path, params=None): diff --git a/patroni/dcs/zookeeper.py b/patroni/dcs/zookeeper.py index 29367419..618cc9bd 100644 --- a/patroni/dcs/zookeeper.py +++ b/patroni/dcs/zookeeper.py @@ -150,7 +150,7 @@ class ZooKeeper(AbstractDCS): # get global dynamic configuration config = self.get_node(self.config_path, watch=self.cluster_watcher) if self._CONFIG in nodes else None - config = config and ClusterConfig.from_node(config[1].version, config[0]) + config = config and ClusterConfig.from_node(config[1].version, config[0], config[1].mzxid) # get list of members members = self.load_members() if self._MEMBERS[:-1] in nodes else [] @@ -209,7 +209,7 @@ class ZooKeeper(AbstractDCS): self._client.retry(self._client.set, self.failover_path, value.encode('utf-8'), version=index or -1) return True except NoNodeError: - return value == '' or (not index and self._create(self.failover_path, value)) + return value == '' or (index is None and self._create(self.failover_path, value)) except: logging.exception('set_failover_value') return False @@ -219,7 +219,7 @@ class ZooKeeper(AbstractDCS): self._client.retry(self._client.set, self.config_path, value.encode('utf-8'), version=index or -1) return True except NoNodeError: - return value == '' or (not index and self._create(self.config_path, value)) + return index is None and self._create(self.config_path, value) except Exception: logging.exception('set_config_value') return False diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 71575e82..1e53e27b 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -71,6 +71,8 @@ class TestPatroni(unittest.TestCase): self.p.api.start = Mock() self.p.config._dynamic_configuration = {} 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)): self.assertRaises(SleepException, self.p.run)