From 9dc588231f72117a2ad89f3655af19c03d6b65c6 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 24 Mar 2016 12:29:30 +0100 Subject: [PATCH 1/4] Speed up reattach of former leader to the cluster Instead of starting it up in "read-only", it wil wait 2 seconds, to give a time to somebody to prompte and after it will execute normal `recover` procedure. --- patroni/ha.py | 20 +++++++++++--------- tests/test_ha.py | 1 + 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index d9916da6..f85064fc 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -110,14 +110,12 @@ class Ha(object): def recover(self): # 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 (self.state_handler.role == 'master') and pg_controldata and\ - pg_controldata.get('Database cluster state', '') == 'in production': # crashed master - self.state_handler.require_rewind() + if self.state_handler.role == 'master': + pg_controldata = self.state_handler.controldata() + if pg_controldata and pg_controldata.get('Database cluster state', '') == 'in production': # crashed master + self.state_handler.require_rewind() self.recovering = True - return self.follow("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", True, True) def follow(self, demote_reason, follow_reason, refresh=True, recovery=False): if refresh: @@ -279,7 +277,11 @@ class Ha(object): self.dcs.delete_leader() self.touch_member() self.dcs.reset_cluster() - self.state_handler.follow(None) + self.state_handler.set_role('replica') + sleep(2) # Give a time to somebody to promote + self.recover() + else: + self.state_handler.follow(None) def process_manual_failover_from_leader(self): failover = self.cluster.failover @@ -331,7 +333,7 @@ class Ha(object): if self.cluster.failover: logger.info('Cleaning up failover key after acquiring leader lock...') self.dcs.manual_failover('', '') - self.dcs.get_cluster() + self.load_cluster_from_dcs() return self.enforce_master_role('acquired session lock as a leader', 'promoted self to leader by acquiring session lock') else: diff --git a/tests/test_ha.py b/tests/test_ha.py index 3ab7442b..8ee8748b 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -274,6 +274,7 @@ class TestHa(unittest.TestCase): self.assertEquals(self.ha.run_cycle(), 'failed to update leader lock during restart') @patch('requests.get', requests_get) + @patch('time.sleep', Mock()) def test_manual_failover_from_leader(self): self.ha.has_lock = true self.ha.cluster = get_cluster_initialized_with_leader(Failover(0, 'blabla', '', None)) From e6af18f0bb8818e01719e97b07417f39ab8f79c3 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 24 Mar 2016 14:45:21 +0100 Subject: [PATCH 2/4] Former leader was not able to reattach to cluster without pg_rewind It was shutdown correctly and I expected such 'join' working, but it was not, because new leader didn't had enough time to catch up with the master before promote. --- features/patroni_api.feature | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/features/patroni_api.feature b/features/patroni_api.feature index b1eded96..3c408dfc 100644 --- a/features/patroni_api.feature +++ b/features/patroni_api.feature @@ -29,20 +29,23 @@ Scenario: check API requests for the primary-replica pair And I receive a response role replica When I issue an empty POST request to http://127.0.0.1:8009/reinitialize Then I receive a response code 200 - Given replication works from postgres0 to postgres1 after 10 seconds When I issue an empty POST request to http://127.0.0.1:8008/restart Then I receive a response code 200 And postgres0 is a leader after 5 seconds + When I sleep for 10 seconds + Then postgres1 role is the secondary after 15 seconds Scenario: check the failover via the API Given I issue a POST request to http://127.0.0.1:8008/failover with leader=postgres0,candidate=postgres1 Then I receive a response code 200 And postgres1 is a leader after 5 seconds + And postgres1 role is the primary after 5 seconds + And postgres0 role is the secondary after 5 seconds And replication works from postgres1 to postgres0 after 15 seconds Scenario: check the scheduled failover Given I issue a scheduled failover at http://127.0.0.1:8009 from postgres1 to postgres0 in 10 seconds Then I receive a response code 200 - And postgres0 is a leader after 15 seconds + And postgres0 is a leader after 20 seconds And replication works from postgres0 to postgres1 after 25 seconds From 9c41ce9f1c22ceeea0d58a8648bb6720bfe8c4a0 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 24 Mar 2016 14:50:34 +0100 Subject: [PATCH 3/4] We should run checkpoint before shutdown all the time except one case (when patroni is being shutdown) --- patroni/__init__.py | 2 +- patroni/postgresql.py | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/patroni/__init__.py b/patroni/__init__.py index 85b6c5df..19ef4ac9 100644 --- a/patroni/__init__.py +++ b/patroni/__init__.py @@ -85,5 +85,5 @@ def main(): pass finally: patroni.api.shutdown() - patroni.postgresql.stop() + patroni.postgresql.stop(checkpoint=False) patroni.dcs.delete_leader() diff --git a/patroni/postgresql.py b/patroni/postgresql.py index df17200d..b3e2ab00 100644 --- a/patroni/postgresql.py +++ b/patroni/postgresql.py @@ -388,7 +388,7 @@ class Postgresql(object): except psycopg2.Error: logging.exception('Exception during CHECKPOINT') - def stop(self, mode='fast', block_callbacks=False): + def stop(self, mode='fast', block_callbacks=False, checkpoint=True): # 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 @@ -400,9 +400,10 @@ class Postgresql(object): self.set_state('stopped') return True - if block_callbacks: + if checkpoint: self.checkpoint() - else: + + if not block_callbacks: self.set_state('stopping') ret = subprocess.call(self._pg_ctl + ['stop', '-m', mode]) == 0 From d4cb15179f2f29ceb5bbdee310febf146df63c65 Mon Sep 17 00:00:00 2001 From: Alexander Kukushkin Date: Thu, 24 Mar 2016 14:52:27 +0100 Subject: [PATCH 4/4] Return "demote reason" more precise depending on cluster state --- patroni/ha.py | 13 ++++++------- tests/test_ha.py | 10 +++++----- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index f85064fc..d4d7fb93 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -115,25 +115,25 @@ class Ha(object): if pg_controldata and pg_controldata.get('Database cluster state', '') == 'in production': # crashed master self.state_handler.require_rewind() self.recovering = True - return self.follow("started as readonly because i had the session lock", "started as a secondary", True, True) + return self.follow("starting as readonly because i had the session lock", "starting as a secondary", True, True) def follow(self, demote_reason, follow_reason, refresh=True, recovery=False): if refresh: self.load_cluster_from_dcs() - if not recovery and self.state_handler.is_leader() or recovery and self.state_handler.role == 'master': - ret = demote_reason - else: - ret = follow_reason + ret = demote_reason if not recovery and self.state_handler.is_leader() 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: 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 node_to_follow and node_to_follow.name == self.state_handler.name: + ret = demote_reason + node_to_follow = None 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)) @@ -277,7 +277,6 @@ class Ha(object): self.dcs.delete_leader() self.touch_member() self.dcs.reset_cluster() - self.state_handler.set_role('replica') sleep(2) # Give a time to somebody to promote self.recover() else: diff --git a/tests/test_ha.py b/tests/test_ha.py index 8ee8748b..d81d5bbd 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -110,25 +110,25 @@ class TestHa(unittest.TestCase): def test_start_as_replica(self): self.p.is_healthy = false - self.assertEquals(self.ha.run_cycle(), 'started as a secondary') + self.assertEquals(self.ha.run_cycle(), 'starting as a secondary') 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 = false - self.assertEquals(self.ha.run_cycle(), 'started as a secondary') + self.assertEquals(self.ha.run_cycle(), 'starting as a secondary') self.assertEquals(self.ha.run_cycle(), 'failed to start postgres') def test_recover_master_failed(self): self.p.follow = false self.p.is_healthy = false self.p.is_running = false - self.ha.has_lock = true + self.p.name = 'leader' self.p.set_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') + self.ha.cluster = get_cluster_initialized_with_leader() + self.assertEquals(self.ha.run_cycle(), 'starting as readonly because i had the session lock') @patch('sys.exit', return_value=1) @patch('patroni.ha.Ha.sysid_valid', MagicMock(return_value=True))