From 953ea749bf902e91ea6f957d6fd68ec8fd580dea Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 13 Oct 2015 15:00:16 +0200 Subject: [PATCH 1/7] Make sure patroni is not using stale connections. After the PostgreSQL crash (i.e. with kill -9), the backend patroni connects to may still exist. In this case, patroni will get stale postgres role from this backend, preventing a restarted node with a leader lock from being promoted. Easily reproducible and also observed in a staging environment after the postgres crash due to out of disk space. --- patroni/postgresql.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index fc7956e9..a7f95a81 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -127,9 +127,15 @@ 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") self._cursor_holder = self.connection().cursor() return self._cursor_holder + def close_connection(self): + if self._cursor_holder and self._cursor_holder.connection and self._cursor_holder.connection.closed == 0: + self._cursor_holder.connection.close() + logger.info("closed patroni connection to the postgresql cluster") + def _query(self, sql, *params): cursor = None try: @@ -269,6 +275,12 @@ class Postgresql: logging.exception('Exception during CHECKPOINT') def stop(self, mode='fast', block_callbacks=False): + # make sure we close all connections established against + # the former node, otherwise, we might get a stalled one + # after kill -9, which would report incorrect data to + # patroni. + + self.close_connection() if not self.is_running(): if not block_callbacks: self.set_state('stopped') From c7246e48d9a4986d603bfba7413711ec54b5480c Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 14 Oct 2015 09:46:20 +0200 Subject: [PATCH 2/7] Work around the differences in pg_controldata names. --- patroni/postgresql.py | 2 +- tests/test_postgresql.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index fc7956e9..263b58ce 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -379,7 +379,7 @@ recovery_target_timeline = 'latest' data = subprocess.check_output(['pg_controldata', self.data_dir]) if data: data = data.splitlines() - result = {l.split(':')[0]: l.split(':')[1].strip() for l in data if l} + result = {l.split(':')[0].replace('Current ', '', 1): l.split(':')[1].strip() for l in data if l} except subprocess.CalledProcessError: logger.exception("Error when calling pg_controldata") finally: diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index e5e2dfd4..b402626a 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -118,12 +118,12 @@ Backup start location: 0/0 Backup end location: 0/0 End-of-backup record required: no wal_level setting: hot_standby -wal_log_hints setting: on -max_connections setting: 100 -max_worker_processes setting: 8 -max_prepared_xacts setting: 0 -max_locks_per_xact setting: 64 -track_commit_timestamp setting: off +Current wal_log_hints setting: on +Current max_connections setting: 100 +Current max_worker_processes setting: 8 +Current max_prepared_xacts setting: 0 +Current max_locks_per_xact setting: 64 +Current track_commit_timestamp setting: off Maximum data alignment: 8 Database block size: 8192 Blocks per segment of large relation: 131072 From 98b59354a9f4d72ec7bd9e668d67ea683a892e06 Mon Sep 17 00:00:00 2001 From: Feike Steenbergen Date: Wed, 14 Oct 2015 14:37:05 +0200 Subject: [PATCH 3/7] Exclude more files from git. --- .gitignore | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index df367db5..699794c2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,11 @@ data/* *.pyc -helpers/*.pyc +*.egg/ +*.egg-info/ +.cache/ +.coverage +.eggs/ +build/ +coverage.xml +junit.xml +pgpass From 5c86b60cd2a96033c3475fd14e6207f0f5fea41a Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Wed, 14 Oct 2015 17:05:09 +0200 Subject: [PATCH 4/7] Fix an exception in the (rather unusual) case of attaching Patroni to an existing running replica. --- patroni/postgresql.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 263b58ce..5a857c1c 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -316,7 +316,8 @@ class Postgresql: return True def check_replication_lag(self, last_leader_operation): - return last_leader_operation - self.xlog_position() <= self.config.get('maximum_lag_on_failover', 0) + return (last_leader_operation if last_leader_operation else 0) - self.xlog_position() <=\ + self.config.get('maximum_lag_on_failover', 0) def write_pg_hba(self): with open(os.path.join(self.data_dir, 'pg_hba.conf'), 'a') as f: From 2f0cf1db06269d05c8ad30f865c8f788078bf16c Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 15 Oct 2015 09:08:16 +0200 Subject: [PATCH 5/7] Mock etcd client delete method --- tests/test_ha.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_ha.py b/tests/test_ha.py index 38bb07a6..a5a816da 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -1,3 +1,4 @@ +import etcd import unittest from mock import Mock, patch @@ -98,6 +99,7 @@ class TestHa(unittest.TestCase): self.e = Etcd('foo', {'ttl': 30, 'host': 'ok:2379', 'scope': 'test'}) self.e.client.read = etcd_read self.e.client.write = etcd_write + self.e.client.delete = Mock(side_effect=etcd.EtcdException()) self.ha = Ha(MockPatroni(self.p, self.e)) self.ha._async_executor.run_async = run_async self.ha.old_cluster = self.e.get_cluster() From 16a0a3481db76cda73207f78e19905b17ca3f4f2 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 15 Oct 2015 09:08:33 +0200 Subject: [PATCH 6/7] fix pep8 formatting --- tests/test_postgresql.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index b402626a..9a02ece6 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -232,8 +232,8 @@ class TestPostgresql(unittest.TestCase): self.p.follow_the_leader(self.leader) self.p.require_rewind() with mock.patch('os.path.islink', MagicMock(return_value=True)): - with mock.patch('os.unlink', MagicMock(return_value=True)): - with mock.patch('patroni.postgresql.Postgresql.can_rewind', new_callable=PropertyMock(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.require_rewind() with mock.patch('patroni.postgresql.Postgresql.can_rewind', new_callable=PropertyMock(return_value=True)): From 921e4fc32357d7b7fdecc57b76c5a7eb84aa6e5e Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Fri, 16 Oct 2015 10:28:15 +0200 Subject: [PATCH 7/7] psycopg2 should be not older than 2.6.1 --- 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 d57df720..fde9c79a 100644 --- a/requirements-py2.txt +++ b/requirements-py2.txt @@ -1,7 +1,7 @@ boto dnspython mock -psycopg2 +psycopg2>=2.6.1 PyYAML requests six >= 1.7 diff --git a/requirements-py3.txt b/requirements-py3.txt index 0fd9dfb3..cc00965b 100644 --- a/requirements-py3.txt +++ b/requirements-py3.txt @@ -1,7 +1,7 @@ boto mock dnspython3 -psycopg2 +psycopg2>=2.6.1 PyYAML requests six