mirror of
https://github.com/outbackdingo/patroni.git
synced 2026-08-25 14:53:37 +00:00
Make sure only sync node tries to grab the lock when switchover (#2406)
- We use `failover.leader` check as an indicator of switchover. The check on `failover.candidate` was unnecessary and incorrect. - Add more unit-tests for switchover in sync mode Close https://github.com/zalando/patroni/issues/2405 Co-authored-by: Polina Bungina <[email protected]>
This commit is contained in:
+6
-1
@@ -737,6 +737,11 @@ class Ha(object):
|
||||
return None
|
||||
return False
|
||||
|
||||
# in synchronous mode when our name is not in the /sync key
|
||||
# we shouldn't take any action even if the candidate is unhealthy
|
||||
if self.is_synchronous_mode() and not self.cluster.sync.matches(self.state_handler.name):
|
||||
return False
|
||||
|
||||
# find specific node and check that it is healthy
|
||||
member = self.cluster.get_member(failover.candidate, fallback_to_leader=False)
|
||||
if member:
|
||||
@@ -797,7 +802,7 @@ class Ha(object):
|
||||
if self.cluster.failover:
|
||||
# When doing a switchover in synchronous mode only synchronous nodes and former leader are allowed to race
|
||||
if self.is_synchronous_mode() and self.cluster.failover.leader and \
|
||||
self.cluster.failover.candidate and not self.cluster.sync.matches(self.state_handler.name):
|
||||
not self.cluster.sync.matches(self.state_handler.name):
|
||||
return False
|
||||
return self.manual_failover_process_no_leader()
|
||||
|
||||
|
||||
+51
-3
@@ -643,12 +643,60 @@ class TestHa(PostgresInit):
|
||||
# same as previous, but set the current member to nofailover. In no case it should be elected as a leader
|
||||
self.ha.patroni.nofailover = True
|
||||
self.assertEqual(self.ha.run_cycle(), 'following a different leader because I am not allowed to promote')
|
||||
# in sync mode only the sync node is allowed to take over
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', 'other', None))
|
||||
self.ha.patroni.nofailover = False
|
||||
|
||||
def test_manual_failover_process_no_leader_in_synchronous_mode(self):
|
||||
self.ha.is_synchronous_mode = true
|
||||
self.p.is_leader = false
|
||||
|
||||
# switchover to a specific node, which name doesn't match our name (postgresql0)
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', 'other', None))
|
||||
self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
|
||||
|
||||
# switchover to our node (postgresql0), which name is not in sync nodes list
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', 'postgresql0', None),
|
||||
sync=('leader1', 'blabla'))
|
||||
self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
|
||||
|
||||
# switchover from a specific leader, but our name (postgresql0) is not in the sync nodes list
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', '', None),
|
||||
sync=('leader', 'blabla'))
|
||||
self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
|
||||
|
||||
# switchover from a specific leader, but the only sync node (us, postgresql0) has nofailover tag
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, 'leader', '', None),
|
||||
sync=('postgresql0'))
|
||||
self.ha.patroni.nofailover = True
|
||||
self.assertEqual(self.ha.run_cycle(), 'following a different leader because I am not allowed to promote')
|
||||
self.ha.patroni.nofailover = False
|
||||
|
||||
# manual failover when our name (postgresql0) isn't in the /sync key and the `other` node is not available
|
||||
self.ha.fetch_node_status = get_node_status(nofailover=True) # accessible, in_recovery
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None),
|
||||
sync=('leader1', 'blabla'))
|
||||
self.assertEqual(self.ha.run_cycle(), 'following a different leader because i am not the healthiest node')
|
||||
|
||||
# manual failover when the `other` node isn't available but our name is in the /sync key
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None),
|
||||
sync=('leader1', 'postgresql0'))
|
||||
self.p.pick_synchronous_standby = Mock(return_value=([], []))
|
||||
self.ha.dcs.write_sync_state = true
|
||||
self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
|
||||
|
||||
# manual failover to our node (postgresql0),
|
||||
# which name is not in sync nodes list (the leader and all sync nodes are not available)
|
||||
self.p.set_role('replica')
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'postgresql0', None),
|
||||
sync=('leader1', 'other'))
|
||||
self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
|
||||
|
||||
# manual failover to our node (postgresql0),
|
||||
# which name is not in sync nodes list (some sync nodes are available)
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'postgresql0', None),
|
||||
sync=('leader1', 'other'))
|
||||
self.p.set_role('replica')
|
||||
self.p.pick_synchronous_standby = Mock(return_value=(['leader1'], ['leader1']))
|
||||
self.assertEqual(self.ha.run_cycle(), 'promoted self to leader by acquiring session lock')
|
||||
|
||||
def test_manual_failover_process_no_leader_in_pause(self):
|
||||
self.ha.is_paused = true
|
||||
self.ha.cluster = get_cluster_initialized_without_leader(failover=Failover(0, '', 'other', None))
|
||||
|
||||
Reference in New Issue
Block a user