From a4af9f2a4cb7670c4d1881366804cc47f682925e Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 11 Dec 2015 18:54:03 +0100 Subject: [PATCH 01/39] Add replicafrom tag. --- patroni/__init__.py | 4 ++++ patroni/dcs.py | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/patroni/__init__.py b/patroni/__init__.py index 045d0d59..028142e9 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -29,6 +29,10 @@ class Patroni: def nofailover(self): return self.tags.get('nofailover', False) + @property + def replicatefrom(self): + return self.tags.get('replicatefrom') + @staticmethod def get_dcs(name, config): if 'etcd' in config: diff --git a/patroni/dcs.py b/patroni/dcs.py index f640787c..85fda68c 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -67,6 +67,10 @@ class Member(namedtuple('Member', 'index,name,session,data')): def nofailover(self): return self.data.get('tags', {}).get('nofailover', False) + @property + def replicatefrom(self): + return self.data.get('tags', {}).get('replicatefrom') + class Leader(namedtuple('Leader', 'index,session,member')): From 39cbd5f1d69d34eb870cd8d365ff6f269c0dd954 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 11 Dec 2015 18:54:38 +0100 Subject: [PATCH 02/39] Unify all follow the leader calls from eventloop. Call normal follow the leader method from HA even during recovery. This provides a single place that changes recovery.conf, making it easier to plug in a cascading replica in the future. Remove an obsolete demote function from PostreSQL module, modified the tests. --- patroni/ha.py | 45 ++++++++++++++++++++++------------------ patroni/postgresql.py | 3 --- tests/test_ha.py | 8 ++++++- tests/test_postgresql.py | 2 -- 4 files changed, 32 insertions(+), 26 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 3a8d46d7..0a2072aa 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -19,6 +19,7 @@ class Ha: self.dcs = patroni.dcs self.cluster = None self.old_cluster = None + self.recovering = False self._async_executor = AsyncExecutor() def load_cluster_from_dcs(self): @@ -94,33 +95,21 @@ class Ha: return 'waiting for leader to bootstrap' def recover(self): - has_lock = self.has_lock() - # try to see if we are the former master that crashed. If so - we likely need to run pg_rewind # in order to join the former standby being promoted. pg_controldata = self.state_handler.controldata() - if not has_lock and pg_controldata and\ + if (self.state_handler.role == 'master') and pg_controldata and\ pg_controldata.get('Database cluster state', '') == 'in production': # crashed master self.state_handler.require_rewind() + self.recovering = True + return self.follow_the_leader("started as readonly because i had the session lock", + "started as a secondary", + refresh=True, recovery=True) - # XXX: follow the leader calls stop, which might take quite some time. - # perhaps we should run sync asynchronously - # (we still need the exit code from follow_the_leader) - ret = self.state_handler.follow_the_leader(None if has_lock else self.cluster.leader, recovery=True) - if not ret: - if not has_lock: - return 'failed to start postgres' - self.dcs.delete_leader() - self.dcs.reset_cluster() - return 'removed leader key after trying and failing to start postgres' - if not has_lock: - return 'started as a secondary' - logger.info('started as readonly because i had the session lock') - self.load_cluster_from_dcs() - - def follow_the_leader(self, demote_reason, follow_reason, refresh=True): + def follow_the_leader(self, demote_reason, follow_reason, refresh=True, recovery=False): refresh and self.load_cluster_from_dcs() - ret = demote_reason if self.state_handler.is_leader() else follow_reason + ret = demote_reason if (not recovery and self.state_handler.is_leader() + or recovery and self.state_handler.role == 'master') else follow_reason leader = self.cluster.leader leader = None if (leader and leader.name) == self.state_handler.name else leader if not self.state_handler.check_recovery_conf(leader): @@ -382,6 +371,15 @@ class Ha: # so even 1 << 32 would have 10 digits. return str(sysid) and len(str(sysid)) >= 10 and str(sysid).isdigit() + def post_recover(self): + if not self.state_handler.is_running(): + if self.has_lock(): + self.dcs.delete_leader() + self.dcs.reset_cluster() + return 'removed leader key after trying and failing to start postgres' + return 'failed to start postgres' + return None + def _run_cycle(self): try: self.load_cluster_from_dcs() @@ -395,6 +393,13 @@ class Ha: if self._async_executor.busy: return self.handle_long_action_in_progress() + # we've go here, so async action has finished. Check if we tried to recover and failed + if self.recovering: + self.recovering = False + msg = self.post_recover() + if msg is not None: + return msg + # currently it can trigger only reinitialize msg = self.process_scheduled_action() if msg is not None: diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 45b871c2..84223675 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -596,9 +596,6 @@ recovery_target_timeline = 'latest' self.call_nowait(ACTION_ON_ROLE_CHANGE) return ret - def demote(self): - self.follow_the_leader(None) - def create_or_update_role(self, name, password, options): self.query("""DO $$ BEGIN diff --git a/tests/test_ha.py b/tests/test_ha.py index d9a408a4..bcc5c93d 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -127,13 +127,18 @@ class TestHa(unittest.TestCase): def test_recover_replica_failed(self): self.p.controldata = lambda: {'Database cluster state': 'in production'} self.p.is_healthy = false + self.p.is_running = false self.p.follow_the_leader = false + self.assertEquals(self.ha.run_cycle(), 'started as a secondary') self.assertEquals(self.ha.run_cycle(), 'failed to start postgres') def test_recover_master_failed(self): self.p.follow_the_leader = false self.p.is_healthy = false + self.p.is_running = false self.ha.has_lock = true + self.p.role = 'master' + self.assertEquals(self.ha.run_cycle(), 'started as readonly because i had the session lock') self.assertEquals(self.ha.run_cycle(), 'removed leader key after trying and failing to start postgres') @patch('sys.exit', return_value=1) @@ -144,7 +149,8 @@ class TestHa(unittest.TestCase): @patch.object(Cluster, 'is_unlocked', Mock(return_value=False)) def test_start_as_readonly(self): - self.p.is_leader = self.p.is_healthy = false + self.p.is_leader = false + self.p.is_healthy = true self.ha.has_lock = true self.assertEquals(self.ha.run_cycle(), 'promoted self to leader because i had the session lock') diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 2097bcb7..a4dae962 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -243,9 +243,7 @@ class TestPostgresql(unittest.TestCase): @patch('patroni.postgresql.Postgresql.single_user_mode', MagicMock(return_value=1)) @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict())) def test_follow_the_leader(self, mock_pg_rewind): - self.p.demote() self.p.follow_the_leader(None) - self.p.demote() self.p.follow_the_leader(self.leader) self.p.follow_the_leader(Leader(-1, 28, self.other)) self.p.rewind = mock_pg_rewind From 64e09f7ca780aa2128309c9b63c7228b4a02eae9 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 15:29:05 +0100 Subject: [PATCH 03/39] Patronictl: Prettier error messages by inheriting from ClickException --- patroni/exceptions.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/patroni/exceptions.py b/patroni/exceptions.py index 43f54e7f..97b88238 100644 --- a/patroni/exceptions.py +++ b/patroni/exceptions.py @@ -1,3 +1,5 @@ +from click import ClickException + class PatroniException(Exception): """Parent class for all kind of exceptions related to selected distributed configuration store""" @@ -13,7 +15,7 @@ class PatroniException(Exception): return repr(self.value) -class PatroniCtlException(Exception): +class PatroniCtlException(ClickException): pass From 47007c333159c245537673294d24926d355fd1c9 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 15:30:11 +0100 Subject: [PATCH 04/39] Dockerfile: Ensure all python packages are available and patronictl is configured --- Dockerfile | 9 +++++++-- docker/entrypoint.sh | 5 +++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 84b9cdf6..b9c3b9ff 100644 --- a/Dockerfile +++ b/Dockerfile @@ -14,14 +14,19 @@ RUN apt-get upgrade -y ENV PGVERSION 9.4 RUN apt-get install python python-yaml python-requests python-boto postgresql-${PGVERSION} python-dnspython python-kazoo python-pip -y -RUN apt-get install python-dev postgresql-server-dev-${PGVERSION} -y -RUN pip install python-etcd psycopg2 +RUN apt-get install python-dev postgresql-server-dev-${PGVERSION} python-prettytable -y +ADD requirements-py2.txt /tmp/ +RUN pip install -r /tmp/requirements-py2.txt ENV PATH /usr/lib/postgresql/${PGVERSION}/bin:$PATH ADD patroni.py /patroni.py +ADD patronictl.py /patronictl.py ADD patroni/ /patroni +RUN ln -s /patroni.py /usr/local/bin/patroni +RUN ln -s /patronictl.py /usr/local/bin/patronictl + ENV ETCDVERSION 2.0.13 RUN curl -L https://github.com/coreos/etcd/releases/download/v${ETCDVERSION}/etcd-v${ETCDVERSION}-linux-amd64.tar.gz | tar xz -C /bin --strip=1 --wildcards --no-anchored etcd etcdctl diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 9edb2120..31d76e8b 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -79,6 +79,11 @@ then ETCD_CLUSTER="127.0.0.1:4001" fi +mkdir -p ~postgres/.config/patroni +cat > ~postgres/.config/patroni/patronictl.yaml <<__EOF__ +{dcs_api: 'etcd://${ETCD_CLUSTER}', namespace: /service/} +__EOF__ + cat > /patroni/postgres.yml <<__EOF__ ttl: &ttl 30 From 42e07148017f2751b65c2213ddfbe006766b657e Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 15:30:49 +0100 Subject: [PATCH 05/39] Patronictl: Allow specification of dbname and user, as well as password prompting. --- patroni/ctl.py | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index 3d457c9f..5da78388 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -252,6 +252,8 @@ def dsn(cluster_name, config_file, dcs, role, member): @option_format @click.option('--format', help='Output format (pretty, json)', default='tsv') @click.option('--file', '-f', help='Execute the SQL commands from this file', type=click.File('rb')) +@click.option('--password', help='force password prompt', is_flag=True) +@click.option('-U', '--username', help='database user name', type=str) @option_dcs @option_watch @option_watchrefresh @@ -260,6 +262,7 @@ def dsn(cluster_name, config_file, dcs, role, member): @click.option('--member', '-m', help='Query a specific member', type=str) @click.option('--delimiter', help='The column delimiter', default='\t') @click.option('--command', '-c', help='The SQL commands to execute') +@click.option('-d', '--dbname', help='database name to connect to', type=str) def query( cluster_name, config_file, @@ -271,6 +274,9 @@ def query( delimiter, command, file, + password, + username, + dbname, format='tsv', ): if role is not None and member is not None: @@ -281,6 +287,17 @@ def query( if file is not None and command is not None: raise PatroniCtlException('--file and --command are mutually exclusive options') + if file is None and command is None: + raise PatroniCtlException('You need to specify either --command or --file') + + connect_parameters = dict() + if username: + connect_parameters['user'] = username + if password: + connect_parameters['password'] = click.prompt('Password', hide_input=True, type=str) + if dbname: + connect_parameters['database'] = dbname + if file is not None: command = file.read() @@ -289,17 +306,17 @@ def query( cursor = None for _ in watching(w, watch, clear=False): - output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command) + output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command, connect_parameters=connect_parameters) print_output(None, output, format=format, delimiter=delimiter) if cursor is None: cluster = dcs.get_cluster() -def query_member(cluster, cursor, member, role, command): +def query_member(cluster, cursor, member, role, command, connect_parameters): try: if cursor is None: - cursor = get_cursor(cluster, role=role, member=member) + cursor = get_cursor(cluster, role=role, member=member, connect_parameters=connect_parameters) if cursor is None: if role is None: From 6568c56c8585bd36e11ac599550f787bdce79302 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 16:17:18 +0100 Subject: [PATCH 06/39] Patronictl: Add tests to increase coverage, fix regression issue. --- patroni/ctl.py | 5 +++-- patroni/exceptions.py | 1 + tests/test_ctl.py | 45 ++++++++++++++++++++++++++----------------- tests/test_ha.py | 2 +- 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index 5da78388..1ddb1625 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -306,14 +306,15 @@ def query( cursor = None for _ in watching(w, watch, clear=False): - output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command, connect_parameters=connect_parameters) + output, cursor = query_member(cluster=cluster, cursor=cursor, member=member, role=role, command=command, + connect_parameters=connect_parameters) print_output(None, output, format=format, delimiter=delimiter) if cursor is None: cluster = dcs.get_cluster() -def query_member(cluster, cursor, member, role, command, connect_parameters): +def query_member(cluster, cursor, member, role, command, connect_parameters=dict()): try: if cursor is None: cursor = get_cursor(cluster, role=role, member=member, connect_parameters=connect_parameters) diff --git a/patroni/exceptions.py b/patroni/exceptions.py index 97b88238..d07e6426 100644 --- a/patroni/exceptions.py +++ b/patroni/exceptions.py @@ -1,5 +1,6 @@ from click import ClickException + class PatroniException(Exception): """Parent class for all kind of exceptions related to selected distributed configuration store""" diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 0d65a05c..a2226b7a 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -102,35 +102,35 @@ y''') result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other N''') - assert 'Aborting failover' in str(result.exception) + assert 'Aborting failover' in str(result.output) result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader leader y''') - assert 'target and source are the same' in str(result.exception) + assert 'target and source are the same' in str(result.output) result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader Reality y''') - assert 'Reality does not exist' in str(result.exception) + assert 'Reality does not exist' in str(result.output) result = runner.invoke(ctl, ['failover', 'dummy', '--force']) assert 'Failing over to new leader' in result.output result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='dummy') - assert 'is not the leader of cluster' in str(result.exception) + assert 'is not the leader of cluster' in str(result.output) with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_only_leader())): result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other y''') - assert 'No candidates found to failover to' in str(result.exception) + assert 'No candidates found to failover to' in str(result.output) with patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_without_leader())): result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader other y''') - assert 'This cluster has no master' in str(result.exception) + assert 'This cluster has no master' in str(result.output) with patch('patroni.ctl.post_patroni', Mock(side_effect=Exception())): result = runner.invoke(ctl, ['failover', 'dummy', '--dcs', '8.8.8.8'], input='''leader @@ -149,13 +149,13 @@ y''') # with patch('patroni.dcs.AbstractDCS.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())): # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='nonsense') -# assert 'is not the leader of cluster' in str(result.exception) +# assert 'is not the leader of cluster' in str(result.output) # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8', '--master', 'nonsense']) - # assert 'is not the leader of cluster' in str(result.exception) + # assert 'is not the leader of cluster' in str(result.output) # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nn') - # assert 'Aborting failover' in str(result.exception) + # assert 'Aborting failover' in str(result.output) # with patch('patroni.ctl.wait_for_leader', Mock(return_value = get_cluster_initialized_with_leader())): # result = runner.invoke(ctl, ['failover', 'alpha', '--dcs', '8.8.8.8'], input='leader\nother\nY') @@ -181,13 +181,19 @@ y''') '--role', 'master', ]) - assert 'mutually exclusive' in str(result.exception) + assert 'mutually exclusive' in str(result.output) with runner.isolated_filesystem(): dummy_file = open('dummy', 'w') dummy_file.write('SELECT 1') dummy_file.close() + result = runner.invoke(ctl, [ + 'query', + 'alpha' + ]) + assert 'You need to specify' in str(result.output) + result = runner.invoke(ctl, [ 'query', 'alpha', @@ -196,7 +202,7 @@ y''') '--command', 'dummy', ]) - assert 'mutually exclusive' in str(result.exception) + assert 'mutually exclusive' in str(result.output) result = runner.invoke(ctl, ['query', 'alpha', '--file', 'dummy']) @@ -205,6 +211,9 @@ y''') result = runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1']) assert 'mock column' in result.output + result = runner.invoke(ctl, ['query', 'alpha', '--command', 'SELECT 1', '--dbname', 'dummy', '--password', '--username', 'dummy'], input='password\n') + assert 'mock column' in result.output + @patch('patroni.ctl.get_cursor', Mock(return_value=MockConnect().cursor())) def test_query_member(self): rows = query_member(None, None, None, 'master', 'SELECT pg_is_in_recovery()') @@ -242,10 +251,10 @@ y''') '--member', 'dummy', ]) - assert 'mutually exclusive' in str(result.exception) + assert 'mutually exclusive' in str(result.output) result = runner.invoke(ctl, ['dsn', 'alpha', '--member', 'dummy']) - assert 'Can not find' in str(result.exception) + assert 'Can not find' in str(result.output) # result = runner.invoke(ctl, ['dsn', 'alpha', '--dcs', '8.8.8.8', '--role', 'replica']) # assert 'host=127.0.0.1 port=5436' in result.output @@ -269,7 +278,7 @@ y''') 'dummy', '--any', ], input='y') - assert 'not a member' in str(result.exception) + assert 'not a member' in str(result.output) with patch('requests.post', Mock(return_value=MockResponse())): result = runner.invoke(ctl, ['restart', 'alpha', '--dcs', '8.8.8.8'], input='y') @@ -282,15 +291,15 @@ y''') result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='alpha\nslave') assert 'Please confirm' in result.output assert 'You are about to remove all' in result.output - assert 'You did not exactly type' in str(result.exception) + assert 'You did not exactly type' in str(result.output) result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='''alpha Yes I am aware slave''') - assert 'You did not specify the current master of the cluster' in str(result.exception) + assert 'You did not specify the current master of the cluster' in str(result.output) result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], input='beta\nleader') - assert 'Cluster names specified do not match' in str(result.exception) + assert 'Cluster names specified do not match' in str(result.output) with patch('patroni.etcd.Etcd.get_cluster', get_cluster_initialized_with_leader): result = runner.invoke(ctl, ['remove', 'alpha', '--dcs', '8.8.8.8'], @@ -304,7 +313,7 @@ leader''') input='''alpha Yes I am aware leader''') - assert 'We have not implemented this for DCS of type' in str(result.exception) + assert 'We have not implemented this for DCS of type' in str(result.output) @patch('patroni.etcd.Etcd.watch', Mock(return_value=None)) @patch('patroni.etcd.Etcd.get_cluster', Mock(return_value=get_cluster_initialized_with_leader())) diff --git a/tests/test_ha.py b/tests/test_ha.py index d9a408a4..5a3f5f9a 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -18,7 +18,7 @@ def false(*args, **kwargs): def get_cluster(initialize, leader, members, failover): - return Cluster(initialize, leader, None, members, failover) + return Cluster(initialize, leader, 10, members, failover) def get_cluster_not_initialized_without_leader(): From feac841aadef08edf7e5dfd6d1b6e91f0411fc69 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Tue, 22 Dec 2015 19:48:35 +0100 Subject: [PATCH 07/39] Docker: Install python packages via pip only, yaml consistency --- Dockerfile | 4 ++-- docker/entrypoint.sh | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Dockerfile b/Dockerfile index b9c3b9ff..97995128 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,8 +13,8 @@ RUN apt-get update -y RUN apt-get upgrade -y ENV PGVERSION 9.4 -RUN apt-get install python python-yaml python-requests python-boto postgresql-${PGVERSION} python-dnspython python-kazoo python-pip -y -RUN apt-get install python-dev postgresql-server-dev-${PGVERSION} python-prettytable -y +RUN apt-get install postgresql-server-dev-${PGVERSION} -y +RUN apt-get install python-pip python-dev -y ADD requirements-py2.txt /tmp/ RUN pip install -r /tmp/requirements-py2.txt diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 31d76e8b..b549cc46 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -84,7 +84,7 @@ cat > ~postgres/.config/patroni/patronictl.yaml <<__EOF__ {dcs_api: 'etcd://${ETCD_CLUSTER}', namespace: /service/} __EOF__ -cat > /patroni/postgres.yml <<__EOF__ +cat > /patroni/postgres.yaml <<__EOF__ ttl: &ttl 30 loop_wait: &loop_wait 10 @@ -131,7 +131,7 @@ postgresql: hot_standby: "on" __EOF__ -cat /patroni/postgres.yml +cat /patroni/postgres.yaml if [ ! -z $CHEAT ] then @@ -140,5 +140,5 @@ then sleep 60 done else - exec python /patroni.py /patroni/postgres.yml + exec python /patroni.py /patroni/postgres.yaml fi From a64c7abdcc07c5cd78501a0a7a19072d8c74bcac Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Wed, 23 Dec 2015 09:13:23 +0100 Subject: [PATCH 08/39] Bugfix: Fixing python-etcd version, as behaviour has changed in newer version. Our current master branch doesn't pass the code coverage test, due to behaviour changes in upstream python-etcd. As a bandaid, fix the version for now. Reference build fail: https://travis-ci.org/zalando/patroni/jobs/98470121 --- requirements-py2.txt | 2 +- requirements-py3.txt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/requirements-py2.txt b/requirements-py2.txt index f8eacc12..23193254 100644 --- a/requirements-py2.txt +++ b/requirements-py2.txt @@ -6,6 +6,6 @@ PyYAML requests six >= 1.7 kazoo>=2.2.1 -python-etcd>=0.4.1 +python-etcd==0.4.1 click>=4.1 prettytable>=0.7 diff --git a/requirements-py3.txt b/requirements-py3.txt index 30b5ce96..3e2df5f1 100644 --- a/requirements-py3.txt +++ b/requirements-py3.txt @@ -6,6 +6,6 @@ PyYAML requests six kazoo>=2.2.1 -python-etcd>=0.4.1 +python-etcd==0.4.1 click>=4.1 prettytable>=0.7 From bf52fa6f570208f5ddc5b2c0779c2eb43ffac732 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 30 Dec 2015 16:05:06 +0100 Subject: [PATCH 09/39] follow_the_leader unconditionally during recovery. Otherwise, we may 'forget' to start the crashed node. This fixes the regression from the former behavior introduced in the previous commit. --- patroni/ha.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 0a2072aa..af1b62b1 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -112,9 +112,9 @@ class Ha: or recovery and self.state_handler.role == 'master') else follow_reason leader = self.cluster.leader leader = None if (leader and leader.name) == self.state_handler.name else leader - if not self.state_handler.check_recovery_conf(leader): + if not self.state_handler.check_recovery_conf(leader) or recovery: self._async_executor.schedule('changing primary_conninfo and restarting') - self._async_executor.run_async(self.state_handler.follow_the_leader, (leader, )) + self._async_executor.run_async(self.state_handler.follow_the_leader, (leader, recovery)) return ret def enforce_master_role(self, message, promote_message): From c650dc092e4e46909baf8cb64d3071884e049cbe Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 30 Dec 2015 18:33:23 +0100 Subject: [PATCH 10/39] Follow the node in the replicatefrom if present. Rename the follow_the_leader to just follow, since the node to be followed is not necessary a leader anymore. Extend the code that manages replication slots to the non-master nodes if they are mentioned in at least one replicatefrom tag. Add the 3rd configuration in order to be able to run cascading replicas. --- patroni/ha.py | 44 ++++++++++------- patroni/postgresql.py | 8 +++- postgres0.yml | 5 +- postgres1.yml | 5 +- postgres2.yml | 101 +++++++++++++++++++++++++++++++++++++++ tests/test_ha.py | 9 ++-- tests/test_postgresql.py | 16 +++---- 7 files changed, 152 insertions(+), 36 deletions(-) create mode 100644 postgres2.yml diff --git a/patroni/ha.py b/patroni/ha.py index af1b62b1..3856366d 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -102,19 +102,25 @@ class Ha: pg_controldata.get('Database cluster state', '') == 'in production': # crashed master self.state_handler.require_rewind() self.recovering = True - return self.follow_the_leader("started as readonly because i had the session lock", - "started as a secondary", - refresh=True, recovery=True) + return self.follow("started as readonly because i had the session lock", + "started as a secondary", + refresh=True, recovery=True) - def follow_the_leader(self, demote_reason, follow_reason, refresh=True, recovery=False): + def follow(self, demote_reason, follow_reason, refresh=True, recovery=False): refresh and self.load_cluster_from_dcs() ret = demote_reason if (not recovery and self.state_handler.is_leader() or recovery and self.state_handler.role == 'master') else follow_reason - leader = self.cluster.leader - leader = None if (leader and leader.name) == self.state_handler.name else leader - if not self.state_handler.check_recovery_conf(leader) or recovery: + # determine the node to follow. If replicatefrom tag is set, + # try to follow the node mentioned there, otherwise, follow the leader. + if self.patroni.replicatefrom: + node_to_follow = [m for m in self.cluster.members if m.name == self.patroni.replicatefrom] + node_to_follow = node_to_follow[0] if node_to_follow else self.cluster.leader + else: + node_to_follow = self.cluster.leader + node_to_follow = None if (node_to_follow and node_to_follow.name) == self.state_handler.name else node_to_follow + if not self.state_handler.check_recovery_conf(node_to_follow) or recovery: self._async_executor.schedule('changing primary_conninfo and restarting') - self._async_executor.run_async(self.state_handler.follow_the_leader, (leader, recovery)) + self._async_executor.run_async(self.state_handler.follow, (node_to_follow, recovery)) return ret def enforce_master_role(self, message, promote_message): @@ -287,14 +293,14 @@ class Ha: return self.enforce_master_role('acquired session lock as a leader', 'promoted self to leader by acquiring session lock') else: - return self.follow_the_leader('demoted self due after trying and failing to obtain lock', - 'following new leader after trying and failing to obtain lock') + return self.follow('demoted self due after trying and failing to obtain lock', + 'following new leader after trying and failing to obtain lock') else: if self.patroni.nofailover: - return self.follow_the_leader('demoting self because I am not allowed to become master', - 'following a different leader because I am not allowed to promote') - return self.follow_the_leader('demoting self because i am not the healthiest node', - 'following a different leader because i am not the healthiest node') + return self.follow('demoting self because I am not allowed to become master', + 'following a different leader because I am not allowed to promote') + return self.follow('demoting self because i am not the healthiest node', + 'following a different leader because i am not the healthiest node') def process_healthy_cluster(self): if self.has_lock(): @@ -312,8 +318,8 @@ class Ha: self.load_cluster_from_dcs() else: logger.info('does not have lock') - return self.follow_the_leader('demoting self because i do not have the lock and i was a leader', - 'no action. i am a secondary and i am following a leader', False) + return self.follow('demoting self because i do not have the lock and i was a leader', + 'no action. i am a secondary and i am following a leader', False) def schedule(self, action): with self._async_executor: @@ -430,7 +436,11 @@ class Ha: else: return self.process_healthy_cluster() finally: - self.state_handler.sync_replication_slots(self.cluster) + # we might not have a valid PostgreSQL connection here if another thread + # stops PostgreSQL, therefore, we only reload replication slots if no + # asyncrhonous processes are running (should be always the case for the master) + if not self._async_executor.busy: + self.state_handler.sync_replication_slots(self.cluster) except DCSError: logger.error('Error communicating with DCS') if self.state_handler.is_running() and self.state_handler.is_leader(): diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 84223675..db47cbff 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -524,7 +524,7 @@ recovery_target_timeline = 'latest' except: logger.exception("Unable to remove {}".format(path)) - def follow_the_leader(self, leader, recovery=False): + def follow(self, leader, recovery=False): if not self.check_recovery_conf(leader) or recovery: change_role = (self.role == 'master') @@ -634,7 +634,11 @@ $$""".format(name, options), name, password, password) if self.use_slots: try: self.load_replication_slots() - slots = [m.name for m in cluster.members if m.name != self.name] if self.role == 'master' else [] + if self.role == 'master': + slots = [m.name for m in cluster.members if m.name != self.name] + else: + # only manage slots for replicas that want to replicate from this one + slots = [m.name for m in cluster.members if m.replicatefrom == self.name] # drop unused slots for slot in set(self.replication_slots) - set(slots): self.query("""SELECT pg_drop_replication_slot(%s) diff --git a/postgres0.yml b/postgres0.yml index 36018ad5..f29e1fee 100644 --- a/postgres0.yml +++ b/postgres0.yml @@ -88,14 +88,13 @@ postgresql: archive_mode: "on" wal_level: hot_standby archive_command: mkdir -p ../wal_archive && test ! -f ../wal_archive/%f && cp %p ../wal_archive/%f - max_wal_senders: 5 + max_wal_senders: 10 wal_keep_segments: 8 archive_timeout: 1800s - max_replication_slots: 5 + max_replication_slots: 10 hot_standby: "on" wal_log_hints: "on" tags: nofailover: False noloadbalance: False clonefrom: False - replicatefrom: 127.0.0.1 diff --git a/postgres1.yml b/postgres1.yml index e1b61b3b..1e7a7045 100644 --- a/postgres1.yml +++ b/postgres1.yml @@ -88,14 +88,13 @@ postgresql: archive_mode: "on" wal_level: hot_standby archive_command: mkdir -p ../wal_archive && test ! -f ../wal_archive/%f && cp %p ../wal_archive/%f - max_wal_senders: 5 + max_wal_senders: 10 wal_keep_segments: 8 archive_timeout: 1800s - max_replication_slots: 5 + max_replication_slots: 10 hot_standby: "on" wal_log_hints: "on" tags: nofailover: False noloadbalance: False clonefrom: False - replicatefrom: 127.0.0.1 diff --git a/postgres2.yml b/postgres2.yml new file mode 100644 index 00000000..99e8b47e --- /dev/null +++ b/postgres2.yml @@ -0,0 +1,101 @@ +ttl: &ttl 30 +loop_wait: &loop_wait 10 +scope: &scope batman +restapi: + listen: 127.0.0.1:8010 + connect_address: 127.0.0.1:8010 + auth: 'username:password' +# certfile: /etc/ssl/certs/ssl-cert-snakeoil.pem +# keyfile: /etc/ssl/private/ssl-cert-snakeoil.key +etcd: + scope: *scope + ttl: *ttl + host: 127.0.0.1:4001 + #discovery_srv: my-etcd.domain +#zookeeper: +# scope: *scope +# session_timeout: *ttl +# reconnect_timeout: *loop_wait +# hosts: +# - 127.0.0.1:2181 +# - 127.0.0.2:2181 +# exhibitor: +# poll_interval: 300 +# port: 8181 +# hosts: +# - host1 +# - host2 +# - host3 +postgresql: + name: postgresql2 + scope: *scope + listen: 127.0.0.1:5434 + connect_address: 127.0.0.1:5434 + data_dir: data/postgresql2 + maximum_lag_on_failover: 1048576 # 1 megabyte in bytes + use_slots: True + pgpass: /tmp/pgpass2 + initdb: ## We allow the following options to be passed on to initdb + # - auth: authmethod + # - auth-host: authmethod + # - auth-local: authmethod + - encoding: UTF8 + # - data-checksums # When pg_rewind is needed on 9.3, this needs to be enabled + # - locale: locale + # - lc-collate: locale + # - lc-ctype: locale + # - lc-messages: locale + # - lc-monetary: locale + # - lc-numeric: locale + # - lc-time: locale + # - text-search-config: CFG + # - xlogdir: directory + # - debug + # - noclean + pg_rewind: + username: postgres + password: zalando + pg_hba: + - host all all 0.0.0.0/0 md5 + - hostssl all all 0.0.0.0/0 md5 + replication: + username: replicator + password: rep-pass + network: 127.0.0.1/32 + superuser: + user: postgres + password: zalando + admin: + username: admin + password: admin +# commented-out example for wal-e provisioning + create_replica_method: + - basebackup +# - wal_e +# commented-out example for wal-e provisioning + #wal_e: + #command: /patroni/scripts/wale_restore.py + #env_dir: /home/postgres/etc/wal-e.d/env + #threshold_megabytes: 10240 + #threshold_backup_size_percentage: 30 + #retries: 2 + #use_iam: 1 + #recovery_conf: + #restore_command: envdir /etc/wal-e.d/env wal-e wal-fetch "%f" "%p" -p 1 + recovery_conf: + restore_command: cp ../wal_archive/%f %p + parameters: + archive_mode: "on" + wal_level: hot_standby + archive_command: mkdir -p ../wal_archive && test ! -f ../wal_archive/%f && cp %p ../wal_archive/%f + max_wal_senders: 10 + wal_keep_segments: 8 + archive_timeout: 1800s + max_replication_slots: 10 + hot_standby: "on" + wal_log_hints: "on" +tags: + nofailover: False + noloadbalance: False + clonefrom: False + replicatefrom: postgresql1 diff --git a/tests/test_ha.py b/tests/test_ha.py index bcc5c93d..336b78a3 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -88,6 +88,7 @@ class MockPatroni: self.api = Mock() self.tags = {} self.nofailover = None + self.replicatefrom = None self.api.connection_string = 'http://127.0.0.1:8008' @@ -128,12 +129,12 @@ class TestHa(unittest.TestCase): self.p.controldata = lambda: {'Database cluster state': 'in production'} self.p.is_healthy = false self.p.is_running = false - self.p.follow_the_leader = false + self.p.follow = false self.assertEquals(self.ha.run_cycle(), 'started as a secondary') self.assertEquals(self.ha.run_cycle(), 'failed to start postgres') def test_recover_master_failed(self): - self.p.follow_the_leader = false + self.p.follow = false self.p.is_healthy = false self.p.is_running = false self.ha.has_lock = true @@ -202,10 +203,12 @@ class TestHa(unittest.TestCase): self.ha.update_lock = false self.assertEquals(self.ha.run_cycle(), 'demoting self because i do not have the lock and i was a leader') - def test_follow_the_leader(self): + def test_follow(self): self.ha.cluster.is_unlocked = false self.p.is_leader = false self.assertEquals(self.ha.run_cycle(), 'no action. i am a secondary and i am following a leader') + self.ha.patroni.replicatefrom = "foo" + self.assertEquals(self.ha.run_cycle(), 'no action. i am a secondary and i am following a leader') def test_no_etcd_connection_master_demote(self): self.ha.load_cluster_from_dcs = Mock(side_effect=DCSError('Etcd is not responding properly')) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index a4dae962..8114272b 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -242,23 +242,23 @@ class TestPostgresql(unittest.TestCase): @patch('patroni.postgresql.Postgresql.remove_data_directory', MagicMock(return_value=True)) @patch('patroni.postgresql.Postgresql.single_user_mode', MagicMock(return_value=1)) @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict())) - def test_follow_the_leader(self, mock_pg_rewind): - self.p.follow_the_leader(None) - self.p.follow_the_leader(self.leader) - self.p.follow_the_leader(Leader(-1, 28, self.other)) + def test_follow(self, mock_pg_rewind): + self.p.follow(None) + self.p.follow(self.leader) + self.p.follow(Leader(-1, 28, self.other)) self.p.rewind = mock_pg_rewind - self.p.follow_the_leader(self.leader) + self.p.follow(self.leader) self.p.require_rewind() with mock.patch('os.path.islink', MagicMock(return_value=True)): with mock.patch('patroni.postgresql.Postgresql.can_rewind', new_callable=PropertyMock(return_value=True)): with mock.patch('os.unlink', MagicMock(return_value=True)): - self.p.follow_the_leader(self.leader, recovery=True) + self.p.follow(self.leader, recovery=True) self.p.require_rewind() with mock.patch('patroni.postgresql.Postgresql.can_rewind', new_callable=PropertyMock(return_value=True)): self.p.rewind.return_value = True - self.p.follow_the_leader(self.leader, recovery=True) + self.p.follow(self.leader, recovery=True) self.p.rewind.return_value = False - self.p.follow_the_leader(self.leader, recovery=True) + self.p.follow(self.leader, recovery=True) def test_can_rewind(self): tmp = self.p.pg_rewind From 70bae1b267db4cf76d3ade299a3bef6741ec2b89 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 8 Jan 2016 16:51:24 +0100 Subject: [PATCH 11/39] Note multiple PostgreSQL listen addresses. --- README.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.rst b/README.rst index 422e0849..d8068b46 100644 --- a/README.rst +++ b/README.rst @@ -77,7 +77,8 @@ For an example file, see ``postgres0.yml``. Regarding settings: - *postgresql*: - *name*: the name of the Postgres host. Must be unique for the cluster. - - *listen*: IP address + port that Postgres listens to; must be accessible from other nodes in the cluster, if you're using streaming replication. + - *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``. The first address from this list will be used by Patroni to establish local connections to the PostgreSQL node. + - *connect\_address*: IP address + port through which Postgres is accessible from other nodes and applications. - *data\_dir*: file path to initialize and store Postgres data files. - *maximum\_lag\_on\_failover*: the maximum bytes a follower may lag. From 15bec1e28c0ca2ca82015fa8fa06b3ae5c8a9c94 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 26 Jan 2016 15:24:32 +0100 Subject: [PATCH 12/39] Support new replicas without the master. The replication method should have no_master flag set and support getting the base backup from some external storage (i.e. S3). At the moment we only support initialization of replicas in the already existing cluster when no master is present, since there is no 'one fits all' way to decide whether to run initidb or wait for the replica data storage to become available when dealing with the new cluster. --- patroni/ha.py | 9 ++++-- patroni/postgresql.py | 57 ++++++++++++++++++++++++--------- patroni/scripts/wale_restore.py | 37 ++++++++++++--------- tests/test_ha.py | 16 +++++++++ tests/test_postgresql.py | 23 ++++++++++++- tests/test_wale_restore.py | 4 ++- 6 files changed, 111 insertions(+), 35 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 3856366d..0feb172b 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -63,12 +63,12 @@ class Ha: self.dcs.touch_member(json.dumps(data, separators=(',', ':'))) def copy_backup_from_leader(self, leader): - if self.state_handler.bootstrap(leader): - logger.info('bootstrapped from leader') + if self.state_handler.bootstrap(True, leader): + logger.info('bootstrapped from leader' if leader else 'bootstrapped without leader') else: self.state_handler.stop('immediate') self.state_handler.remove_data_directory() - logger.error('failed to bootstrap from leader') + logger.error('failed to bootstrap from leader' if leader else 'failed to bootstrap (without leader)') def bootstrap(self): if not self.cluster.is_unlocked(): # cluster already has leader @@ -92,6 +92,9 @@ class Ha: else: return 'failed to acquire initialize lock' else: + if self.state_handler.can_create_replica_without_leader(): + self._async_executor.run_async(self.copy_backup_from_leader, args=(None, )) + return "trying to bootstrap without leader" return 'waiting for leader to bootstrap' def recover(self): diff --git a/patroni/postgresql.py b/patroni/postgresql.py index db47cbff..f329fad5 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -220,9 +220,9 @@ class Postgresql: return env def sync_from_leader(self, leader): - r = parseurl(leader.conn_url) - - env = self.write_pgpass(r) + if leader: + r = parseurl(leader.conn_url) + env = self.write_pgpass(r) if leader else os.environ.copy() ret = self.create_replica(leader, env) == 0 ret and self.delete_trigger_file() return ret @@ -235,14 +235,30 @@ class Postgresql: """ return ' '.join('{}={}'.format(param, val) for param, val in sorted(conn.items())) + def replica_method_can_work_without_leader(self, method): + return method != 'basebackup' and self.config and self.config.get(method, {}).get('no_master') + + def can_create_replica_without_leader(self): + """ go through the replication methods to see if there are ones + that does not require a running leader to create the replica. + """ + replica_methods = self.config.get('create_replica_method', []) + for replica_method in replica_methods: + if self.replica_method_can_work_without_leader(replica_method): + return True + return False + def create_replica(self, leader, env): # create the replica according to the replica_method # defined by the user. this is a list, so we need to # loop through all methods the user supplies - connstring = leader.conn_url + connstring = leader.conn_url if leader else "" # get list of replica methods from config. # If there is no configuration key, or no value is specified, use basebackup replica_methods = self.config.get('create_replica_method') or ['basebackup'] + # if we don't have any leader, leave only replica methods that work without it + replica_methods = [r for r in replica_methods if self.replica_method_can_work_without_leader(r)] if not leader \ + else replica_methods # go through them in priority order ret = 1 for replica_method in replica_methods: @@ -435,7 +451,7 @@ class Postgresql: return pattern and (pattern in line) return not pattern - def write_recovery_conf(self, leader): + def write_recovery_conf(self, leader, bootstrap=False): with open(self.recovery_conf, 'w') as f: f.write("""standby_mode = 'on' recovery_target_timeline = 'latest' @@ -444,6 +460,7 @@ recovery_target_timeline = 'latest' f.write("""primary_conninfo = '{}'\n""".format(self.primary_conninfo(leader.conn_url))) if self.use_slots: f.write("""primary_slot_name = '{}'\n""".format(self.name)) + if (leader and leader.conn_url) or bootstrap: for name, value in self.config.get('recovery_conf', {}).items(): f.write("{} = '{}'\n".format(name, value)) @@ -658,18 +675,28 @@ $$""".format(name, options), name, password, password) def last_operation(self): return str(self.xlog_position()) - def bootstrap(self, current_leader=None): + def bootstrap(self, initialize=False, current_leader=None): """ - Initially bootstrap PostgreSQL, either by creating a data - directory with initdb, or by initalizing a replica from an - exiting leader. Failure in the first case always leads to - exception, since there is no point in continuing if initdb failed. - In the second case, however, a False is returned on failure, since - it is normal for the replica to retry a failed attempt to initialize - from the master. + Populate PostgreSQL data directory by doing one of the following: + - create with initdb if there is no master. + - initialize the replica from an existing master + - initialize the replica using the replica creation method that + works without the master (i.e. restore from on-disk base backup) + + The choice between the last 2 is triggered by the initialize flag. + We should never try to initdb an already initialized cluster, nor + try to bootstrap the cluster that lacks the initialize key from from + the master-less replica creation method (in the latter case, there is + no clear inidicator of the moment we should abandon our attempts and + swich to initdb). + + Failure during initdb always leads to an exception, since there is + no point in continuing if initdb fails. For the rest of the cases, + the function returns False in order to inidicate a failed attempt + that should be retried in the future. """ ret = False - if not current_leader: + if not (initialize or current_leader): ret = self.initialize() and self.start() if ret: self.create_replication_user() @@ -679,7 +706,7 @@ $$""".format(name, options), name, password, password) else: if self.sync_from_leader(current_leader): self.restore_configuration_files() - self.write_recovery_conf(current_leader) + self.write_recovery_conf(current_leader, True) ret = self.start() return ret diff --git a/patroni/scripts/wale_restore.py b/patroni/scripts/wale_restore.py index f54c37a3..58fdd420 100755 --- a/patroni/scripts/wale_restore.py +++ b/patroni/scripts/wale_restore.py @@ -42,7 +42,7 @@ logger = logging.getLogger(__name__) class WALERestore(object): - def __init__(self, scope, datadir, connstring, env_dir, threshold_mb, threshold_pct, use_iam): + def __init__(self, scope, datadir, connstring, env_dir, threshold_mb, threshold_pct, use_iam, no_master): self.scope = scope self.master_connection = connstring self.data_dir = datadir @@ -51,6 +51,7 @@ class WALERestore(object): self.wal_e.threshold_mb = threshold_mb self.wal_e.threshold_pct = threshold_pct self.wal_e.iam_string = ' --aws-instance-profile ' if use_iam == 1 else '' + self.no_master = no_master self.wal_e.cmd = 'envdir {0} wal-e {1} '.format(self.wal_e.dir, self.wal_e.iam_string) self.init_error = (not os.path.exists(self.wal_e.dir)) @@ -109,19 +110,23 @@ class WALERestore(object): conn = None cursor = None diff_in_bytes = long(backup_size) - try: - # get the difference in bytes between the current WAL location and the backup start offset - conn = psycopg2.connect(self.master_connection) - conn.autocommit = True - cursor = conn.cursor() - cursor.execute("SELECT pg_xlog_location_diff(pg_current_xlog_location(), %s)", (backup_start_lsn,)) - diff_in_bytes = long(cursor.fetchone()[0]) - except psycopg2.Error as e: - logger.error('could not determine difference with the master location: {}'.format(e)) - return False - finally: - cursor and cursor.close() - conn and conn.close() + if not self.no_master: + try: + # get the difference in bytes between the current WAL location and the backup start offset + conn = psycopg2.connect(self.master_connection) + conn.autocommit = True + cursor = conn.cursor() + cursor.execute("SELECT pg_xlog_location_diff(pg_current_xlog_location(), %s)", (backup_start_lsn,)) + diff_in_bytes = long(cursor.fetchone()[0]) + except psycopg2.Error as e: + logger.error('could not determine difference with the master location: {}'.format(e)) + return False + finally: + cursor and cursor.close() + conn and conn.close() + else: + # always try to use WAL-E if base backup is available + diff_in_bytes = 0 # if the size of the accumulated WAL segments is more than a certan percentage of the backup size # or exceeds the pre-determined size - pg_basebackup is chosen instead. @@ -150,13 +155,15 @@ def main(): parser.add_argument('--threshold_megabytes', type=int, default=10240) parser.add_argument('--threshold_backup_size_percentage', type=int, default=30) parser.add_argument('--use_iam', type=int, default=0) + parser.add_argument('--no_master', type=int, default=0) args = parser.parse_args() # retry cloning in a loop for retry in range(0, args.retries + 1): restore = WALERestore(scope=args.scope, datadir=args.datadir, connstring=args.connstring, env_dir=args.envdir, threshold_mb=args.threshold_megabytes, - threshold_pct=args.threshold_backup_size_percentage, use_iam=args.use_iam) + threshold_pct=args.threshold_backup_size_percentage, use_iam=args.use_iam, + no_master=args.no_master) ret = restore.run() if ret == 0: break diff --git a/tests/test_ha.py b/tests/test_ha.py index 336b78a3..4c8500f4 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -103,6 +103,7 @@ class TestHa(unittest.TestCase): def setUp(self, mock_machines): mock_machines.__get__ = Mock(return_value=['http://remotehost:2379']) self.p = MockPostgresql() + self.p.can_create_replica_without_leader = MagicMock(return_value=False) self.e = Etcd('foo', {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'}) self.e.client.read = etcd_read self.e.client.write = etcd_write @@ -139,6 +140,7 @@ class TestHa(unittest.TestCase): self.p.is_running = false self.ha.has_lock = true self.p.role = 'master' + self.p.controldata = lambda: {'Database cluster state': 'in production'} self.assertEquals(self.ha.run_cycle(), 'started as readonly because i had the session lock') self.assertEquals(self.ha.run_cycle(), 'removed leader key after trying and failing to start postgres') @@ -223,6 +225,11 @@ class TestHa(unittest.TestCase): self.ha.cluster = get_cluster_initialized_without_leader() self.assertEquals(self.ha.bootstrap(), 'waiting for leader to bootstrap') + def test_bootstrap_without_leader(self): + self.ha.cluster = get_cluster_initialized_without_leader() + self.p.can_create_replica_without_leader = MagicMock(return_value=True) + self.assertEquals(self.ha.bootstrap(), "trying to bootstrap without leader") + def test_bootstrap_initialize_lock_failed(self): self.ha.cluster = get_cluster_not_initialized_without_leader() self.assertEquals(self.ha.bootstrap(), 'failed to acquire initialize lock') @@ -342,3 +349,12 @@ class TestHa(unittest.TestCase): self.ha.fetch_node_status(member) member = Member(0, 'test', 1, {'api_url': 'http://localhost:8011/patroni'}) self.ha.fetch_node_status(member) + + def test_post_recover(self): + self.p.is_running = false + self.ha.has_lock = true + self.assertEqual(self.ha.post_recover(), 'removed leader key after trying and failing to start postgres') + self.ha.has_lock = false + self.assertEqual(self.ha.post_recover(), 'failed to start postgres') + self.p.is_running = true + self.assertIsNone(self.ha.post_recover()) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 8114272b..3681562f 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -259,6 +259,8 @@ class TestPostgresql(unittest.TestCase): self.p.follow(self.leader, recovery=True) self.p.rewind.return_value = False self.p.follow(self.leader, recovery=True) + with mock.patch('patroni.postgresql.Postgresql.check_recovery_conf', MagicMock(return_value=True)): + self.assertTrue(self.p.follow(None)) def test_can_rewind(self): tmp = self.p.pg_rewind @@ -305,6 +307,9 @@ class TestPostgresql(unittest.TestCase): 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) @patch.object(MockConnect, 'closed', 2) def test__query(self): @@ -364,7 +369,8 @@ class TestPostgresql(unittest.TestCase): with patch('subprocess.call', Mock(return_value=1)): self.assertRaises(PostgresException, self.p.bootstrap) self.p.bootstrap() - self.p.bootstrap(self.leader) + with patch('patroni.postgresql.Postgresql.sync_from_leader', MagicMock(return_value=True)): + self.p.bootstrap(self.leader) def test_remove_data_directory(self): self.p.data_dir = 'data_dir' @@ -478,3 +484,18 @@ class TestPostgresql(unittest.TestCase): def test_restore_configuration_files(self, mock_copy): shutil.copy = mock_copy self.p.restore_configuration_files() + + def test_can_create_replica_without_leader(self): + self.p.config['create_replica_method'] = [] + self.assertFalse(self.p.can_create_replica_without_leader()) + self.p.config['create_replica_method'] = ['wale', 'basebackup'] + self.p.config['wale'] = {'command': 'foo', 'no_master': 1} + self.assertTrue(self.p.can_create_replica_without_leader()) + + def test_replica_method_can_work_without_leader(self): + self.assertFalse(self.p.replica_method_can_work_without_leader('basebackup')) + self.assertFalse(self.p.replica_method_can_work_without_leader('foobar')) + self.p.config['foo'] = {'command': 'bar', 'no_master': 1} + self.assertTrue(self.p.replica_method_can_work_without_leader('foo')) + self.p.config['foo'] = {'command': 'bar'} + self.assertFalse(self.p.replica_method_can_work_without_leader('foo')) diff --git a/tests/test_wale_restore.py b/tests/test_wale_restore.py index 05f34187..ba4c3bea 100644 --- a/tests/test_wale_restore.py +++ b/tests/test_wale_restore.py @@ -58,7 +58,7 @@ class TestWALERestore(unittest.TestCase): def setUp(self): self.wale_restore = WALERestore("batman", "/data", - "host=batman port=5432 user=batman", "/etc", 100, 100, 1) + "host=batman port=5432 user=batman", "/etc", 100, 100, 1, 0) def tearDown(self): pass @@ -76,6 +76,8 @@ class TestWALERestore(unittest.TestCase): self.assertFalse(self.wale_restore.should_use_s3_to_create_replica()) self.wale_restore.should_use_s3_to_create_replica() + self.wale_restore.no_master = 1 + self.assertTrue(self.wale_restore.should_use_s3_to_create_replica()) def test_create_replica_with_s3(self): with patch('subprocess.call', MagicMock(return_value=0)): From 34437550d41013236ceabbb6d7cff75231b1e75b Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 26 Jan 2016 15:40:53 +0100 Subject: [PATCH 13/39] Fix a new flake8 warning (line break before the binary operator) --- patroni/ha.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 0feb172b..453ec03e 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -111,8 +111,8 @@ class Ha: def follow(self, demote_reason, follow_reason, refresh=True, recovery=False): refresh and self.load_cluster_from_dcs() - ret = demote_reason if (not recovery and self.state_handler.is_leader() - or recovery and self.state_handler.role == 'master') else follow_reason + ret = demote_reason if (not recovery and self.state_handler.is_leader() or + recovery and self.state_handler.role == 'master') else follow_reason # determine the node to follow. If replicatefrom tag is set, # try to follow the node mentioned there, otherwise, follow the leader. if self.patroni.replicatefrom: From aa350b71394fe24ddd8691f16d035f1e4b1ae3e6 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 26 Jan 2016 15:57:23 +0100 Subject: [PATCH 14/39] Increase the tests coverage. --- tests/test_patroni.py | 5 +++++ tests/test_wale_restore.py | 6 +++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/tests/test_patroni.py b/tests/test_patroni.py index 18f5c14b..d1db99cb 100644 --- a/tests/test_patroni.py +++ b/tests/test_patroni.py @@ -80,3 +80,8 @@ class TestPatroni(unittest.TestCase): self.assertTrue(self.p.nofailover) self.p.tags['nofailover'] = None self.assertFalse(self.p.nofailover) + + def test_replicatefrom(self): + self.assertIsNone(self.p.replicatefrom) + self.p.tags['replicatefrom'] = 'foo' + self.assertEqual(self.p.replicatefrom, 'foo') diff --git a/tests/test_wale_restore.py b/tests/test_wale_restore.py index ba4c3bea..f353a4c9 100644 --- a/tests/test_wale_restore.py +++ b/tests/test_wale_restore.py @@ -3,7 +3,7 @@ from mock import MagicMock, patch, PropertyMock import os import psycopg2 import subprocess -from patroni.scripts.wale_restore import WALERestore +from patroni.scripts.wale_restore import WALERestore, main def fake_cursor_fetchone(*args, **kwargs): @@ -91,3 +91,7 @@ class TestWALERestore(unittest.TestCase): with patch.object(self.wale_restore, 'should_use_s3_to_create_replica', MagicMock(return_value=True)): with patch.object(self.wale_restore, 'create_replica_with_s3', MagicMock(return_value=0)): self.assertEqual(self.wale_restore.run(), 0) + + def test_main(self): + with patch('sys.exit', MagicMock(return_value=0)): + main() From c9de062ef5de8121b40e930b5f5b46e6f670e992 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 26 Jan 2016 16:07:17 +0100 Subject: [PATCH 15/39] declare the test as a static method to make the code analyzing tool happy. --- tests/test_wale_restore.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_wale_restore.py b/tests/test_wale_restore.py index f353a4c9..e922a9db 100644 --- a/tests/test_wale_restore.py +++ b/tests/test_wale_restore.py @@ -92,6 +92,7 @@ class TestWALERestore(unittest.TestCase): with patch.object(self.wale_restore, 'create_replica_with_s3', MagicMock(return_value=0)): self.assertEqual(self.wale_restore.run(), 0) + @staticmethod def test_main(self): with patch('sys.exit', MagicMock(return_value=0)): main() From 72d30974ad2cdbbf11b332a437b369640c266bff Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 26 Jan 2016 16:22:28 +0100 Subject: [PATCH 16/39] Another attempt at making the Quantifiedcode happy. --- tests/test_wale_restore.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/test_wale_restore.py b/tests/test_wale_restore.py index e922a9db..aa834b96 100644 --- a/tests/test_wale_restore.py +++ b/tests/test_wale_restore.py @@ -92,7 +92,6 @@ class TestWALERestore(unittest.TestCase): with patch.object(self.wale_restore, 'create_replica_with_s3', MagicMock(return_value=0)): self.assertEqual(self.wale_restore.run(), 0) - @staticmethod def test_main(self): with patch('sys.exit', MagicMock(return_value=0)): - main() + self.assertEqual(main(), None) From d1e54174c718133352e5b98a7d4e4f80f4b3a29f Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 27 Jan 2016 13:13:52 +0100 Subject: [PATCH 17/39] Make the code slightly more readable. --- patroni/ha.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/ha.py b/patroni/ha.py index 453ec03e..4f35cd8c 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -63,7 +63,7 @@ class Ha: self.dcs.touch_member(json.dumps(data, separators=(',', ':'))) def copy_backup_from_leader(self, leader): - if self.state_handler.bootstrap(True, leader): + if self.state_handler.bootstrap(initialize=True, current_leader=leader): logger.info('bootstrapped from leader' if leader else 'bootstrapped without leader') else: self.state_handler.stop('immediate') From abaef496705c8972c8a4e569549cb64628bb0d6a Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Fri, 29 Jan 2016 09:05:52 +0100 Subject: [PATCH 18/39] Disable auth in order to use patronictl with the default configuration, remove obsolete replication_methods like. --- postgres0.yml | 3 +-- postgres1.yml | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/postgres0.yml b/postgres0.yml index 36018ad5..244a0f19 100644 --- a/postgres0.yml +++ b/postgres0.yml @@ -4,7 +4,7 @@ scope: &scope batman restapi: listen: 127.0.0.1:8008 connect_address: 127.0.0.1:8008 - auth: 'username:password' +# auth: 'username:password' # certfile: /etc/ssl/certs/ssl-cert-snakeoil.pem # keyfile: /etc/ssl/private/ssl-cert-snakeoil.key etcd: @@ -72,7 +72,6 @@ postgresql: - basebackup # - wal_e # commented-out example for wal-e provisioning - #create_replica_method: wal_e, basebackup #wal_e: #command: /patroni/scripts/wale_restore.py #env_dir: /etc/wal-e.d/env diff --git a/postgres1.yml b/postgres1.yml index e1b61b3b..58fc308c 100644 --- a/postgres1.yml +++ b/postgres1.yml @@ -4,7 +4,7 @@ scope: &scope batman restapi: listen: 127.0.0.1:8009 connect_address: 127.0.0.1:8009 - auth: 'username:password' +# auth: 'username:password' # certfile: /etc/ssl/certs/ssl-cert-snakeoil.pem # keyfile: /etc/ssl/private/ssl-cert-snakeoil.key etcd: From 704b29e6868eff28ffdea12d044e962b90d177e3 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Fri, 29 Jan 2016 12:55:08 +0100 Subject: [PATCH 19/39] Provide more context for healthchecks and monitoring. Include version numbers of both PostgreSQL and patroni for the /patroni endpoint. Scope is also returned. --- patroni/__init__.py | 2 ++ patroni/api.py | 2 ++ patroni/postgresql.py | 1 + 3 files changed, 5 insertions(+) diff --git a/patroni/__init__.py b/patroni/__init__.py index 045d0d59..1ded41aa 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -10,6 +10,7 @@ from patroni.ha import Ha from patroni.postgresql import Postgresql from patroni.utils import setup_signal_handlers, reap_children from patroni.zookeeper import ZooKeeper +from .version import __version__ logger = logging.getLogger(__name__) @@ -21,6 +22,7 @@ class Patroni: self.tags = config.get('tags', dict()) self.postgresql = Postgresql(config['postgresql']) self.dcs = self.get_dcs(self.postgresql.name, config) + self.version = __version__ self.api = RestApiServer(self, config['restapi']) self.ha = Ha(self) self.next_run = time.time() diff --git a/patroni/api.py b/patroni/api.py index 47416b10..2318fbf0 100644 --- a/patroni/api.py +++ b/patroni/api.py @@ -92,6 +92,7 @@ class RestApiHandler(BaseHTTPRequestHandler): def do_GET_patroni(self): response = self.get_postgresql_status(True) response.update(self.get_tags()) + response['patroni'] = {'version': self.server.patroni.version, 'scope': self.server.patroni.postgresql.scope} self.send_response(200) self.send_header('Content-Type', 'application/json') @@ -233,6 +234,7 @@ class RestApiHandler(BaseHTTPRequestHandler): 'state': self.server.patroni.postgresql.state, 'postmaster_start_time': row[0], 'role': 'replica' if row[1] else 'master', + 'server_version': self.server.patroni.postgresql.server_version, 'xlog': ({ 'received_location': row[3], 'replayed_location': row[4], diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 45b871c2..6e4ecddf 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -133,6 +133,7 @@ class Postgresql: r = parseurl('postgres://{}/postgres'.format(self.local_address)) self._connection = psycopg2.connect(**r) self._connection.autocommit = True + self.server_version = self._connection.server_version return self._connection def _cursor(self): From bce96df1777a9212e231cf709a40452e21cfb512 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Fri, 29 Jan 2016 13:29:51 +0100 Subject: [PATCH 20/39] Add attributes to Mocked classes --- tests/test_api.py | 3 +++ tests/test_etcd.py | 3 +++ tests/test_ha.py | 2 ++ 3 files changed, 8 insertions(+) diff --git a/tests/test_api.py b/tests/test_api.py index 013e5065..4b24704d 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -16,6 +16,8 @@ class MockPostgresql(Mock): name = 'test' state = 'running' role = 'master' + server_version = '999999' + scope = 'dummy' def connection(self): return psycopg2_connect() @@ -51,6 +53,7 @@ class MockPatroni: ha = MockHa() dcs = Mock() tags = {} + version = '0.00' class MockRequest: diff --git a/tests/test_etcd.py b/tests/test_etcd.py index e9a334c8..443dc2d8 100644 --- a/tests/test_etcd.py +++ b/tests/test_etcd.py @@ -40,6 +40,9 @@ class MockResponse: class MockPostgresql(Mock): + server_version = '999999' + scope = 'dummy' + def last_operation(self): return '0' diff --git a/tests/test_ha.py b/tests/test_ha.py index d9a408a4..9ff6547d 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -48,6 +48,8 @@ class MockPostgresql(Mock): role = 'replica' state = 'running' connection_string = 'postgres://foo@bar/postgres' + server_version = '999999' + scope = 'dummy' def is_healthy(self): return True From 2d457ae26af4e94f1c10919ce255751c5aaa9352 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Mon, 1 Feb 2016 11:37:59 +0100 Subject: [PATCH 21/39] Fix a problem with mutable default arguments. Also bump up the version of python-etcd in requirements to the latest one that that does not have https://github.com/jplana/python-etcd/issues/152 --- patroni/ctl.py | 12 +++++++++--- requirements-py2.txt | 2 +- requirements-py3.txt | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/patroni/ctl.py b/patroni/ctl.py index 1ddb1625..635b0a89 100644 --- a/patroni/ctl.py +++ b/patroni/ctl.py @@ -168,7 +168,9 @@ def watching(w, watch, max_count=None, clear=True): yield 0 -def build_connect_parameters(conn_url, connect_parameters={}): +def build_connect_parameters(conn_url, connect_parameters=None): + if connect_parameters is None: + connect_parameters = {} params = connect_parameters.copy() parsed = parseurl(conn_url) params['host'] = parsed['host'] @@ -200,7 +202,9 @@ def get_any_member(cluster, role='master', member=None): return None -def get_cursor(cluster, role='master', member=None, connect_parameters={}): +def get_cursor(cluster, role='master', member=None, connect_parameters=None): + if connect_parameters is None: + connect_parameters = {} member = get_any_member(cluster=cluster, role=role, member=member) if member is None: return None @@ -314,7 +318,9 @@ def query( cluster = dcs.get_cluster() -def query_member(cluster, cursor, member, role, command, connect_parameters=dict()): +def query_member(cluster, cursor, member, role, command, connect_parameters=None): + if connect_parameters is None: + connect_parameters = {} try: if cursor is None: cursor = get_cursor(cluster, role=role, member=member, connect_parameters=connect_parameters) diff --git a/requirements-py2.txt b/requirements-py2.txt index 23193254..1e194bc0 100644 --- a/requirements-py2.txt +++ b/requirements-py2.txt @@ -6,6 +6,6 @@ PyYAML requests six >= 1.7 kazoo>=2.2.1 -python-etcd==0.4.1 +python-etcd==0.4.2 click>=4.1 prettytable>=0.7 diff --git a/requirements-py3.txt b/requirements-py3.txt index 3e2df5f1..13c3010c 100644 --- a/requirements-py3.txt +++ b/requirements-py3.txt @@ -6,6 +6,6 @@ PyYAML requests six kazoo>=2.2.1 -python-etcd==0.4.1 +python-etcd==0.4.2 click>=4.1 prettytable>=0.7 From 1d689d1e27c894bb68518701c6381781b2a7ed1a Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Mon, 1 Feb 2016 12:54:23 +0100 Subject: [PATCH 22/39] Spelling --- patroni/ha.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 4f35cd8c..8974b815 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -25,7 +25,7 @@ class Ha: def load_cluster_from_dcs(self): cluster = self.dcs.get_cluster() - # We want to keep the state of cluster when it was healhy + # We want to keep the state of cluster when it was healthy if not cluster.is_unlocked() or not self.old_cluster: self.old_cluster = cluster self.cluster = cluster @@ -296,7 +296,7 @@ class Ha: return self.enforce_master_role('acquired session lock as a leader', 'promoted self to leader by acquiring session lock') else: - return self.follow('demoted self due after trying and failing to obtain lock', + return self.follow('demoted self after trying and failing to obtain lock', 'following new leader after trying and failing to obtain lock') else: if self.patroni.nofailover: @@ -402,7 +402,7 @@ class Ha: if self._async_executor.busy: return self.handle_long_action_in_progress() - # we've go here, so async action has finished. Check if we tried to recover and failed + # we've got here, so any async action has finished. Check if we tried to recover and failed if self.recovering: self.recovering = False msg = self.post_recover() @@ -441,7 +441,7 @@ class Ha: finally: # we might not have a valid PostgreSQL connection here if another thread # stops PostgreSQL, therefore, we only reload replication slots if no - # asyncrhonous processes are running (should be always the case for the master) + # asynchronous processes are running (should be always the case for the master) if not self._async_executor.busy: self.state_handler.sync_replication_slots(self.cluster) except DCSError: @@ -450,7 +450,7 @@ class Ha: self.demote(delete_leader=False) return 'demoted self because DCS is not accessible and i was a leader' except (psycopg2.Error, PostgresConnectionException): - logger.exception('Error communicating with Postgresql. Will try again later') + logger.exception('Error communicating with PostgreSQL. Will try again later') def run_cycle(self): with self._async_executor: From 1a8eaf8b936241c8b19ce4e1cfa09c69d835f5d2 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Mon, 1 Feb 2016 13:02:21 +0100 Subject: [PATCH 23/39] Spelling: Even spelling can be tested --- tests/test_ha.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_ha.py b/tests/test_ha.py index 4c8500f4..ec02de97 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -167,7 +167,7 @@ class TestHa(unittest.TestCase): def test_demote_after_failing_to_obtain_lock(self): self.ha.acquire_lock = false - self.assertEquals(self.ha.run_cycle(), 'demoted self due after trying and failing to obtain lock') + self.assertEquals(self.ha.run_cycle(), 'demoted self after trying and failing to obtain lock') def test_follow_new_leader_after_failing_to_obtain_lock(self): self.ha.is_healthiest_node = true From b724757b8aa894acd3482974b00685e2a168013d Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Mon, 1 Feb 2016 14:53:55 +0100 Subject: [PATCH 24/39] Bugfix: Ensure to inject the superuser username when connecting. When running patroni with a superuser different than the os-user, the connection was being established using the os-username. This fixes this. --- patroni/postgresql.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 6e4ecddf..403da667 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -131,6 +131,7 @@ class Postgresql: def connection(self): if not self._connection or self._connection.closed != 0: r = parseurl('postgres://{}/postgres'.format(self.local_address)) + r['user'] = self.superuser['user'] self._connection = psycopg2.connect(**r) self._connection.autocommit = True self.server_version = self._connection.server_version @@ -138,7 +139,7 @@ class Postgresql: def _cursor(self): if not self._cursor_holder or self._cursor_holder.closed or self._cursor_holder.connection.closed != 0: - logger.info("established a new patroni connection to the postgres cluster") + logger.info("establishing a new patroni connection to the postgres cluster") self._cursor_holder = self.connection().cursor() return self._cursor_holder From a5207e7d5795ee000057153e92686d4cd507bafb Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Mon, 1 Feb 2016 14:58:40 +0100 Subject: [PATCH 25/39] Superuser specification: only overwrite values if specified. --- patroni/postgresql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 403da667..5e2312cb 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -131,7 +131,7 @@ class Postgresql: def connection(self): if not self._connection or self._connection.closed != 0: r = parseurl('postgres://{}/postgres'.format(self.local_address)) - r['user'] = self.superuser['user'] + r.update(self.superuser) self._connection = psycopg2.connect(**r) self._connection.autocommit = True self.server_version = self._connection.server_version From 7db5ec1269f459b3f16c69c1949f4cf5bd2bfb44 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Wed, 3 Feb 2016 16:46:30 +0100 Subject: [PATCH 26/39] Revert global Docker changes --- Dockerfile | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/Dockerfile b/Dockerfile index 97995128..362c9bf4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -13,10 +13,9 @@ RUN apt-get update -y RUN apt-get upgrade -y ENV PGVERSION 9.4 -RUN apt-get install postgresql-server-dev-${PGVERSION} -y -RUN apt-get install python-pip python-dev -y -ADD requirements-py2.txt /tmp/ -RUN pip install -r /tmp/requirements-py2.txt +RUN apt-get install python python-yaml python-requests python-boto postgresql-${PGVERSION} python-dnspython python-kazoo python-pip -y +RUN apt-get install python-dev postgresql-server-dev-${PGVERSION} -y +RUN pip install python-etcd psycopg2 ENV PATH /usr/lib/postgresql/${PGVERSION}/bin:$PATH @@ -24,9 +23,6 @@ ADD patroni.py /patroni.py ADD patronictl.py /patronictl.py ADD patroni/ /patroni -RUN ln -s /patroni.py /usr/local/bin/patroni -RUN ln -s /patronictl.py /usr/local/bin/patronictl - ENV ETCDVERSION 2.0.13 RUN curl -L https://github.com/coreos/etcd/releases/download/v${ETCDVERSION}/etcd-v${ETCDVERSION}-linux-amd64.tar.gz | tar xz -C /bin --strip=1 --wildcards --no-anchored etcd etcdctl From af0db5916dd73276f84b1f157fda42c3d898f718 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Wed, 3 Feb 2016 16:53:47 +0100 Subject: [PATCH 27/39] Docker improvements. Latest greatest upstream (PostgreSQL 9.5, etcd 2.2.5) Install python packages using requirements file (vs installing from apt-get). --- Dockerfile | 14 +++++++++----- docker/entrypoint.sh | 3 ++- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/Dockerfile b/Dockerfile index 84b9cdf6..aea9ad97 100644 --- a/Dockerfile +++ b/Dockerfile @@ -12,17 +12,21 @@ RUN curl https://www.postgresql.org/media/keys/ACCC4CF8.asc | apt-key add - RUN apt-get update -y RUN apt-get upgrade -y -ENV PGVERSION 9.4 -RUN apt-get install python python-yaml python-requests python-boto postgresql-${PGVERSION} python-dnspython python-kazoo python-pip -y -RUN apt-get install python-dev postgresql-server-dev-${PGVERSION} -y -RUN pip install python-etcd psycopg2 +ENV PGVERSION 9.5 +RUN apt-get install postgresql-${PGVERSION} postgresql-server-dev-${PGVERSION} -y +RUN apt-get install python python-dev python-pip -y +ADD requirements-py2.txt /requirements-py2.txt +RUN pip install -r /requirements-py2.txt ENV PATH /usr/lib/postgresql/${PGVERSION}/bin:$PATH ADD patroni.py /patroni.py ADD patroni/ /patroni -ENV ETCDVERSION 2.0.13 +RUN ln -s /patroni.py /usr/local/bin/patroni +RUN ln -s /patronictl.py /usr/local/bin/patronictl + +ENV ETCDVERSION 2.2.5 RUN curl -L https://github.com/coreos/etcd/releases/download/v${ETCDVERSION}/etcd-v${ETCDVERSION}-linux-amd64.tar.gz | tar xz -C /bin --strip=1 --wildcards --no-anchored etcd etcdctl ### Setting up a simple script that will serve as an entrypoint diff --git a/docker/entrypoint.sh b/docker/entrypoint.sh index 9edb2120..71c6cb36 100755 --- a/docker/entrypoint.sh +++ b/docker/entrypoint.sh @@ -119,7 +119,8 @@ postgresql: archive_command: 'true' max_wal_senders: 20 listen_addresses: 0.0.0.0 - checkpoint_segments: 64 + max_wal_size: 1GB + min_wal_size: 128MB wal_keep_segments: 64 archive_timeout: 1800s max_replication_slots: 20 From 458f12f8a25deb7ccb512027fd32ad81e69afe8b Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 4 Feb 2016 12:24:31 +0100 Subject: [PATCH 28/39] Rename the badly named parameter. --- patroni/ha.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/ha.py b/patroni/ha.py index 8974b815..7ceac620 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -63,7 +63,7 @@ class Ha: self.dcs.touch_member(json.dumps(data, separators=(',', ':'))) def copy_backup_from_leader(self, leader): - if self.state_handler.bootstrap(initialize=True, current_leader=leader): + if self.state_handler.bootstrap(cluster_initialized=True, current_leader=leader): logger.info('bootstrapped from leader' if leader else 'bootstrapped without leader') else: self.state_handler.stop('immediate') From 1a87bbd830849ebe0b2062356e1ca3e1e65858f1 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 4 Feb 2016 15:31:22 +0100 Subject: [PATCH 29/39] Fix handling of replication slots on the master. Master shouldn't keep a replication slot for the members that replicate from other members instead of the master (replicatefrom). Otherwise, the master will keep collecting WAL segments that won't be requested ever. Of course, if the destination of replicatefrom is not part of the cluster, master should create the slot. --- patroni/dcs.py | 3 +++ patroni/postgresql.py | 13 ++++++++++--- tests/test_postgresql.py | 3 ++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index 85fda68c..62e5a654 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -111,6 +111,9 @@ class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,mem def is_unlocked(self): return not (self.leader and self.leader.name) + def has_member(self, member_name): + return len([m for m in self.members if m.name == member_name]) > 0 + class AbstractDCS: diff --git a/patroni/postgresql.py b/patroni/postgresql.py index f329fad5..293f7b51 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -651,11 +651,18 @@ $$""".format(name, options), name, password, password) if self.use_slots: try: self.load_replication_slots() + # if the replicatefrom tag is set on the member - we should not create the replication slot for it on + # the current master, because that member would replicate from elsewhere. We still create the slot if + # the replicatefrom destination member is currently not a member of the cluster (fallback to the + # master), or if replicatefrom destination member happens to be the current master if self.role == 'master': - slots = [m.name for m in cluster.members if m.name != self.name] + slots = [m.name for m in cluster.members if m.name != self.name and + (not cluster.has_member(m.replicatefrom) + if m.replicatefrom and m.replicatefrom != self.name else True)] else: # only manage slots for replicas that want to replicate from this one slots = [m.name for m in cluster.members if m.replicatefrom == self.name] + logger.info("setting replication slots for members {0}".format(slots)) # drop unused slots for slot in set(self.replication_slots) - set(slots): self.query("""SELECT pg_drop_replication_slot(%s) @@ -675,7 +682,7 @@ $$""".format(name, options), name, password, password) def last_operation(self): return str(self.xlog_position()) - def bootstrap(self, initialize=False, current_leader=None): + def bootstrap(self, cluster_initialized=False, current_leader=None): """ Populate PostgreSQL data directory by doing one of the following: - create with initdb if there is no master. @@ -696,7 +703,7 @@ $$""".format(name, options), name, password, password) that should be retried in the future. """ ret = False - if not (initialize or current_leader): + if not (cluster_initialized or current_leader): ret = self.initialize() and self.start() if ret: self.create_replication_user() diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 3681562f..a82bde99 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -181,7 +181,8 @@ class TestPostgresql(unittest.TestCase): os.makedirs(self.p.data_dir) self.leadermem = Member(0, 'leader', 28, {'conn_url': 'postgres://replicator:rep-pass@127.0.0.1:5435/postgres'}) self.leader = Leader(-1, 28, self.leadermem) - self.other = Member(0, 'test1', 28, {'conn_url': 'postgres://replicator:rep-pass@127.0.0.1:5433/postgres'}) + self.other = Member(0, 'test1', 28, {'conn_url': 'postgres://replicator:rep-pass@127.0.0.1:5433/postgres', + 'tags': {'replicatefrom': 'leader'}}) self.me = Member(0, 'test0', 28, {'conn_url': 'postgres://replicator:rep-pass@127.0.0.1:5434/postgres'}) def tearDown(self): From 5284a2144d07ad10d7478eb10d152f4ceeed82c3 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 4 Feb 2016 18:55:02 +0100 Subject: [PATCH 30/39] Add maintainers file. --- MAINTAINERS | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 MAINTAINERS diff --git a/MAINTAINERS b/MAINTAINERS new file mode 100644 index 00000000..fbe0251f --- /dev/null +++ b/MAINTAINERS @@ -0,0 +1,3 @@ +Alexander Kukushkin +Feike Steenbergen +Oleksii Kliukin From 03b56ae5b90ece76c572ff313e8e7b718aec3848 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 4 Feb 2016 19:14:22 +0100 Subject: [PATCH 31/39] Code refactoring per review by Alex Shulgin. In particular, rename most of the functions that have leader in the name if they can be called in the context where the leader is None. --- patroni/dcs.py | 2 +- patroni/ha.py | 10 +++++----- patroni/postgresql.py | 8 ++++---- tests/test_postgresql.py | 6 +++--- 4 files changed, 13 insertions(+), 13 deletions(-) diff --git a/patroni/dcs.py b/patroni/dcs.py index 62e5a654..a6b9d856 100644 --- a/patroni/dcs.py +++ b/patroni/dcs.py @@ -112,7 +112,7 @@ class Cluster(namedtuple('Cluster', 'initialize,leader,last_leader_operation,mem return not (self.leader and self.leader.name) def has_member(self, member_name): - return len([m for m in self.members if m.name == member_name]) > 0 + return any(m for m in self.members if m.name == member_name) class AbstractDCS: diff --git a/patroni/ha.py b/patroni/ha.py index 7ceac620..7e78eb77 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -62,7 +62,7 @@ class Ha: pass self.dcs.touch_member(json.dumps(data, separators=(',', ':'))) - def copy_backup_from_leader(self, leader): + def clone(self, leader): if self.state_handler.bootstrap(cluster_initialized=True, current_leader=leader): logger.info('bootstrapped from leader' if leader else 'bootstrapped without leader') else: @@ -73,7 +73,7 @@ class Ha: def bootstrap(self): if not self.cluster.is_unlocked(): # cluster already has leader self._async_executor.schedule('bootstrap from leader') - self._async_executor.run_async(self.copy_backup_from_leader, args=(self.cluster.leader, )) + self._async_executor.run_async(self.clone, args=(self.cluster.leader, )) return 'trying to bootstrap from leader' elif not self.cluster.initialize and not self.patroni.nofailover: # no initialize key if self.dcs.initialize(create_new=True): # race for initialization @@ -93,7 +93,7 @@ class Ha: return 'failed to acquire initialize lock' else: if self.state_handler.can_create_replica_without_leader(): - self._async_executor.run_async(self.copy_backup_from_leader, args=(None, )) + self._async_executor.run_async(self.clone, args=(None, )) return "trying to bootstrap without leader" return 'waiting for leader to bootstrap' @@ -120,7 +120,7 @@ class Ha: node_to_follow = node_to_follow[0] if node_to_follow else self.cluster.leader else: node_to_follow = self.cluster.leader - node_to_follow = None if (node_to_follow and node_to_follow.name) == self.state_handler.name else node_to_follow + node_to_follow = None if node_to_follow and node_to_follow.name == self.state_handler.name else node_to_follow if not self.state_handler.check_recovery_conf(node_to_follow) or recovery: self._async_executor.schedule('changing primary_conninfo and restarting') self._async_executor.run_async(self.state_handler.follow, (node_to_follow, recovery)) @@ -350,7 +350,7 @@ class Ha: def reinitialize(self, cluster): self.state_handler.stop('immediate') self.state_handler.remove_data_directory() - self.copy_backup_from_leader(cluster.leader) + self.clone(cluster.leader) def process_scheduled_action(self): if self.reinitialize_scheduled(): diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 293f7b51..113d8e31 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -219,7 +219,7 @@ class Postgresql: env['PGPASSFILE'] = self.pgpass return env - def sync_from_leader(self, leader): + def sync_replica(self, leader): if leader: r = parseurl(leader.conn_url) env = self.write_pgpass(r) if leader else os.environ.copy() @@ -657,8 +657,8 @@ $$""".format(name, options), name, password, password) # master), or if replicatefrom destination member happens to be the current master if self.role == 'master': slots = [m.name for m in cluster.members if m.name != self.name and - (not cluster.has_member(m.replicatefrom) - if m.replicatefrom and m.replicatefrom != self.name else True)] + (m.replicatefrom is None or m.replicatefrom == self.name or + not cluster.has_member(m.replicatefrom))] else: # only manage slots for replicas that want to replicate from this one slots = [m.name for m in cluster.members if m.replicatefrom == self.name] @@ -711,7 +711,7 @@ $$""".format(name, options), name, password, password) else: raise PostgresException("Could not bootstrap master PostgreSQL") else: - if self.sync_from_leader(current_leader): + if self.sync_replica(current_leader): self.restore_configuration_files() self.write_recovery_conf(current_leader, True) ret = self.start() diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index a82bde99..a88f150c 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -229,8 +229,8 @@ class TestPostgresql(unittest.TestCase): self.p.write_pgpass({'host': 'localhost', 'port': '5432', 'user': 'foo', 'password': 'bar'}) @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict())) - def test_sync_from_leader(self): - self.assertTrue(self.p.sync_from_leader(self.leader)) + def test_sync_replica(self): + self.assertTrue(self.p.sync_replica(self.leader)) @patch('subprocess.call', side_effect=Exception("Test")) @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict())) @@ -370,7 +370,7 @@ class TestPostgresql(unittest.TestCase): with patch('subprocess.call', Mock(return_value=1)): self.assertRaises(PostgresException, self.p.bootstrap) self.p.bootstrap() - with patch('patroni.postgresql.Postgresql.sync_from_leader', MagicMock(return_value=True)): + with patch('patroni.postgresql.Postgresql.sync_replica', MagicMock(return_value=True)): self.p.bootstrap(self.leader) def test_remove_data_directory(self): From 09ecd1cbece954beab765c772b9628eb1d673ffe Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Thu, 4 Feb 2016 19:27:26 +0100 Subject: [PATCH 32/39] Fix another issue with replication slots. Do not try to create replication slots on the replica for the member that wants to replicate from it if the member's currently holds the master role. Remove a debug message. --- patroni/postgresql.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 113d8e31..44d5acc3 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -660,9 +660,9 @@ $$""".format(name, options), name, password, password) (m.replicatefrom is None or m.replicatefrom == self.name or not cluster.has_member(m.replicatefrom))] else: - # only manage slots for replicas that want to replicate from this one - slots = [m.name for m in cluster.members if m.replicatefrom == self.name] - logger.info("setting replication slots for members {0}".format(slots)) + # only manage slots for replicas that replicate from this one, except for the leader among them + slots = [m.name for m in cluster.members if m.replicatefrom == self.name and + m.name != cluster.leader.name] # drop unused slots for slot in set(self.replication_slots) - set(slots): self.query("""SELECT pg_drop_replication_slot(%s) From 7ab366a7356230b68c65da88010d333aff44ad77 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 10 Feb 2016 10:26:22 +0100 Subject: [PATCH 33/39] Fix psycopg2 username/user confusion. Make sure username is translated into the user when calling psycopg2.connect. Also, fix the sample configuration files to use username everywhere. --- patroni/postgresql.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 0e279402..5a6cb390 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -132,6 +132,8 @@ class Postgresql: if not self._connection or self._connection.closed != 0: r = parseurl('postgres://{}/postgres'.format(self.local_address)) r.update(self.superuser) + if r.get('username'): + r['user'] = r['username'] self._connection = psycopg2.connect(**r) self._connection.autocommit = True self.server_version = self._connection.server_version From a513893a1903b9d6e14be4e05ddd2d861d90884e Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 10 Feb 2016 10:29:21 +0100 Subject: [PATCH 34/39] Really fix the sample configuration files. --- postgres1.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/postgres1.yml b/postgres1.yml index a8802226..c2d84ea0 100644 --- a/postgres1.yml +++ b/postgres1.yml @@ -63,7 +63,7 @@ postgresql: password: rep-pass network: 127.0.0.1/32 superuser: - user: postgres + username: postgres password: zalando admin: username: admin From 435eeeb85d141f87d343e28171647e7aae5eee87 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 10 Feb 2016 10:30:05 +0100 Subject: [PATCH 35/39] Add postgres2.yml as well. --- postgres2.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/postgres2.yml b/postgres2.yml index 99e8b47e..33620823 100644 --- a/postgres2.yml +++ b/postgres2.yml @@ -63,7 +63,7 @@ postgresql: password: rep-pass network: 127.0.0.1/32 superuser: - user: postgres + username: postgres password: zalando admin: username: admin From ad17b2070a4a48c62d72f4090f309620b749e9a4 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 10 Feb 2016 12:37:34 +0100 Subject: [PATCH 36/39] Create superuser with the name specified in config file And later use this name to connect to the cluster --- patroni/postgresql.py | 35 ++++++++++++++++++++--------------- tests/test_postgresql.py | 8 +------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 5a6cb390..9d46952f 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -131,9 +131,10 @@ class Postgresql: def connection(self): if not self._connection or self._connection.closed != 0: r = parseurl('postgres://{}/postgres'.format(self.local_address)) - r.update(self.superuser) - if r.get('username'): - r['user'] = r['username'] + if 'username' in self.superuser: + r['user'] = self.superuser['username'] + if 'password' in self.superuser: + r['password'] = self.superuser['password'] self._connection = psycopg2.connect(**r) self._connection.autocommit = True self.server_version = self._connection.server_version @@ -196,11 +197,15 @@ class Postgresql: self.set_state('initalizing new cluster') options = self.get_initdb_options() pwfile = None - if self.superuser and 'username' not in self.superuser and 'password' in self.superuser: - (fd, pwfile) = tempfile.mkstemp() - os.write(fd, self.superuser['password'].encode()) - os.close(fd) - options.append('--pwfile={}'.format(pwfile)) + + if self.superuser: + if 'username' in self.superuser: + options.append('--username={}'.format(self.superuser['username'])) + if 'password' in self.superuser: + (fd, pwfile) = tempfile.mkstemp() + os.write(fd, self.superuser['password'].encode()) + os.close(fd) + options.append('--pwfile={}'.format(pwfile)) ret = subprocess.call(self._pg_ctl + ['initdb'] + (['-o', ' '.join(options)] if options else [])) == 0 if pwfile: @@ -351,7 +356,10 @@ class Postgresql: if not block_callbacks: self.set_state('starting') - ret = subprocess.call(self._pg_ctl + ['start', '-o', self.server_options()]) == 0 + env = os.environ.copy() + if 'username' in self.superuser: + env['PGUSER'] = self.superuser['username'] + ret = subprocess.call(self._pg_ctl + ['start', '-o', self.server_options()], env=env) == 0 self.set_state('running' if ret else 'start failed') @@ -633,11 +641,8 @@ $$""".format(name, options), name, password, password) def create_replication_user(self): self.create_or_update_role(self.replication['username'], self.replication['password'], 'REPLICATION') - def create_connection_users(self): - if 'username' in self.superuser: - self.create_or_update_role(self.superuser['username'], self.superuser['password'], 'SUPERUSER') - if self.admin: - self.create_or_update_role(self.admin['username'], self.admin['password'], 'CREATEDB CREATEROLE') + def create_connection_user(self): + self.admin and self.create_or_update_role(self.admin['username'], self.admin['password'], 'CREATEDB CREATEROLE') def xlog_position(self): return self.query("""SELECT pg_xlog_location_diff(CASE WHEN pg_is_in_recovery() @@ -711,7 +716,7 @@ $$""".format(name, options), name, password, password) ret = self.initialize() and self.start() if ret: self.create_replication_user() - self.create_connection_users() + self.create_connection_user() else: raise PostgresException("Could not bootstrap master PostgreSQL") else: diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index a88f150c..3c93be44 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -165,7 +165,7 @@ class TestPostgresql(unittest.TestCase): self.p = Postgresql({'name': 'test0', 'scope': 'batman', 'data_dir': 'data/test0', 'listen': '127.0.0.1, *:5432', 'connect_address': '127.0.0.2:5432', 'pg_hba': ['hostssl all all 0.0.0.0/0 md5', 'host all all 0.0.0.0/0 md5'], - 'superuser': {'password': 'test'}, + 'superuser': {'username': 'test', 'password': 'test'}, 'admin': {'username': 'admin', 'password': 'admin'}, 'pg_rewind': {'username': 'admin', 'password': 'admin'}, 'replication': {'username': 'replicator', @@ -295,12 +295,6 @@ class TestPostgresql(unittest.TestCase): with patch('subprocess.call', Mock(side_effect=Exception("foo"))): self.assertEquals(self.p.create_replica(self.leader, ''), 1) - def test_create_connection_users(self): - cfg = self.p.config - cfg['superuser']['username'] = 'test' - p = Postgresql(cfg) - p.create_connection_users() - def test_sync_replication_slots(self): self.p.start() cluster = Cluster(True, self.leader, 0, [self.me, self.other, self.leadermem], None) From b4af126bc3f3de31b2aeaa789910c23435ff4bbd Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 10 Feb 2016 13:50:05 +0100 Subject: [PATCH 37/39] Apply superuser name and password when doing checkpoint --- patroni/postgresql.py | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 9d46952f..4e60aa70 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -128,14 +128,18 @@ class Postgresql: break return local_address + ':' + self.port + @property + def _connect_kwargs(self): + r = parseurl('postgres://{}/postgres'.format(self.local_address)) + if 'username' in self.superuser: + r['user'] = self.superuser['username'] + if 'password' in self.superuser: + r['password'] = self.superuser['password'] + return r + def connection(self): if not self._connection or self._connection.closed != 0: - r = parseurl('postgres://{}/postgres'.format(self.local_address)) - if 'username' in self.superuser: - r['user'] = self.superuser['username'] - if 'password' in self.superuser: - r['password'] = self.superuser['password'] - self._connection = psycopg2.connect(**r) + self._connection = psycopg2.connect(**self._connect_kwargs) self._connection.autocommit = True self.server_version = self._connection.server_version return self._connection @@ -370,10 +374,12 @@ class Postgresql: ret and not block_callbacks and self.call_nowait(ACTION_ON_START) return ret - def checkpoint(self, connstring=None): + def checkpoint(self, connect_kwargs=None): + connect_kwargs = connect_kwargs or self._connect_kwargs + for p in ['connect_timeout', 'options']: + connect_kwargs.pop(p, None) try: - connstring = connstring or 'postgres://{}/postgres'.format(self.local_address) - with psycopg2.connect(connstring) as conn: + with psycopg2.connect(**connect_kwargs) as conn: conn.autocommit = True with conn.cursor() as cur: cur.execute("SET statement_timeout = 0") @@ -480,12 +486,12 @@ recovery_target_timeline = 'latest' # prepare pg_rewind connection r = parseurl(leader.conn_url) r.update(self.pg_rewind) - r['user'] = r['username'] + r['user'] = r.pop('username') env = self.write_pgpass(r) pc = "user={user} host={host} port={port} dbname=postgres sslmode=prefer sslcompression=1".format(**r) # first run a checkpoint on a promoted master in order # to make it store the new timeline (5540277D.8020309@iki.fi) - self.checkpoint(pc) + self.checkpoint(r) logger.info("running pg_rewind from {}".format(pc)) pg_rewind = ['pg_rewind', '-D', self.data_dir, '--source-server', pc] try: From 7b524bc5571f2c49d7375d2cbd4eb3c7d303dca8 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Wed, 10 Feb 2016 14:51:20 +0100 Subject: [PATCH 38/39] Create new session and set the process group ID for the postmaster Otherwice it was receiving signal when you pressed Ctrl+C in the terminal where patroni is running. --- patroni/postgresql.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 4e60aa70..2b2aefe3 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -363,7 +363,7 @@ class Postgresql: env = os.environ.copy() if 'username' in self.superuser: env['PGUSER'] = self.superuser['username'] - ret = subprocess.call(self._pg_ctl + ['start', '-o', self.server_options()], env=env) == 0 + ret = subprocess.call(self._pg_ctl + ['start', '-o', self.server_options()], env=env, preexec_fn=os.setsid) == 0 self.set_state('running' if ret else 'start failed') From d530133f6280111c2b5a1287b1367433cd2bec10 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Wed, 10 Feb 2016 16:33:47 +0100 Subject: [PATCH 39/39] Fixes #133 --- tests/test_ctl.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/test_ctl.py b/tests/test_ctl.py index 81f825aa..70678a39 100644 --- a/tests/test_ctl.py +++ b/tests/test_ctl.py @@ -8,7 +8,8 @@ import psycopg2 import requests import patroni.exceptions import etcd -from mock import patch, Mock +from mock import patch, Mock, MagicMock + from click.testing import CliRunner from patroni.ctl import ctl, members, store_config, load_config, output_members, post_patroni, get_dcs, \ @@ -326,8 +327,9 @@ leader''') assert cluster.leader.member.name == 'leader' def test_post_patroni(self): - member = get_cluster_initialized_with_leader().leader.member - self.assertRaises(requests.exceptions.ConnectionError, post_patroni, member, 'dummy', {}) + with patch('requests.post', MagicMock(side_effect=requests.exceptions.ConnectionError('foo'))): + member = get_cluster_initialized_with_leader().leader.member + self.assertRaises(requests.exceptions.ConnectionError, post_patroni, member, 'dummy', {}) def test_ctl(self): runner = CliRunner()