diff --git a/patroni/ha.py b/patroni/ha.py index a2a206a7..89ecf04f 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -1250,12 +1250,15 @@ class Ha(object): lag = self.cluster.status.last_lsn - wal_position return lag > global_config.maximum_lag_on_failover - def _is_healthiest_node(self, members: Collection[Member], check_replication_lag: bool = True) -> bool: + def _is_healthiest_node(self, members: Collection[Member], + check_replication_lag: bool = True, + leader: Optional[Leader] = None) -> bool: """Determine whether the current node is healthy enough to become a new leader candidate. :param members: the list of nodes to check against :param check_replication_lag: whether to take the replication lag into account. If the lag exceeds configured threshold the node disqualifies itself. + :param leader: the old cluster leader, it will be used to ignore its ``failover_priority`` value. :returns: ``True`` if the node is eligible to become the new leader. Since this method is executed on multiple nodes independently it is possible that multiple nodes could count themselves as the healthiest because they received/replayed up to the same LSN, @@ -1297,6 +1300,12 @@ class Ha(object): quorum_votes = 0 if self.state_handler.name in voting_set else -1 nodes_ahead = 0 + # we need to know the name of the former leader to ignore it if it has higher failover_priority + if self.sync_mode_is_active(): + leader_name = self.cluster.sync.leader + else: + leader_name = leader and leader.name + for st in self.fetch_nodes_statuses(members): if st.failover_limitation() is None: if st.in_recovery is False: @@ -1315,6 +1324,11 @@ class Ha(object): low_priority = my_wal_position == st.wal_position \ and self.patroni.failover_priority < st.failover_priority + if low_priority and leader_name and leader_name == st.member.name: + logger.info('Ignoring former leader %s having priority %s higher than this nodes %s priority', + leader_name, st.failover_priority, self.patroni.failover_priority) + low_priority = False + if low_priority and (not self.sync_mode_is_active() or quorum_vote): # There's a higher priority non-lagging replica logger.info( @@ -1505,7 +1519,7 @@ class Ha(object): # run usual health check members = {m.name: m for m in all_known_members} - return self._is_healthiest_node(members.values()) + return self._is_healthiest_node(members.values(), leader=self.old_cluster.leader) def _delete_leader(self, last_lsn: Optional[int] = None) -> None: self.set_is_leader(False) diff --git a/tests/test_api.py b/tests/test_api.py index 92641a1f..63cffad3 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -18,6 +18,7 @@ from patroni.psycopg import OperationalError from patroni.utils import RetryFailedError, tzutc from . import MockConnect, psycopg_connect +from .test_etcd import socket_getaddrinfo from .test_ha import get_cluster_initialized_without_leader future_restart_time = datetime.datetime.now(tzutc) + datetime.timedelta(days=5) @@ -711,6 +712,7 @@ class TestRestApiServer(unittest.TestCase): patch.object(MockRestApiServer, 'server_close', Mock()): self.srv.reload_config({'listen': ':8008'}) + @patch('socket.getaddrinfo', socket_getaddrinfo) @patch.object(MockPatroni, 'dcs') def test_check_access(self, mock_dcs): mock_dcs.cluster = get_cluster_initialized_without_leader() diff --git a/tests/test_ha.py b/tests/test_ha.py index 2e41f5dd..b5803982 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -1076,6 +1076,9 @@ class TestHa(PostgresInit): # if there is a higher-priority node but it has a lower WAL position then this node should race self.ha.fetch_node_status = get_node_status(failover_priority=6, wal_position=9) self.assertTrue(self.ha._is_healthiest_node(self.ha.old_cluster.members)) + # if the old leader is a higher-priority node on the same WAL position then this node should race + self.ha.fetch_node_status = get_node_status(failover_priority=6) + self.assertTrue(self.ha._is_healthiest_node(self.ha.old_cluster.members, leader=self.ha.old_cluster.leader)) self.ha.fetch_node_status = get_node_status(wal_position=11) # accessible, in_recovery, wal position ahead self.assertFalse(self.ha._is_healthiest_node(self.ha.old_cluster.members)) # in synchronous_mode consider itself healthy if the former leader is accessible in read-only and ahead of us