diff --git a/patroni/ha.py b/patroni/ha.py index 09c4f2a6..c073933e 100644 --- a/patroni/ha.py +++ b/patroni/ha.py @@ -912,6 +912,27 @@ class Ha(object): lag = (self.cluster.last_lsn or 0) - wal_position return lag > self.global_config.maximum_lag_on_failover + def has_members_eligible_to_promote(self, members: List[Member], reference_lsn: int = 0, + fast_path: bool = False) -> bool: + ret = False + cluster_timeline = self.cluster.timeline + + for st in self.fetch_nodes_statuses(members): + not_allowed_reason = st.failover_limitation() + if not_allowed_reason: + logger.info('Member %s is %s', st.member.name, not_allowed_reason) + elif fast_path: + return True + elif reference_lsn and st.wal_position < reference_lsn or \ + not reference_lsn and self.is_lagging(st.wal_position): + logger.info('Member %s exceeds maximum replication lag', st.member.name) + elif self.check_timeline() and (not st.timeline or st.timeline < cluster_timeline): + logger.info('Timeline %s of member %s is behind the cluster timeline %s', + st.timeline, st.member.name, cluster_timeline) + else: + ret = True + return ret + def _is_healthiest_node(self, members: Collection[Member], check_replication_lag: bool = True) -> bool: """This method tries to determine whether I am healthy enough to became a new leader candidate or not.""" @@ -964,21 +985,7 @@ class Ha(object): elif not candidates: logger.warning('%s: candidates list is empty', action) - ret = False - cluster_timeline = self.cluster.timeline - for st in self.fetch_nodes_statuses(candidates): - not_allowed_reason = st.failover_limitation() - if not_allowed_reason: - logger.info('Member %s is %s', st.member.name, not_allowed_reason) - elif cluster_lsn and st.wal_position < cluster_lsn or \ - not cluster_lsn and self.is_lagging(st.wal_position): - logger.info('Member %s exceeds maximum replication lag', st.member.name) - elif self.check_timeline() and (not st.timeline or st.timeline < cluster_timeline): - logger.info('Timeline %s of member %s is behind the cluster timeline %s', - st.timeline, st.member.name, cluster_timeline) - else: - ret = True - return ret + return self.has_members_eligible_to_promote(candidates, cluster_lsn) def manual_failover_process_no_leader(self) -> Optional[bool]: """Handles manual failover/switchover when the old leader already stepped down. diff --git a/patroni/manual_failover.py b/patroni/manual_failover.py index 96af819a..9225cbb9 100644 --- a/patroni/manual_failover.py +++ b/patroni/manual_failover.py @@ -87,11 +87,7 @@ class ManualFailover(object): if not members: return ManualFailoverPrecheckStatus.ONLY_LEADER - if self.patroni: - for st in self.patroni.ha.fetch_nodes_statuses(members): - if st.failover_limitation() is None: - break - else: - return ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES + if self.patroni and not self.patroni.ha.has_members_eligible_to_promote(members, fast_path=True): + return ManualFailoverPrecheckStatus.NO_GOOD_CANDIDATES return ManualFailoverPrecheckStatus.CHECK_PASSED diff --git a/tests/test_api.py b/tests/test_api.py index 5025dd61..fe7270bc 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -122,6 +122,9 @@ class MockHa(object): def is_paused(): return True + def has_members_eligible_to_promote(*args, **kwargs): + return True + class MockLogger(object): @@ -599,7 +602,7 @@ class TestRestApiHandler(unittest.TestCase): dcs.manual_failover.return_value = True # Candidate is not healthy to be promoted - with patch.object(MockHa, 'fetch_nodes_statuses', Mock(return_value=[])), \ + with patch.object(MockHa, 'has_members_eligible_to_promote', Mock(return_value=False)), \ patch.object(RestApiHandler, 'write_response') as response_mock: MockRestApiServer(RestApiHandler, request) response_mock.assert_called_with(412, 'switchover is not possible: no good candidates have been found') diff --git a/tests/test_ha.py b/tests/test_ha.py index 3bb3fee0..a8c53176 100644 --- a/tests/test_ha.py +++ b/tests/test_ha.py @@ -1626,3 +1626,13 @@ class TestHa(PostgresInit): self.assertEqual(self.ha.patroni.request.call_args[1]['timeout'], 2) mock_logger.assert_called() self.assertTrue(mock_logger.call_args[0][0].startswith('Request to Citus coordinator')) + + def test_has_members_eligible_to_promote(self): + self.ha.fetch_node_status = get_node_status() + members = [ + Member(0, 'test', 1, {'api_url': 'http://127.0.0.1:8011/patroni', 'conn_url': 'postgres://127.0.0.1:5432/postgres'}), + Member(0, 'test2', 1, {'api_url': 'http://127.0.0.1:8011/patroni', 'conn_url': 'postgres://127.0.0.1:5432/postgres'}), + ] + with patch('patroni.ha.logger.info') as mock_logger: + self.assertTrue(self.ha.has_members_eligible_to_promote(members, fast_path=True)) + mock_logger.assert_not_called()