From 0afdb816ba3652029908d90caac3987f8fa58184 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 30 Aug 2016 10:38:40 +0200 Subject: [PATCH 1/2] Unfinished promote may not break paused cluster. When a node to promote dies before finishing the promote and the cluster is in a standby mode, the failover key sticks indefinitely, preventing any master to take over the leader role. Prevent it by letting the node in a master role cleanup the failover key if the node to failover is not present among the members. The master check cannot be performed by the node role alone, since the node will not change its cached role on a manual promote. We need to check the DB state as well. --- patroni/ha.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/patroni/ha.py b/patroni/ha.py index 60734ee6..5d106512 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -237,6 +237,11 @@ class Ha(object): if failover.candidate == self.state_handler.name: # manual failover to me return True elif self.is_paused(): + # Remove failover key if the node to failover has terminated to avoid waiting for it indefinitely + # In order to avoid race conditions only the master is allowed to do so. + if (not self.cluster.get_member(failover.candidate, fallback_to_leader=False) and + (self.state_handler.is_leader() or self.state_handler.role == 'master')): + self.dcs.manual_failover('', '', index=self.cluster.failover.index) return False # find specific node and check that it is healthy From 11359a26a919802236f7d75052a06c2a76219760 Mon Sep 17 00:00:00 2001 From: Oleksii Kliukin Date: Tue, 30 Aug 2016 12:00:51 +0200 Subject: [PATCH 2/2] Improve incomplete failover is a paused mode. Instead of empying the stale failover key as a master and bailing out, continue with the healthiest node evaluation. This should make the actual master acquire the leader key faster. Emit the warning message as well and add unit tests. --- patroni/ha.py | 11 ++++++++--- tests/test_ha.py | 3 +++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/patroni/ha.py b/patroni/ha.py index 5d106512..26565f2f 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -238,10 +238,12 @@ class Ha(object): return True elif self.is_paused(): # Remove failover key if the node to failover has terminated to avoid waiting for it indefinitely - # In order to avoid race conditions only the master is allowed to do so. + # In order to avoid attempts to delete this key from all nodes only the master is allowed to do it. if (not self.cluster.get_member(failover.candidate, fallback_to_leader=False) and - (self.state_handler.is_leader() or self.state_handler.role == 'master')): + self.state_handler.is_leader()): + logger.warning("manual failover: removing failover key because failover candidate is not running") self.dcs.manual_failover('', '', index=self.cluster.failover.index) + return None return False # find specific node and check that it is healthy @@ -281,7 +283,9 @@ class Ha(object): def is_healthiest_node(self): if self.is_paused() and not self.patroni.nofailover and \ self.cluster.failover and not self.cluster.failover.scheduled_at: - return self.manual_failover_process_no_leader() + ret = self.manual_failover_process_no_leader() + if ret is not None: # continue if we just deleted the stale failover key as a master + return ret if self.state_handler.is_leader(): # leader is always the healthiest return True @@ -377,6 +381,7 @@ class Ha(object): def process_unhealthy_cluster(self): """Cluster has no leader key""" + if self.is_healthiest_node(): if self.acquire_lock(): if self.cluster.failover: diff --git a/tests/test_ha.py b/tests/test_ha.py index 52b7af18..dd717771 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -394,6 +394,9 @@ class TestHa(unittest.TestCase): self.assertEquals(self.ha.run_cycle(), 'PAUSE: continue to run as master without lock') self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', '', None)) self.assertEquals(self.ha.run_cycle(), 'PAUSE: continue to run as master without lock') + self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', 'blabla', None)) + self.p.is_leader = true + self.assertEquals('PAUSE: acquired session lock as a leader', self.ha.run_cycle()) def test_is_healthiest_node(self): self.ha.state_handler.is_leader = false