From e86cf9a722d1da399f6acdd6afd574251f9302ee Mon Sep 17 00:00:00 2001 From: Dmytro Aleksandrov Date: Mon, 1 Aug 2016 10:16:13 +0300 Subject: [PATCH 01/23] Attempt to make single binary build with pyinstaller --- .gitignore | 1 + mkbinary.sh | 5 +++++ patroni.spec | 29 +++++++++++++++++++++++++++++ patroni/config.py | 2 +- patroni/dcs/__init__.py | 41 +++++++++++++++++++++++++++-------------- requirements-bin.txt | 2 ++ 6 files changed, 65 insertions(+), 15 deletions(-) create mode 100755 mkbinary.sh create mode 100644 patroni.spec create mode 100644 requirements-bin.txt diff --git a/.gitignore b/.gitignore index fbb294d4..c03acb2b 100644 --- a/.gitignore +++ b/.gitignore @@ -6,6 +6,7 @@ data/* .coverage .eggs/ build/ +dist/ coverage.xml junit.xml pgpass diff --git a/mkbinary.sh b/mkbinary.sh new file mode 100755 index 00000000..67829074 --- /dev/null +++ b/mkbinary.sh @@ -0,0 +1,5 @@ +#!/bin/sh +set -e + +pip install --ignore-installed -r requirements-bin.txt +pyinstaller --clean --onefile patroni.spec diff --git a/patroni.spec b/patroni.spec new file mode 100644 index 00000000..8c5c5347 --- /dev/null +++ b/patroni.spec @@ -0,0 +1,29 @@ +# -*- mode: python -*- + +block_cipher = None + + +a = Analysis(['patroni/__main__.py', 'patroni/dcs/consul.py', 'patroni/dcs/etcd.py', 'patroni/dcs/exhibitor.py', 'patroni/dcs/zookeeper.py'], + pathex=[], + binaries=None, + datas=None, + hiddenimports=['patroni.dcs.consul', 'patroni.dcs.etcd', 'patroni.dcs.exhibitor', 'patroni.dcs.zookeeper'], + hookspath=[], + runtime_hooks=[], + excludes=[], + win_no_prefer_redirects=False, + win_private_assemblies=False, + cipher=block_cipher) + +pyz = PYZ(a.pure, a.zipped_data, cipher=block_cipher) + +exe = EXE(pyz, + a.scripts, + a.binaries, + a.zipfiles, + a.datas, + name='patroni', + debug=False, + strip=False, + upx=True, + console=True) diff --git a/patroni/config.py b/patroni/config.py index f09cfca4..69757067 100644 --- a/patroni/config.py +++ b/patroni/config.py @@ -64,7 +64,7 @@ class Config(object): print('Usage: {0} config.yml'.format(sys.argv[0])) print('\tPatroni may also read the configuration from the {0} environment variable'. format(self.PATRONI_CONFIG_VARIABLE)) - exit(1) + sys.exit(1) self.__effective_configuration = self._build_effective_configuration({}, self._local_configuration) self._data_dir = self.__effective_configuration['postgresql']['data_dir'] diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index d7a69997..0045083c 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -6,6 +6,7 @@ import json import os import pkgutil import six +import sys from collections import namedtuple from patroni.exceptions import PatroniException @@ -31,22 +32,34 @@ def parse_connection_string(value): return conn_url, api_url +def dcs_modules(): + """Get names of DCS modules, depending on execution environment. If being packaged with PyInstaller, + modules aren't discoverable dynamically by scanning source directory. Thus, when running in bundle, + a predefined list of dcs modules is returned. See: + https://pyinstaller.readthedocs.io/en/stable/runtime-information.html#run-time-information""" + + if getattr(sys, 'frozen', False): + return ['consul', 'etcd', 'zookeeper', 'exhibitor'] + else: + module_names = (name for _, name, is_pkg in pkgutil.iter_modules([os.path.dirname(__file__)]) if not is_pkg) + return module_names + + def get_dcs(config): available_implementations = set() - for _, module_name, is_pkg in pkgutil.iter_modules([os.path.dirname(__file__)]): - if not is_pkg: - module = importlib.import_module(__package__ + '.' + module_name) - for name in filter(lambda name: not name.startswith('__'), dir(module)): # iterate through module content - value = getattr(module, name) - name = name.lower() - # try to find implementation of AbstractDCS interface, class name must match with module_name - if inspect.isclass(value) and issubclass(value, AbstractDCS) and name == module_name: - available_implementations.add(name) - if name in config: # which has configuration section in the config file - # propagate some parameters - config[name].update({p: config[p] for p in ('namespace', 'name', 'scope', - 'loop_wait', 'ttl', 'retry_timeout') if p in config}) - return value(config[name]) + for module_name in dcs_modules(): + module = importlib.import_module(__package__ + '.' + module_name) + for name in filter(lambda name: not name.startswith('__'), dir(module)): # iterate through module content + value = getattr(module, name) + name = name.lower() + # try to find implementation of AbstractDCS interface, class name must match with module_name + if inspect.isclass(value) and issubclass(value, AbstractDCS) and name == module_name: + available_implementations.add(name) + if name in config: # which has configuration section in the config file + # propagate some parameters + config[name].update({p: config[p] for p in ('namespace', 'name', 'scope', + 'loop_wait', 'ttl', 'retry_timeout') if p in config}) + return value(config[name]) raise PatroniException("""Can not find suitable configuration of distributed configuration store Available implementations: """ + ', '.join(available_implementations)) diff --git a/requirements-bin.txt b/requirements-bin.txt new file mode 100644 index 00000000..7e43a3fd --- /dev/null +++ b/requirements-bin.txt @@ -0,0 +1,2 @@ +setuptools==19.2 +pyinstaller From ebbc8bad4cd53a91c9a5ce6624ac12422e07b802 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 29 Aug 2016 15:19:18 +0200 Subject: [PATCH 02/23] Create pause.rst --- docs/pause.rst | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 docs/pause.rst diff --git a/docs/pause.rst b/docs/pause.rst new file mode 100644 index 00000000..46f3dbcb --- /dev/null +++ b/docs/pause.rst @@ -0,0 +1,37 @@ +Pause/Resume mode for the cluster +================================= + +The goal +-------- + +Under certain cicuimstances Patroni needs to temporary step down from managing the cluster, while still retaining the cluster state in DCS. Possible use cases are uncommon activities on the cluster, such as major version upgrades or corruption recovery. During those activities nodes are often started and stopped for the reason unknown to Patroni, some nodes +can be even temporary promoted, violating the assumption of running only +one master. + + +The implementation +------------------ + +When Patroni runs in a paused mode, it does not change the state of PostgreSQL, except for the following cases: + +- For each node, the member key in DCS is updated with the current information about the cluster. This causes Patroni to run read-only +queries on a member node if the member is running. + +- For the leader node, Patroni maintains the leader key and promotes the node holding the leader key if it is not the master. + +- Manual unscheduled restart and manual failover are allowed. Manual failover is only allowed if the node to failover to is specified. In the paused mode, manual failover does not requre a running master node. + +- If 'paralel' masters are detected by Patroni, it will acknowledge them, but will not try to demote those masters not having the leader lock. + +- If there is no leader lock in the cluster, Patroni promotes a running master. If there is more than one master node, then the first master node to try to acquire the lock wins. There is an exception in this rule: if there is no leader lock because the old master has demoted itself due to the +manual promotion, than only the node mentioned in the promotion may take the leader lock. + +When the new leader is promoted, Patroni makes sure the replicas that were streaming from the previous leader will switch to the new one. + + +User guide +---------- + +``patronictl`` supports ``pause`` and ``resume`` commands. + +One can also issue a ``PATCH`` request to the ``{namespace}/{cluser}/config`` key with ``{'pause': True/False}`` From b498b225844c7112fa1f6b2492cfc78e2d635a9c Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 29 Aug 2016 15:23:59 +0200 Subject: [PATCH 03/23] Minor formatting fix. --- docs/pause.rst | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/docs/pause.rst b/docs/pause.rst index 46f3dbcb..d031d2d9 100644 --- a/docs/pause.rst +++ b/docs/pause.rst @@ -4,9 +4,7 @@ Pause/Resume mode for the cluster The goal -------- -Under certain cicuimstances Patroni needs to temporary step down from managing the cluster, while still retaining the cluster state in DCS. Possible use cases are uncommon activities on the cluster, such as major version upgrades or corruption recovery. During those activities nodes are often started and stopped for the reason unknown to Patroni, some nodes -can be even temporary promoted, violating the assumption of running only -one master. +Under certain cicuimstances Patroni needs to temporary step down from managing the cluster, while still retaining the cluster state in DCS. Possible use cases are uncommon activities on the cluster, such as major version upgrades or corruption recovery. During those activities nodes are often started and stopped for the reason unknown to Patroni, some nodes can be even temporary promoted, violating the assumption of running only one master. The implementation @@ -14,8 +12,7 @@ The implementation When Patroni runs in a paused mode, it does not change the state of PostgreSQL, except for the following cases: -- For each node, the member key in DCS is updated with the current information about the cluster. This causes Patroni to run read-only -queries on a member node if the member is running. +- For each node, the member key in DCS is updated with the current information about the cluster. This causes Patroni to run read-only queries on a member node if the member is running. - For the leader node, Patroni maintains the leader key and promotes the node holding the leader key if it is not the master. @@ -23,8 +20,7 @@ queries on a member node if the member is running. - If 'paralel' masters are detected by Patroni, it will acknowledge them, but will not try to demote those masters not having the leader lock. -- If there is no leader lock in the cluster, Patroni promotes a running master. If there is more than one master node, then the first master node to try to acquire the lock wins. There is an exception in this rule: if there is no leader lock because the old master has demoted itself due to the -manual promotion, than only the node mentioned in the promotion may take the leader lock. +- If there is no leader lock in the cluster, Patroni promotes a running master. If there is more than one master node, then the first master node to try to acquire the lock wins. There is an exception in this rule: if there is no leader lock because the old master has demoted itself due to the manual promotion, than only the node mentioned in the promotion may take the leader lock. When the new leader is promoted, Patroni makes sure the replicas that were streaming from the previous leader will switch to the new one. From 2743dc0f7aa9756c877660c251771e3ba43dfb88 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 29 Aug 2016 15:35:57 +0200 Subject: [PATCH 04/23] Spellcheck minor stylistic fixes --- docs/pause.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/pause.rst b/docs/pause.rst index d031d2d9..6080bd40 100644 --- a/docs/pause.rst +++ b/docs/pause.rst @@ -4,7 +4,7 @@ Pause/Resume mode for the cluster The goal -------- -Under certain cicuimstances Patroni needs to temporary step down from managing the cluster, while still retaining the cluster state in DCS. Possible use cases are uncommon activities on the cluster, such as major version upgrades or corruption recovery. During those activities nodes are often started and stopped for the reason unknown to Patroni, some nodes can be even temporary promoted, violating the assumption of running only one master. +Under certain circumstances Patroni needs to temporary step down from managing the cluster, while still retaining the cluster state in DCS. Possible use cases are uncommon activities on the cluster, such as major version upgrades or corruption recovery. During those activities nodes are often started and stopped for the reason unknown to Patroni, some nodes can be even temporary promoted, violating the assumption of running only one master. The implementation @@ -16,9 +16,9 @@ When Patroni runs in a paused mode, it does not change the state of PostgreSQL, - For the leader node, Patroni maintains the leader key and promotes the node holding the leader key if it is not the master. -- Manual unscheduled restart and manual failover are allowed. Manual failover is only allowed if the node to failover to is specified. In the paused mode, manual failover does not requre a running master node. +- Manual unscheduled restart and manual failover are allowed. Manual failover is only allowed if the node to failover to is specified. In the paused mode, manual failover does not require a running master node. -- If 'paralel' masters are detected by Patroni, it will acknowledge them, but will not try to demote those masters not having the leader lock. +- If 'parallel' masters are detected by Patroni, it emits a warning, but does not demote the masters without the leader lock. - If there is no leader lock in the cluster, Patroni promotes a running master. If there is more than one master node, then the first master node to try to acquire the lock wins. There is an exception in this rule: if there is no leader lock because the old master has demoted itself due to the manual promotion, than only the node mentioned in the promotion may take the leader lock. @@ -30,4 +30,4 @@ User guide ``patronictl`` supports ``pause`` and ``resume`` commands. -One can also issue a ``PATCH`` request to the ``{namespace}/{cluser}/config`` key with ``{'pause': True/False}`` +One can also issue a ``PATCH`` request to the ``{namespace}/{cluster}/config`` key with ``{'pause': True/False}`` From de6cba6e0450b62bc23de68a8eb45dc36f557d3a Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 29 Aug 2016 18:13:45 +0200 Subject: [PATCH 05/23] Rephrase the leader lock section, mention reinit --- docs/pause.rst | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/docs/pause.rst b/docs/pause.rst index 6080bd40..64f733b0 100644 --- a/docs/pause.rst +++ b/docs/pause.rst @@ -4,7 +4,8 @@ Pause/Resume mode for the cluster The goal -------- -Under certain circumstances Patroni needs to temporary step down from managing the cluster, while still retaining the cluster state in DCS. Possible use cases are uncommon activities on the cluster, such as major version upgrades or corruption recovery. During those activities nodes are often started and stopped for the reason unknown to Patroni, some nodes can be even temporary promoted, violating the assumption of running only one master. +Under certain circumstances Patroni needs to temporary step down from managing the cluster, while still retaining the cluster state in DCS. Possible use cases are uncommon activities on the cluster, such as major version upgrades or corruption recovery. During those activities nodes are often started and stopped for the reason unknown to Patroni, some nodes can be even temporary promoted, violating the assumption of running only one master. Therefore, Patroni needs to be able to "detach" from the running cluster, implementing an equivalent of the maintenance mode in Pacemaker. + The implementation @@ -18,12 +19,15 @@ When Patroni runs in a paused mode, it does not change the state of PostgreSQL, - Manual unscheduled restart and manual failover are allowed. Manual failover is only allowed if the node to failover to is specified. In the paused mode, manual failover does not require a running master node. +- Replica reinitialize is allowed. + - If 'parallel' masters are detected by Patroni, it emits a warning, but does not demote the masters without the leader lock. -- If there is no leader lock in the cluster, Patroni promotes a running master. If there is more than one master node, then the first master node to try to acquire the lock wins. There is an exception in this rule: if there is no leader lock because the old master has demoted itself due to the manual promotion, than only the node mentioned in the promotion may take the leader lock. +- If there is no leader lock in the cluster, the running master acquires the lock. If there is more than one master node, then the first master to acquire the lock wins. If there are no masters altogether, Patroni does not try to promote any replicas. There is an exception in this rule: if there is no leader lock because the old master has demoted itself due to the manual promotion, then only the candidate node mentioned in the promotion request may take the leader lock. -When the new leader is promoted, Patroni makes sure the replicas that were streaming from the previous leader will switch to the new one. +When the new leader lock is granted (i.e. after promoting a replica manually), Patroni makes sure the replicas that were streaming from the previous leader will switch to the new one. +- When Postgres is stopped, Patroni does not try to start it. When Patroni is stopped, it does not to stop Postgres instance it is managing. User guide ---------- From 64e3e2fb2d234d6b686065404c00d56d3e82bdab Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 29 Aug 2016 18:15:16 +0200 Subject: [PATCH 06/23] Formatting fix --- docs/pause.rst | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/docs/pause.rst b/docs/pause.rst index 64f733b0..f489a078 100644 --- a/docs/pause.rst +++ b/docs/pause.rst @@ -23,9 +23,7 @@ When Patroni runs in a paused mode, it does not change the state of PostgreSQL, - If 'parallel' masters are detected by Patroni, it emits a warning, but does not demote the masters without the leader lock. -- If there is no leader lock in the cluster, the running master acquires the lock. If there is more than one master node, then the first master to acquire the lock wins. If there are no masters altogether, Patroni does not try to promote any replicas. There is an exception in this rule: if there is no leader lock because the old master has demoted itself due to the manual promotion, then only the candidate node mentioned in the promotion request may take the leader lock. - -When the new leader lock is granted (i.e. after promoting a replica manually), Patroni makes sure the replicas that were streaming from the previous leader will switch to the new one. +- If there is no leader lock in the cluster, the running master acquires the lock. If there is more than one master node, then the first master to acquire the lock wins. If there are no masters altogether, Patroni does not try to promote any replicas. There is an exception in this rule: if there is no leader lock because the old master has demoted itself due to the manual promotion, then only the candidate node mentioned in the promotion request may take the leader lock. When the new leader lock is granted (i.e. after promoting a replica manually), Patroni makes sure the replicas that were streaming from the previous leader will switch to the new one. - When Postgres is stopped, Patroni does not try to start it. When Patroni is stopped, it does not to stop Postgres instance it is managing. From 030ba4c898cde071def1d1c01d3bbf57edc5cf5e Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 30 Aug 2016 10:58:20 +0200 Subject: [PATCH 07/23] Address the review - fix the false claim that Patroni in a paused mode may promote a master. - merge all allowed API actions together. - describe the payload of the API pause request as JSON. --- docs/pause.rst | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/docs/pause.rst b/docs/pause.rst index f489a078..fb049347 100644 --- a/docs/pause.rst +++ b/docs/pause.rst @@ -15,11 +15,9 @@ When Patroni runs in a paused mode, it does not change the state of PostgreSQL, - For each node, the member key in DCS is updated with the current information about the cluster. This causes Patroni to run read-only queries on a member node if the member is running. -- For the leader node, Patroni maintains the leader key and promotes the node holding the leader key if it is not the master. +- For the Postgres master with the leader lock Patroni updates the lock. If the node with the leader lock stops being the master (i.e. is demoted manually), Patroni will release the lock instead of promoting the node back. -- Manual unscheduled restart and manual failover are allowed. Manual failover is only allowed if the node to failover to is specified. In the paused mode, manual failover does not require a running master node. - -- Replica reinitialize is allowed. +- Manual unscheduled restart, reinitialize and manual failover are allowed. Manual failover is only allowed if the node to failover to is specified. In the paused mode, manual failover does not require a running master node. - If 'parallel' masters are detected by Patroni, it emits a warning, but does not demote the masters without the leader lock. @@ -32,4 +30,4 @@ User guide ``patronictl`` supports ``pause`` and ``resume`` commands. -One can also issue a ``PATCH`` request to the ``{namespace}/{cluster}/config`` key with ``{'pause': True/False}`` +One can also issue a ``PATCH`` request to the ``{namespace}/{cluster}/config`` key with ``{"pause": true/false/null}`` From 5c2cad20d7e80cfd55db710ce637f7034e115a52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Wed, 31 Aug 2016 15:30:31 +0200 Subject: [PATCH 08/23] Add custom_conf configuration parameter This will be used in place of postgresql.base.conf, to be included on the main postgresql.conf. --- patroni/postgresql.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 37d09460..a189acc4 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -100,9 +100,13 @@ class Postgresql(object): self._postgresql_conf = os.path.join(self._data_dir, config_base_name + '.conf') self._postgresql_base_conf_name = config_base_name + '.base.conf' self._postgresql_base_conf = os.path.join(self._data_dir, self._postgresql_base_conf_name) + self._postgresql_custom_conf = config.get('custom_conf') self._recovery_conf = os.path.join(self._data_dir, 'recovery.conf') - self._configuration_to_save = (self._postgresql_conf, self._postgresql_base_conf, - os.path.join(self._data_dir, 'pg_hba.conf')) + self._configuration_to_save = [self._postgresql_conf] + if not self._postgresql_custom_conf: + self._configuration_to_save.append(self._postgresql_base_conf) + if not config['parameters'].get('hba_file'): + self._configuration_to_save.append(os.path.join(self._data_dir, 'pg_hba.conf')) self._postmaster_pid = os.path.join(self._data_dir, 'postmaster.pid') self._trigger_file = config.get('recovery_conf', {}).get('trigger_file') or 'promote' self._trigger_file = os.path.abspath(os.path.join(self._data_dir, self._trigger_file)) @@ -622,12 +626,12 @@ class Postgresql(object): def _write_postgresql_conf(self): # rename the original configuration if it is necessary - if not os.path.exists(self._postgresql_base_conf): + if not self._postgresql_custom_conf and not os.path.exists(self._postgresql_base_conf): os.rename(self._postgresql_conf, self._postgresql_base_conf) with open(self._postgresql_conf, 'w') as f: f.write('# Do not edit this file manually!\n# It will be overwritten by Patroni!\n') - f.write("include '{0}'\n\n".format(self._postgresql_base_conf_name)) + f.write("include '{0}'\n\n".format(self._postgresql_custom_conf or self._postgresql_base_conf_name)) for name, value in sorted(self._server_parameters.items()): if name not in self.CMDLINE_OPTIONS: f.write("{0} = '{1}'\n".format(name, value)) From 1fb562e118cc92c5749393bddd89f2acbe430d98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Wed, 31 Aug 2016 15:38:42 +0200 Subject: [PATCH 09/23] Add custom_conf parameter documentation --- docs/SETTINGS.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/SETTINGS.rst b/docs/SETTINGS.rst index 24b01dd3..6d5894ea 100644 --- a/docs/SETTINGS.rst +++ b/docs/SETTINGS.rst @@ -69,6 +69,7 @@ PostgreSQL - **listen**: IP address + port that Postgres listens to; must be accessible from other nodes in the cluster, if you're using streaming replication. Multiple comma-separated addresses are permitted, as long as the port component is appended after to the last one with a colon, i.e. ``listen: 127.0.0.1,127.0.0.2:5432``. Patroni will use the first address from this list to establish local connections to the PostgreSQL node. - **pgpass**: path to the `.pgpass `__ password file. Patroni creates this file before executing pg\_basebackup and under some other circumstances. The location must be writable by Patroni. - **recovery\_conf**: additional configuration settings written to recovery.conf when configuring follower. +- **custom_conf** : path to a custom `postgresql.conf` file, that will be used in place of `postgresql.base.conf`. The file must exist and will be included from its location on the real `postgresql.conf`. - **parameters**: list of configuration settings for Postgres. Many of these are required for replication to work. - **pg\_ctl\_timeout**: How long should pg_ctl wait when doing ``start``, ``stop`` or ``restart``. Default value is 60 seconds. - **use\_pg\_rewind**: try to use pg\_rewind on the former leader when it joins cluster as a replica. From a642860ae8d0798f8d4145b2778bf9d988567f42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Thu, 1 Sep 2016 17:06:33 +0200 Subject: [PATCH 10/23] Turn _configuration_to_save into a property method --- patroni/postgresql.py | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index a189acc4..0baff425 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -102,11 +102,6 @@ class Postgresql(object): self._postgresql_base_conf = os.path.join(self._data_dir, self._postgresql_base_conf_name) self._postgresql_custom_conf = config.get('custom_conf') self._recovery_conf = os.path.join(self._data_dir, 'recovery.conf') - self._configuration_to_save = [self._postgresql_conf] - if not self._postgresql_custom_conf: - self._configuration_to_save.append(self._postgresql_base_conf) - if not config['parameters'].get('hba_file'): - self._configuration_to_save.append(os.path.join(self._data_dir, 'pg_hba.conf')) self._postmaster_pid = os.path.join(self._data_dir, 'postmaster.pid') self._trigger_file = config.get('recovery_conf', {}).get('trigger_file') or 'promote' self._trigger_file = os.path.abspath(os.path.join(self._data_dir, self._trigger_file)) @@ -128,6 +123,15 @@ class Postgresql(object): self.set_role('master' if self.is_leader() else 'replica') self._write_postgresql_conf() # we are "joining" already running postgres + @property + def _configuration_to_save(self): + configuration = [self._postgresql_conf] + if not self._postgresql_custom_conf: + configuration.append(self._postgresql_base_conf) + if not self.config['parameters'].get('hba_file'): + configuration.append(os.path.join(self._data_dir, 'pg_hba.conf')) + return configuration + @property def use_slots(self): return self._use_slots and self._major_version >= 9.4 From f58ff3a96f7f93bfaba2b023ecdbbb76c3e01007 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Thu, 1 Sep 2016 17:59:47 +0200 Subject: [PATCH 11/23] Document custom_conf parameter --- docs/SETTINGS.rst | 2 +- docs/dynamic_configuration.rst | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/docs/SETTINGS.rst b/docs/SETTINGS.rst index 6d5894ea..aa6b3e31 100644 --- a/docs/SETTINGS.rst +++ b/docs/SETTINGS.rst @@ -69,7 +69,7 @@ PostgreSQL - **listen**: IP address + port that Postgres listens to; must be accessible from other nodes in the cluster, if you're using streaming replication. Multiple comma-separated addresses are permitted, as long as the port component is appended after to the last one with a colon, i.e. ``listen: 127.0.0.1,127.0.0.2:5432``. Patroni will use the first address from this list to establish local connections to the PostgreSQL node. - **pgpass**: path to the `.pgpass `__ password file. Patroni creates this file before executing pg\_basebackup and under some other circumstances. The location must be writable by Patroni. - **recovery\_conf**: additional configuration settings written to recovery.conf when configuring follower. -- **custom_conf** : path to a custom `postgresql.conf` file, that will be used in place of `postgresql.base.conf`. The file must exist and will be included from its location on the real `postgresql.conf`. +- **custom_conf** : path to an optional custom ``postgresql.conf`` file, that will be used in place of ``postgresql.base.conf``. The file must exist on all cluster nodes, be readable by PostgreSQL and will be included from its location on the real ``postgresql.conf``. Note that Patroni will not monitor this file for changes, nor backup it. However, its settings can still be overriden by Patroni's own configuration facilities - see `dynamic configuration `__ for details. - **parameters**: list of configuration settings for Postgres. Many of these are required for replication to work. - **pg\_ctl\_timeout**: How long should pg_ctl wait when doing ``start``, ``stop`` or ``restart``. Default value is 60 seconds. - **use\_pg\_rewind**: try to use pg\_rewind on the former leader when it joins cluster as a replica. diff --git a/docs/dynamic_configuration.rst b/docs/dynamic_configuration.rst index 9aa100dd..ce773b60 100644 --- a/docs/dynamic_configuration.rst +++ b/docs/dynamic_configuration.rst @@ -48,23 +48,24 @@ To be on the safe side parameters from the above lists are not written into ``po When applying the local or dynamic configuration options, the following actions are taken: -- The node first checks if there is a postgresql.base.conf. -- If it exists, it contains the renamed "original" configuration. -- If it doesn't, the original postgresql.conf is taken and renamed to postgresql.base.conf. +- The node first checks if there is a postgresql.base.conf or if the ``custom_conf`` parameter is set. +- If the `custom_conf` parameter is set, it will take the file specified on it as a base configuration, ignoring `postgresql.base.conf` and `postgresql.conf`. +- If the `custom_conf` parameter is not set and `postgresql.base.conf` exists, it contains the renamed "original" configuration and it will be used as a base configuration. +- If there is no `custom_conf` nor `postgresql.base.conf`, the original postgresql.conf is taken and renamed to postgresql.base.conf. - The dynamic options (with the exceptions above) are dumped into the postgresql.conf and an include is set in - postgresql.conf to postgresql.base.conf. Therefore, we would be able to apply new options without re-reading the configuration file to check if the include is present not. + postgresql.conf to the used base configuration (either postgresql.base.conf or what is on ``custom_conf``). Therefore, we would be able to apply new options without re-reading the configuration file to check if the include is present not. - Some parameters that are essential for Patroni to manage the cluster are overridden using the command line. - If some of the options that require restart are changed (we should look at the context in pg_settings and at the actual values of those options), a pending_restart flag of a given node is set. This flag is reset on any restart. The parameters would be applied in the following order (run-time are given the highest priority): -1. load parameters from file `postgresql.base.conf` +1. load parameters from file `postgresql.base.conf` (or from a `custom_conf` file, if set) 2. load parameters from file `postgresql.conf` 3. load parameters from file `postgresql.auto.conf` 4. run-time parameter using `-o --name=value` -This allows configuration for all the nodes (2), configuration for a specific node using `ALTER SYSTEM` (3) and ensures that parameters essential to the running of Patroni are enforced. (4) +This allows configuration for all the nodes (2), configuration for a specific node using `ALTER SYSTEM` (3) and ensures that parameters essential to the running of Patroni are enforced (4), as well as leaves room for configuration tools that manage `postgresql.conf` directly without involving Patroni (1). Also, the following Patroni configuration options can be changed only dynamically: From 005d1815073017ca510b113ff68f9b18f5707af1 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 2 Sep 2016 08:56:25 +0200 Subject: [PATCH 12/23] Callbacks should be loaded not only on init. Load callbacks from config (which is properly reloaded) instead of reading them from init only once. --- patroni/postgresql.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 37d09460..d35fe4bf 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -94,7 +94,6 @@ class Postgresql(object): self._schedule_load_slots = self.use_slots self._pgpass = config.get('pgpass') or os.path.join(os.path.expanduser('~'), 'pgpass') - self.callback = config.get('callbacks') or {} self.__cb_called = False config_base_name = config.get('config_base_name', 'postgresql') self._postgresql_conf = os.path.join(self._data_dir, config_base_name + '.conf') @@ -128,6 +127,10 @@ class Postgresql(object): def use_slots(self): return self._use_slots and self._major_version >= 9.4 + @property + def callback(self): + return self.config.get('callbacks') or {} + def _version_file_exists(self): return not self.data_directory_empty() and os.path.isfile(self._version_file) From 75cd8422ebcb2ea6e5cc7d3d6041cf036292a426 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Fri, 2 Sep 2016 11:25:11 +0200 Subject: [PATCH 13/23] Do not attempt to parse missing conn url --- patroni/postgresql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 37d09460..6ea0279f 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -416,7 +416,7 @@ class Postgresql(object): # If there is no configuration key, or no value is specified, use basebackup replica_methods = self.config.get('create_replica_method') or ['basebackup'] - if clone_member: + if clone_member and clone_member.conn_url: r = clone_member.conn_kwargs(self._replication) connstring = 'postgres://{user}@{host}:{port}/{database}'.format(**r) # add the credentials to connect to the replica origin to pgpass. From 19c80df442b8afaa8ef136e50bd850b080bab1e5 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 2 Sep 2016 13:44:47 +0200 Subject: [PATCH 14/23] Try to mitigate EtcdEventIndexCleared exception (#287) This error is send by etcd when Patroni is doing "watch" on leader key which is never updated after creation and etcd cluster receives a lot of updates, what cleans history of events. Instead of doing watch on modifiedIndex + 1 we will do watch on X-Etcd-Index, which is probably still available... --- patroni/dcs/etcd.py | 5 +++-- tests/test_etcd.py | 4 +++- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/patroni/dcs/etcd.py b/patroni/dcs/etcd.py index d81cf968..c58a2ddb 100644 --- a/patroni/dcs/etcd.py +++ b/patroni/dcs/etcd.py @@ -292,7 +292,8 @@ class Etcd(AbstractDCS): if leader: member = Member(-1, leader.value, None, {}) member = ([m for m in members if m.name == leader.value] or [member])[0] - leader = Leader(leader.modifiedIndex, leader.ttl, member) + index = result.etcd_index if result.etcd_index > leader.modifiedIndex else leader.modifiedIndex + 1 + leader = Leader(index, leader.ttl, member) # failover key failover = nodes.get(self._FAILOVER) @@ -371,7 +372,7 @@ class Etcd(AbstractDCS): while timeout >= 1: # when timeout is too small urllib3 doesn't have enough time to connect try: - self._client.watch(self.leader_path, index=cluster.leader.index + 1, timeout=timeout + 0.5) + self._client.watch(self.leader_path, index=cluster.leader.index, timeout=timeout + 0.5) # Synchronous work of all cluster members with etcd is less expensive # than reestablishing http connection every time from every replica. return True diff --git a/tests/test_etcd.py b/tests/test_etcd.py index f3a32a41..16035d6e 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -103,7 +103,9 @@ def etcd_read(self, key, **kwargs): "expiration": "2015-05-15T09:11:09.611860899Z", "ttl": 30, "modifiedIndex": 20730, "createdIndex": 20730}], "modifiedIndex": 1581, "createdIndex": 1581}], "modifiedIndex": 1581, "createdIndex": 1581}} - return etcd.EtcdResult(**response) + result = etcd.EtcdResult(**response) + result.etcd_index = 0 + return result class SleepException(Exception): From 07e95912a231e69b5403a35e974a02c924606ac6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Fri, 2 Sep 2016 16:22:11 +0200 Subject: [PATCH 15/23] Fetch custom_conf dynamically --- patroni/postgresql.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 0baff425..62c7809d 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -100,7 +100,6 @@ class Postgresql(object): self._postgresql_conf = os.path.join(self._data_dir, config_base_name + '.conf') self._postgresql_base_conf_name = config_base_name + '.base.conf' self._postgresql_base_conf = os.path.join(self._data_dir, self._postgresql_base_conf_name) - self._postgresql_custom_conf = config.get('custom_conf') self._recovery_conf = os.path.join(self._data_dir, 'recovery.conf') self._postmaster_pid = os.path.join(self._data_dir, 'postmaster.pid') self._trigger_file = config.get('recovery_conf', {}).get('trigger_file') or 'promote' @@ -126,7 +125,7 @@ class Postgresql(object): @property def _configuration_to_save(self): configuration = [self._postgresql_conf] - if not self._postgresql_custom_conf: + if not config.get('custom_conf'): configuration.append(self._postgresql_base_conf) if not self.config['parameters'].get('hba_file'): configuration.append(os.path.join(self._data_dir, 'pg_hba.conf')) @@ -630,12 +629,12 @@ class Postgresql(object): def _write_postgresql_conf(self): # rename the original configuration if it is necessary - if not self._postgresql_custom_conf and not os.path.exists(self._postgresql_base_conf): + if not config.get('custom_conf') and not os.path.exists(self._postgresql_base_conf): os.rename(self._postgresql_conf, self._postgresql_base_conf) with open(self._postgresql_conf, 'w') as f: f.write('# Do not edit this file manually!\n# It will be overwritten by Patroni!\n') - f.write("include '{0}'\n\n".format(self._postgresql_custom_conf or self._postgresql_base_conf_name)) + f.write("include '{0}'\n\n".format(config.get('custom_conf') or self._postgresql_base_conf_name)) for name, value in sorted(self._server_parameters.items()): if name not in self.CMDLINE_OPTIONS: f.write("{0} = '{1}'\n".format(name, value)) From ebf64828e1eaf0e36aa303a9a99b738064909da5 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Fri, 2 Sep 2016 16:37:06 +0200 Subject: [PATCH 16/23] Decode output from wal-e list backup When running this script using Python3, the output is bytestring instead of string. We explicitly decode it to ensure checks further down are ok. The trigger for this patch is: ERROR: unable to get some of WALE backup parameters: 'expanded_size_bytes' --- patroni/scripts/wale_restore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/scripts/wale_restore.py b/patroni/scripts/wale_restore.py index 4383e986..28eefe2e 100755 --- a/patroni/scripts/wale_restore.py +++ b/patroni/scripts/wale_restore.py @@ -73,7 +73,7 @@ class WALERestore(object): # base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z # 20310671 00000001000000000000007F 00000040 # 00000001000000000000007F 00000240 - backup_strings = latest_backup.splitlines() if latest_backup else () + backup_strings = latest_backup.decode('utf-8').splitlines() if latest_backup else () if len(backup_strings) != 2: return False From 80abe67ed2b4572c28363f6ef8ef3033ae9befe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Fri, 2 Sep 2016 16:42:40 +0200 Subject: [PATCH 17/23] Fix missing self --- patroni/postgresql.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 62c7809d..879adb94 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -125,7 +125,7 @@ class Postgresql(object): @property def _configuration_to_save(self): configuration = [self._postgresql_conf] - if not config.get('custom_conf'): + if not self.config.get('custom_conf'): configuration.append(self._postgresql_base_conf) if not self.config['parameters'].get('hba_file'): configuration.append(os.path.join(self._data_dir, 'pg_hba.conf')) @@ -629,12 +629,12 @@ class Postgresql(object): def _write_postgresql_conf(self): # rename the original configuration if it is necessary - if not config.get('custom_conf') and not os.path.exists(self._postgresql_base_conf): + if not self.config.get('custom_conf') and not os.path.exists(self._postgresql_base_conf): os.rename(self._postgresql_conf, self._postgresql_base_conf) with open(self._postgresql_conf, 'w') as f: f.write('# Do not edit this file manually!\n# It will be overwritten by Patroni!\n') - f.write("include '{0}'\n\n".format(config.get('custom_conf') or self._postgresql_base_conf_name)) + f.write("include '{0}'\n\n".format(self.config.get('custom_conf') or self._postgresql_base_conf_name)) for name, value in sorted(self._server_parameters.items()): if name not in self.CMDLINE_OPTIONS: f.write("{0} = '{1}'\n".format(name, value)) From d98f255b64053b59e038e2990f6758129f95a1f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Fri, 2 Sep 2016 16:44:40 +0200 Subject: [PATCH 18/23] Do not fetch values in configuration checks --- patroni/postgresql.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 879adb94..1c06f2fc 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -125,7 +125,7 @@ class Postgresql(object): @property def _configuration_to_save(self): configuration = [self._postgresql_conf] - if not self.config.get('custom_conf'): + if 'custom_conf' not in self.config: configuration.append(self._postgresql_base_conf) if not self.config['parameters'].get('hba_file'): configuration.append(os.path.join(self._data_dir, 'pg_hba.conf')) @@ -629,7 +629,7 @@ class Postgresql(object): def _write_postgresql_conf(self): # rename the original configuration if it is necessary - if not self.config.get('custom_conf') and not os.path.exists(self._postgresql_base_conf): + if 'custom_conf' not in self.config and not os.path.exists(self._postgresql_base_conf): os.rename(self._postgresql_conf, self._postgresql_base_conf) with open(self._postgresql_conf, 'w') as f: From 3f7fa4b41f143282c48548d5d4561e830154d357 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 2 Sep 2016 17:00:37 +0200 Subject: [PATCH 19/23] Avoid retries when syncing replication slots. (#282) * Avoid retries when syncing replication slots. Do not retry postgres queries that fetch, create and drop slots at the end of the HA cycle. The complete run_cycle routine executes with the async_executor lock. This lock is also used with scheduling operations like reinit or restart in different threads. Looks like CPython threading class has fairness issues when multiple threads try to acquire the same lock and one of them executes long-running actions while holding it: the others have little chances of acquiring the lock in order. To get around this issue, the long action (i.e. retrying the query) is removed. Investigation by Ants Aasma and Alexander Kukushkin. --- patroni/__init__.py | 4 ++++ patroni/ha.py | 4 +--- patroni/postgresql.py | 17 +++++++++-------- tests/test_postgresql.py | 6 ++---- 4 files changed, 16 insertions(+), 15 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 37a8e848..38cde421 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -84,6 +84,10 @@ class Patroni(object): nap_time = self.next_run - current_time if nap_time <= 0: self.next_run = current_time + # Release the GIL so we don't starve anyone waiting on async_executor lock + time.sleep(0.001) + # Warn user that Patroni is not keeping up + logger.warning("Loop time exceeded, rescheduling immediately.") elif self.dcs.watch(nap_time): self.next_run = time.time() diff --git a/patroni/ha.py b/patroni/ha.py index 93567eed..233b7f9d 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -571,9 +571,7 @@ class Ha(object): # try to start dead postgres if not self.state_handler.is_healthy(): - msg = self.recover() - if msg is not None: - return msg + return self.recover() try: if self.cluster.is_unlocked(): diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 0eec2daa..dc76be03 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -890,7 +890,7 @@ $$""".format(name, ' '.join(options)), name, password, password) def load_replication_slots(self): if self.use_slots and self._schedule_load_slots: - cursor = self.query("SELECT slot_name FROM pg_replication_slots WHERE slot_type='physical'") + cursor = self._query("SELECT slot_name FROM pg_replication_slots WHERE slot_type='physical'") self._replication_slots = [r[0] for r in cursor] self._schedule_load_slots = False @@ -930,19 +930,20 @@ $$""".format(name, ' '.join(options)), name, password, password) # drop unused slots for slot in set(self._replication_slots) - slots: - self.query("""SELECT pg_drop_replication_slot(%s) - WHERE EXISTS(SELECT 1 FROM pg_replication_slots - WHERE slot_name = %s AND NOT active)""", slot, slot) + self._query("""SELECT pg_drop_replication_slot(%s) + WHERE EXISTS(SELECT 1 FROM pg_replication_slots + WHERE slot_name = %s AND NOT active)""", slot, slot) # create new slots for slot in slots - set(self._replication_slots): - self.query("""SELECT pg_create_physical_replication_slot(%s) - WHERE NOT EXISTS (SELECT 1 FROM pg_replication_slots - WHERE slot_name = %s)""", slot, slot) + self._query("""SELECT pg_create_physical_replication_slot(%s) + WHERE NOT EXISTS (SELECT 1 FROM pg_replication_slots + WHERE slot_name = %s)""", slot, slot) self._replication_slots = slots - except psycopg2.Error: + except Exception: logger.exception('Exception when changing replication slots') + self._schedule_load_slots = True def last_operation(self): return str(self.xlog_position()) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 95a251a2..f8f15b42 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -315,11 +315,9 @@ class TestPostgresql(unittest.TestCase): def test_sync_replication_slots(self): self.p.start() cluster = Cluster(True, None, self.leader, 0, [self.me, self.other, self.leadermem], None) + with mock.patch('patroni.postgresql.Postgresql._query', Mock(side_effect=psycopg2.OperationalError)): + self.p.sync_replication_slots(cluster) self.p.sync_replication_slots(cluster) - self.p.query = Mock(side_effect=psycopg2.OperationalError) - self.p.schedule_load_slots = True - self.p.sync_replication_slots(cluster) - self.p.schedule_load_slots = False with mock.patch('patroni.postgresql.Postgresql.role', new_callable=PropertyMock(return_value='replica')): self.p.sync_replication_slots(cluster) with mock.patch('patroni.postgresql.logger.error', new_callable=Mock()) as errorlog_mock: From 5ba1294d604b1df58f3c12db551a51684670b58d Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Fri, 2 Sep 2016 16:52:51 +0200 Subject: [PATCH 20/23] Fix tests for wal-e restore --- tests/test_wale_restore.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/tests/test_wale_restore.py b/tests/test_wale_restore.py index 9e15dc9b..6f93be63 100644 --- a/tests/test_wale_restore.py +++ b/tests/test_wale_restore.py @@ -10,26 +10,25 @@ def fake_backup_data(self, *args, **kwargs): """ return the fake result of WAL-E backup-list""" return """name last_modified expanded_size_bytes wal_segment_backup_start wal_segment_offset_backup_start wal_segment_backup_stop wal_segment_offset_backup_stop base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z 167772160 00000001000000000000007F 00000040 00000001000000000000007F 00000240 -""" - +""".encode('utf-8') def fake_backup_data_2(self, *args, **kwargs): """ return the fake result of WAL-E backup-list""" - return """name last_modified expanded_size_bytes wal_segment_backup_start wal_segment_offset_backup_start wal_segment_backup_stop wal_segment_offset_backup_stop """ + return """name last_modified expanded_size_bytes wal_segment_backup_start wal_segment_offset_backup_start wal_segment_backup_stop wal_segment_offset_backup_stop """.encode('utf-8') def fake_backup_data_3(self, *args, **kwargs): """ return the fake result of WAL-E backup-list""" return """name last_modified expanded_size_bytes wal_segment_backup_start wal_segment_offset_backup_start wal_segment_backup_stop base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z 167772160 00000001000000000000007F 00000040 00000001000000000000007F 00000240 -""" +""".encode('utf-8') def fake_backup_data_4(self, *args, **kwargs): """ return the fake result of WAL-E backup-list""" return """name last_modified expanded_size_foo wal_segment_backup_start wal_segment_offset_backup_start wal_segment_backup_stop wal_segment_offset_backup_stop base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z 167772160 00000001000000000000007F 00000040 00000001000000000000007F 00000240 -""" +""".encode('utf-8') @patch('os.access', MagicMock(return_value=True)) From 57a0ac90861e85ae6c1ff0667539dac289e3492e Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 5 Sep 2016 12:14:37 +0200 Subject: [PATCH 21/23] pep8 format of test_wale_restore.py --- tests/test_wale_restore.py | 37 ++++++++++--------------------------- 1 file changed, 10 insertions(+), 27 deletions(-) diff --git a/tests/test_wale_restore.py b/tests/test_wale_restore.py index 6f93be63..4e7080e0 100644 --- a/tests/test_wale_restore.py +++ b/tests/test_wale_restore.py @@ -6,29 +6,10 @@ from mock import MagicMock, patch, PropertyMock from patroni.scripts.wale_restore import WALERestore, main as _main -def fake_backup_data(self, *args, **kwargs): - """ return the fake result of WAL-E backup-list""" - return """name last_modified expanded_size_bytes wal_segment_backup_start wal_segment_offset_backup_start wal_segment_backup_stop wal_segment_offset_backup_stop -base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z 167772160 00000001000000000000007F 00000040 00000001000000000000007F 00000240 -""".encode('utf-8') - -def fake_backup_data_2(self, *args, **kwargs): - """ return the fake result of WAL-E backup-list""" - return """name last_modified expanded_size_bytes wal_segment_backup_start wal_segment_offset_backup_start wal_segment_backup_stop wal_segment_offset_backup_stop """.encode('utf-8') - - -def fake_backup_data_3(self, *args, **kwargs): - """ return the fake result of WAL-E backup-list""" - return """name last_modified expanded_size_bytes wal_segment_backup_start wal_segment_offset_backup_start wal_segment_backup_stop -base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z 167772160 00000001000000000000007F 00000040 00000001000000000000007F 00000240 -""".encode('utf-8') - - -def fake_backup_data_4(self, *args, **kwargs): - """ return the fake result of WAL-E backup-list""" - return """name last_modified expanded_size_foo wal_segment_backup_start wal_segment_offset_backup_start wal_segment_backup_stop wal_segment_offset_backup_stop -base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z 167772160 00000001000000000000007F 00000040 00000001000000000000007F 00000240 -""".encode('utf-8') +wale_output = b'name last_modified expanded_size_bytes wal_segment_backup_start ' +\ + b'wal_segment_offset_backup_start wal_segment_backup_stop wal_segment_offset_backup_stop\n' +\ + b'base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z 167772160 ' +\ + b'00000001000000000000007F 00000040 00000001000000000000007F 00000240\n' @patch('os.access', MagicMock(return_value=True)) @@ -38,7 +19,7 @@ base_00000001000000000000007F_00000040 2015-05-18T10:13:25.000Z 167772160 000 @patch('psycopg2.extensions.cursor', MagicMock(autospec=True)) @patch('psycopg2.extensions.connection', MagicMock(autospec=True)) @patch('psycopg2.connect', MagicMock(autospec=True)) -@patch('subprocess.check_output', MagicMock(side_effect=fake_backup_data)) +@patch('subprocess.check_output', MagicMock(return_value=wale_output)) class TestWALERestore(unittest.TestCase): def setUp(self): @@ -49,11 +30,13 @@ class TestWALERestore(unittest.TestCase): self.assertFalse(self.wale_restore.should_use_s3_to_create_replica()) with patch('subprocess.check_output', MagicMock(side_effect=subprocess.CalledProcessError(1, "cmd", "foo"))): self.assertFalse(self.wale_restore.should_use_s3_to_create_replica()) - with patch('subprocess.check_output', MagicMock(side_effect=fake_backup_data_2)): + with patch('subprocess.check_output', MagicMock(return_value=wale_output.split(b'\n')[0])): self.assertFalse(self.wale_restore.should_use_s3_to_create_replica()) - with patch('subprocess.check_output', MagicMock(side_effect=fake_backup_data_3)): + with patch('subprocess.check_output', + MagicMock(return_value=wale_output.replace(b' wal_segment_offset_backup_stop', b''))): self.assertFalse(self.wale_restore.should_use_s3_to_create_replica()) - with patch('subprocess.check_output', MagicMock(side_effect=fake_backup_data_4)): + with patch('subprocess.check_output', + MagicMock(return_value=wale_output.replace(b'expanded_size_bytes', b'expanded_size_foo'))): self.assertFalse(self.wale_restore.should_use_s3_to_create_replica()) self.wale_restore.should_use_s3_to_create_replica() From 5c7efa3a65dce0155887e3f1334412b46875730a Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 5 Sep 2016 14:10:21 +0200 Subject: [PATCH 22/23] Update gitignore --- .gitignore | 49 ++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 42 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index c03acb2b..236f02ea 100644 --- a/.gitignore +++ b/.gitignore @@ -1,13 +1,48 @@ -data/* -*.pyc -*.egg/ -*.egg-info/ +*.py[cod] + +# vi(m) swap files: +*.sw? + +# C extensions +*.so + +# Packages .cache/ +*.egg +*.eggs +*.egg-info +dist +build +eggs +parts +bin +var +sdist +develop-eggs +.installed.cfg +lib +lib64 + +# Installer logs +pip-log.txt + +# Unit test / coverage reports .coverage -.eggs/ -build/ -dist/ +.tox +nosetests.xml coverage.xml +htmlcov junit.xml +features/output +dummy + +# Translations +*.mo + +# Mr Developer +.mr.developer.cfg +.project +.pydevproject + pgpass scm-source.json From 2086c90a4a32f25ce2ff6aff32a21592f6b47be0 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Mon, 5 Sep 2016 14:11:53 +0200 Subject: [PATCH 23/23] Try to get rid from hardcoded names when building binary --- mkbinary.sh | 2 +- patroni.spec | 14 ++++++++++++-- patroni/dcs/__init__.py | 19 +++++++++++-------- requirements-bin.txt | 2 -- tests/test_patroni.py | 7 +++++++ 5 files changed, 31 insertions(+), 13 deletions(-) delete mode 100644 requirements-bin.txt diff --git a/mkbinary.sh b/mkbinary.sh index 67829074..86cf8215 100755 --- a/mkbinary.sh +++ b/mkbinary.sh @@ -1,5 +1,5 @@ #!/bin/sh set -e -pip install --ignore-installed -r requirements-bin.txt +pip install --ignore-installed setuptools==19.2 pyinstaller pyinstaller --clean --onefile patroni.spec diff --git a/patroni.spec b/patroni.spec index 8c5c5347..2afe8eac 100644 --- a/patroni.spec +++ b/patroni.spec @@ -3,11 +3,21 @@ block_cipher = None -a = Analysis(['patroni/__main__.py', 'patroni/dcs/consul.py', 'patroni/dcs/etcd.py', 'patroni/dcs/exhibitor.py', 'patroni/dcs/zookeeper.py'], +def hiddenimports(): + import sys + sys.path.insert(0, '.') + try: + import patroni.dcs + return patroni.dcs.dcs_modules() + finally: + sys.path.pop(0) + + +a = Analysis(['patroni/__main__.py'], pathex=[], binaries=None, datas=None, - hiddenimports=['patroni.dcs.consul', 'patroni.dcs.etcd', 'patroni.dcs.exhibitor', 'patroni.dcs.zookeeper'], + hiddenimports=hiddenimports(), hookspath=[], runtime_hooks=[], excludes=[], diff --git a/patroni/dcs/__init__.py b/patroni/dcs/__init__.py index 0045083c..733efbb0 100644 --- a/patroni/dcs/__init__.py +++ b/patroni/dcs/__init__.py @@ -34,26 +34,29 @@ def parse_connection_string(value): def dcs_modules(): """Get names of DCS modules, depending on execution environment. If being packaged with PyInstaller, - modules aren't discoverable dynamically by scanning source directory. Thus, when running in bundle, - a predefined list of dcs modules is returned. See: - https://pyinstaller.readthedocs.io/en/stable/runtime-information.html#run-time-information""" + modules aren't discoverable dynamically by scanning source directory because `FrozenImporter` doesn't + implement `iter_modules` method. But it is still possible to find all potential DCS modules by + iterating through `toc`, which contains list of all "frozen" resources.""" + + dcs_dirname = os.path.dirname(__file__) + module_prefix = __package__ + '.' if getattr(sys, 'frozen', False): - return ['consul', 'etcd', 'zookeeper', 'exhibitor'] + importer = pkgutil.get_importer(dcs_dirname) + return [module for module in list(importer.toc) if module.startswith(module_prefix) and module.count('.') == 2] else: - module_names = (name for _, name, is_pkg in pkgutil.iter_modules([os.path.dirname(__file__)]) if not is_pkg) - return module_names + return [module_prefix + name for _, name, is_pkg in pkgutil.iter_modules([dcs_dirname]) if not is_pkg] def get_dcs(config): available_implementations = set() for module_name in dcs_modules(): - module = importlib.import_module(__package__ + '.' + module_name) + module = importlib.import_module(module_name) for name in filter(lambda name: not name.startswith('__'), dir(module)): # iterate through module content value = getattr(module, name) name = name.lower() # try to find implementation of AbstractDCS interface, class name must match with module_name - if inspect.isclass(value) and issubclass(value, AbstractDCS) and name == module_name: + if inspect.isclass(value) and issubclass(value, AbstractDCS) and __package__ + '.' + name == module_name: available_implementations.add(name) if name in config: # which has configuration section in the config file # propagate some parameters diff --git a/requirements-bin.txt b/requirements-bin.txt deleted file mode 100644 index 7e43a3fd..00000000 --- a/requirements-bin.txt +++ /dev/null @@ -1,2 +0,0 @@ -setuptools==19.2 -pyinstaller diff --git a/tests/test_patroni.py b/tests/test_patroni.py index f9f135cc..d9c0e93a 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -14,6 +14,11 @@ from test_etcd import SleepException, etcd_read, etcd_write from test_postgresql import Postgresql, psycopg2_connect +class MockFrozenImporter(object): + + toc = set(['patroni.dcs.etcd']) + + @patch('time.sleep', Mock()) @patch('subprocess.call', Mock(return_value=0)) @patch('psycopg2.connect', psycopg2_connect) @@ -27,6 +32,8 @@ from test_postgresql import Postgresql, psycopg2_connect @patch.object(etcd.Client, 'read', etcd_read) class TestPatroni(unittest.TestCase): + @patch('pkgutil.get_importer', Mock(return_value=MockFrozenImporter())) + @patch('sys.frozen', Mock(return_value=True), create=True) @patch.object(etcd.Client, 'read', etcd_read) def setUp(self): RestApiServer._BaseServer__is_shut_down = Mock()