mirror of
https://github.com/outbackdingo/patroni.git
synced 2026-08-25 07:20:20 +00:00
Fix bug with switchover in synchronous_mode=quorum (#3310)
When the candidate is specified we don't have to check quorum requirements. The problem was introduced in #3278 Close #3307
This commit is contained in:
+12
-4
@@ -1377,7 +1377,14 @@ class Ha(object):
|
||||
quorum_votes += 1
|
||||
|
||||
# In case of quorum replication we need to make sure that there is enough healthy synchronous replicas!
|
||||
return quorum_votes >= (self.cluster.sync.quorum if self.quorum_commit_mode_is_active() else 0)
|
||||
# However, when failover candidate is set, we can ignore quorum requirements.
|
||||
check_quorum = self.quorum_commit_mode_is_active() and\
|
||||
not (self.cluster.failover and self.cluster.failover.candidate and not exclude_failover_candidate)
|
||||
if check_quorum and quorum_votes < self.cluster.sync.quorum:
|
||||
logger.info('Quorum requirement %d can not be reached', self.cluster.sync.quorum)
|
||||
return False
|
||||
|
||||
return quorum_votes >= 0
|
||||
|
||||
def manual_failover_process_no_leader(self) -> Optional[bool]:
|
||||
"""Handles manual failover/switchover when the old leader already stepped down.
|
||||
@@ -2418,8 +2425,9 @@ class Ha(object):
|
||||
return False
|
||||
# Don't spend time on "nofailover" nodes checking.
|
||||
# We also don't need nodes which we can't query with the api in the list.
|
||||
return node.name not in exclude and \
|
||||
not node.nofailover and bool(node.api_url) and \
|
||||
(not failover or not failover.candidate or node.name == failover.candidate)
|
||||
# And, if exclude_failover_candidate is True we want to skip node.name == failover.candidate check.
|
||||
return node.name not in exclude and not node.nofailover and bool(node.api_url) and \
|
||||
(exclude_failover_candidate or not failover
|
||||
or not failover.candidate or node.name == failover.candidate)
|
||||
|
||||
return list(filter(is_eligible, self.cluster.members))
|
||||
|
||||
+42
-2
@@ -68,7 +68,8 @@ def get_cluster_initialized_without_leader(leader=False, failover=None, sync=Non
|
||||
'tags': {'clonefrom': True},
|
||||
'scheduled_restart': {'schedule': "2100-01-01 10:53:07.560445+00:00",
|
||||
'postgres_version': '99.0.0'}})
|
||||
syncstate = SyncState(0 if sync else None, sync and sync[0], sync and sync[1], 0)
|
||||
syncstate = SyncState(0 if sync else None, sync and sync[0],
|
||||
sync and sync[1], sync[2] if sync and len(sync) > 2 else 0)
|
||||
failsafe = {m.name: m.api_url for m in (m1, m2)} if failsafe else None
|
||||
return get_cluster(SYSID, leader, [m1, m2], failover, syncstate, cluster_config, failsafe)
|
||||
|
||||
@@ -1015,7 +1016,7 @@ class TestHa(PostgresInit):
|
||||
|
||||
# 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'))
|
||||
sync=('postgresql0', None))
|
||||
self.ha.patroni.nofailover = True
|
||||
self.assertEqual(self.ha.run_cycle(), 'following a different leader because I am not allowed to promote')
|
||||
|
||||
@@ -1844,3 +1845,42 @@ class TestHa(PostgresInit):
|
||||
# Test that _process_quorum_replication doesn't take longer than loop_wait
|
||||
with patch('time.time', Mock(side_effect=[30, 60, 90, 120])):
|
||||
self.ha.process_sync_replication()
|
||||
|
||||
def test_is_failover_possible(self):
|
||||
self.p._major_version = 140000 # supports_multiple_sync
|
||||
self.p.name = 'leader'
|
||||
self.ha.fetch_node_status = get_node_status()
|
||||
self.ha.cluster = get_cluster_initialized_with_leader(sync=('leader', 'foo,other', 1),
|
||||
failover=Failover(0, 'leader', 'other', None))
|
||||
self.ha.cluster.members.append(Member(0, 'foo', 28, {'api_url': 'http://127.0.0.1:8011/patroni'}))
|
||||
# switchover when synchronous_mode = off
|
||||
self.assertTrue(self.ha.is_failover_possible())
|
||||
|
||||
with patch.object(global_config.__class__, 'is_synchronous_mode', PropertyMock(return_value=True)):
|
||||
# switchover to synchronous node when synchronous_mode = on
|
||||
self.assertTrue(self.ha.is_failover_possible())
|
||||
with patch.object(global_config.__class__, 'is_quorum_commit_mode', PropertyMock(return_value=True)):
|
||||
# switchover to synchronous node when synchronous_mode = quorum
|
||||
self.assertTrue(self.ha.is_failover_possible()) # success, despite the fact that quorum is low
|
||||
# failover candidate is unhealthy, we are checking if there are other good candidates, but quorum is low
|
||||
self.assertFalse(self.ha.is_failover_possible(exclude_failover_candidate=True))
|
||||
# now we satisfy quorum requirements
|
||||
with patch.object(SyncState, 'quorum', PropertyMock(return_value=0)):
|
||||
self.assertTrue(self.ha.is_failover_possible(exclude_failover_candidate=True))
|
||||
|
||||
self.ha.cluster = get_cluster_initialized_with_leader(sync=('leader', 'foo,other', 1),
|
||||
failover=Failover(0, '', 'foo', None))
|
||||
# failover to missing node foo
|
||||
self.assertFalse(self.ha.is_failover_possible())
|
||||
|
||||
self.ha.cluster = get_cluster_initialized_with_leader(sync=('leader', 'foo,other', 1),
|
||||
failover=Failover(0, 'leader', '', None))
|
||||
# switchover from leader when synchronous_mode = off
|
||||
self.assertTrue(self.ha.is_failover_possible())
|
||||
|
||||
with patch.object(global_config.__class__, 'is_synchronous_mode', PropertyMock(return_value=True)):
|
||||
# switchover from leader when synchronous_mode = on
|
||||
self.assertTrue(self.ha.is_failover_possible())
|
||||
with patch.object(global_config.__class__, 'is_quorum_commit_mode', PropertyMock(return_value=True)):
|
||||
# switchover from leader when synchronous_mode = quorum
|
||||
self.assertFalse(self.ha.is_failover_possible()) # failure, because quorum is low
|
||||
|
||||
Reference in New Issue
Block a user