From aa10f429132d1cdd90688a6d7856e2067874cf93 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 30 Jun 2016 10:45:54 +0200 Subject: [PATCH] checkpoint method returns string status message --- patroni/postgresql.py | 17 +++++++---------- tests/test_postgresql.py | 14 +++++++------- 2 files changed, 14 insertions(+), 17 deletions(-) diff --git a/patroni/postgresql.py b/patroni/postgresql.py index 8e1d7075..97d91832 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -550,11 +550,11 @@ class Postgresql(object): if check_not_is_in_recovery: cur.execute('SELECT pg_is_in_recovery()') if cur.fetchone()[0]: - return False - cur.execute('CHECKPOINT') - return True + return 'is_in_recovery=true' + return cur.execute('CHECKPOINT') except psycopg2.Error: logging.exception('Exception during CHECKPOINT') + return 'not accessible or not healty' def stop(self, mode='fast', block_callbacks=False, checkpoint=True): # make sure we close all connections established against @@ -741,7 +741,7 @@ class Postgresql(object): stopped = self.stop() self.set_role('unknown') if not stopped: - return logger.warning('Can not run pg_rewind because posgres is still running') + return logger.warning('Can not run pg_rewind because postgres is still running') if not (leader and leader.conn_url): return logger.info('Leader unknown, can not rewind') @@ -753,18 +753,15 @@ class Postgresql(object): # from the master and run a checkpoint on a t in order to # make it store the new timeline (5540277D.8020309@iki.fi) leader_status = self.checkpoint(r) - if not leader_status: - return logger.warning('Can not use %s for rewind: %s', leader.name, - 'is_in_recovery=true' if leader_status is False else 'not accessible') + if leader_status: + return logger.warning('Can not use %s for rewind: %s', leader.name, leader_status) # at present, pg_rewind only runs when the cluster is shut down cleanly # and not shutdown in recovery. We have to remove the recovery.conf if present # and start/shutdown in a single user mode to emulate this. # XXX: if recovery.conf is linked, it will be written anew as a normal file. - if os.path.islink(self._recovery_conf): + if os.path.isfile(self._recovery_conf) or os.path.islink(self._recovery_conf): os.unlink(self._recovery_conf) - elif os.path.isfile(self._recovery_conf): - os.remove(self._recovery_conf) # Archived segments might be useful to pg_rewind, # clean the flags that tell we should remove them. diff --git a/tests/test_postgresql.py b/tests/test_postgresql.py index 97643aba..dde9647c 100644 --- a/tests/test_postgresql.py +++ b/tests/test_postgresql.py @@ -232,9 +232,10 @@ class TestPostgresql(unittest.TestCase): def test_checkpoint(self): with patch.object(MockCursor, 'fetchone', Mock(return_value=(True, ))): - self.assertFalse(self.p.checkpoint({'user': 'postgres'})) - with patch.object(MockCursor, 'execute', Mock()): - self.assertTrue(self.p.checkpoint()) + self.assertEquals(self.p.checkpoint({'user': 'postgres'}), 'is_in_recovery=true') + with patch.object(MockCursor, 'execute', Mock(return_value=None)): + self.assertIsNone(self.p.checkpoint()) + self.assertEquals(self.p.checkpoint(), 'not accessible or not healty') @patch('subprocess.call', side_effect=OSError) @patch('patroni.postgresql.Postgresql.write_pgpass', MagicMock(return_value=dict())) @@ -270,10 +271,9 @@ class TestPostgresql(unittest.TestCase): self.p.follow(self.leader, self.leader) # "leader" is not accessible or is_in_recovery - with patch.object(Postgresql, 'checkpoint', Mock(return_value=True)): - with patch('os.path.islink', Mock(return_value=True)): - self.p.follow(self.leader, self.leader) - self.p.set_role('master') + with patch.object(Postgresql, 'checkpoint', Mock(return_value=None)): + self.p.follow(self.leader, self.leader) + self.p.set_role('master') mock_pg_rewind.return_value = True self.p.follow(self.leader, self.leader)