Cyrill commented on code in PR #7507:
URL: https://github.com/apache/ignite-3/pull/7507#discussion_r2872926642
##########
modules/raft/src/test/java/org/apache/ignite/internal/raft/client/PhysicalTopologyAwareRaftGroupServiceRunTest.java:
##########
@@ -880,49 +828,222 @@ private void verifyExact3PeersCalled() {
assertThat(triedPeers, equalTo(expectedPeers));
}
- private static class TestResponse {
+ //
==========================================================================================
+ // Tests for refreshLeader and refreshAndGetLeaderWithTerm (RANDOM
strategy)
+ //
==========================================================================================
+
+ /**
+ * Tests that for GetLeaderRequest with UNKNOWN/EINTERNAL/ENOENT errors,
the executor tries another peer
+ * instead of retrying the same peer. This matches RaftGroupServiceImpl
behavior.
+ *
+ * <p>The test verifies that after receiving one of these errors from a
peer, the next request goes to
+ * a DIFFERENT peer, not the same one.
+ */
+ @ParameterizedTest
+ @EnumSource(names = {"UNKNOWN", "EINTERNAL", "ENOENT"})
+ void testGetLeaderRequestTriesDifferentPeerOnTransientError(RaftError
error) throws Exception {
+ Set<String> calledPeers = ConcurrentHashMap.newKeySet();
+ AtomicInteger callCount = new AtomicInteger(0);
+
+ when(messagingService.invoke(
+ any(InternalClusterNode.class),
+ any(GetLeaderRequest.class),
+ anyLong())
+ ).thenAnswer(invocation -> {
+ InternalClusterNode target = invocation.getArgument(0);
+ calledPeers.add(target.name());
+ int count = callCount.incrementAndGet();
+
+ if (count == 1) {
+ // First call returns transient error.
+ return completedFuture(FACTORY.errorResponse()
+ .errorCode(error.getNumber())
+ .build());
+ }
+
+ // Second call succeeds.
+ return completedFuture(FACTORY.getLeaderResponse()
+ .leaderId(PeerId.fromPeer(NODES.get(0)).toString())
+ .currentTerm(CURRENT_TERM)
+ .build());
+ });
+
+ PhysicalTopologyAwareRaftGroupService svc = startService();
+
+ CompletableFuture<LeaderWithTerm> result =
svc.refreshAndGetLeaderWithTerm(TIMEOUT);
+
+ assertThat(result, willCompleteSuccessfully());
+
+ // Verify that at least 2 different peers were called.
+ // If the same peer was retried, calledPeers would have size 1.
+ assertThat("Should try different peers on " + error + " error, but
called peers were: " + calledPeers,
+ calledPeers.size(), greaterThan(1));
}
/**
- * Tests single-attempt mode (timeout=0) with 5 nodes: all return "no
leader".
+ * Tests that RANDOM strategy with bounded timeout keeps cycling through
peers until timeout.
*
- * <p>In single-attempt mode, "no leader" is treated same as unavailable.
- * Each peer is tried exactly once, then fails with
ReplicationGroupUnavailableException.
+ * <p>When all peers return EPERM (no leader), RaftGroupServiceImpl resets
unavailable peers
+ * and keeps cycling until timeout. RaftCommandExecutor should do the same
for RANDOM strategy.
*/
@Test
- void testSingleAttemptModeWithAllNoLeader() {
+ void testRandomStrategyWithBoundedTimeoutKeepsCyclingUntilTimeout() throws
Exception {
+ AtomicInteger callCount = new AtomicInteger(0);
+ CountDownLatch sixCallsReached = new CountDownLatch(6);
Review Comment:
changed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
