Cyrill commented on code in PR #7286:
URL: https://github.com/apache/ignite-3/pull/7286#discussion_r2650919481


##########
modules/raft/src/test/java/org/apache/ignite/internal/raft/client/PhysicalTopologyAwareRaftGroupServiceRunTest.java:
##########
@@ -294,35 +302,41 @@ void testZeroTimeoutSuccessWhenLeaderAvailable() {
     }
 
     /**
-     * Tests that with timeout=0, the command throws 
ReplicationGroupUnavailableException when no leader and all peers tried.
+     * Tests that with timeout=0, all peers are tried before failing with 
ReplicationGroupUnavailableException.
      */
     @Test
-    void testZeroTimeoutFailWhenNoLeader() {
+    void testZeroTimeoutTriesAllPeersBeforeFailing() {
+        // All peers return EPERM with no leader.
         mockUserInputNoLeader();
 
         PhysicalTopologyAwareRaftGroupService svc = startService();
 
-        // No leader election simulated - service is in WAITING_FOR_LEADER 
state.
-        // With timeout=0, should fail immediately.
+        // With timeout=0, should try each peer once and then fail.
         CompletableFuture<Object> result = svc.run(testWriteCommand(), 0);
 
         assertThat(result, 
willThrow(ReplicationGroupUnavailableException.class));
+
+        verifyExact3PeersCalled();
     }
 
-    /**
-     * Tests that with timeout=0, all peers are tried before failing.
-     */
-    @Test
-    void testZeroTimeoutTriesAllPeersBeforeFailing() {
-        // All peers return EPERM with no leader.
-        mockUserInputNoLeader();
+    private void verifyExact3PeersCalled() {

Review Comment:
   done



##########
modules/raft/src/test/java/org/apache/ignite/internal/raft/client/PhysicalTopologyAwareRaftGroupServiceRunTest.java:
##########
@@ -436,6 +472,217 @@ void 
testBoundedTimeoutSucceedsWhenLeaderAppearsBeforeTimeout() {
         assertThat(result, willCompleteSuccessfully());
     }
 
+    /**
+     * Tests that with bounded timeout, the command fails if peer responses 
are slow and exceed the deadline.
+     */
+    @Test
+    void testBoundedTimeoutExpiresWhileTryingPeers() {
+        // Each peer response takes 200ms and returns EPERM (no leader).
+        // With 3 peers and 500ms timeout, we should exceed the deadline.
+        when(messagingService.invoke(
+                any(InternalClusterNode.class),
+                argThat(this::isTestWriteCommand),
+                anyLong()
+        )).thenAnswer(invocation -> {
+            return CompletableFuture.supplyAsync(() -> {
+                try {
+                    Thread.sleep(200);
+                } catch (InterruptedException e) {
+                    Thread.currentThread().interrupt();
+                }
+                return FACTORY.errorResponse()
+                        .errorCode(RaftError.EPERM.getNumber())
+                        .build();
+            }, executor);
+        });
+
+        PhysicalTopologyAwareRaftGroupService svc = startService();
+
+        // With 500ms timeout and 3 peers each taking 200ms, total time ~600ms 
> 500ms.
+        CompletableFuture<Object> result = svc.run(testWriteCommand(), 500);
+
+        assertThat(result, 
willThrow(ReplicationGroupUnavailableException.class, 2, TimeUnit.SECONDS));
+    }
+
+    /**
+     * Tests that individual peer request timeouts are bounded by the deadline.
+     *
+     * <p>If the deadline is 500ms but the default response timeout is 3000ms, 
the first peer request
+     * should timeout at ~500ms, not wait the full 3000ms.
+     */
+    @Test
+    void testBoundedTimeoutLimitsIndividualPeerRequestTimeout() {
+        // First peer never responds (simulating network issue).
+        // Default response timeout is 3000ms, but our deadline is 500ms.
+        when(messagingService.invoke(
+                any(InternalClusterNode.class),
+                argThat(this::isTestWriteCommand),
+                anyLong()
+        )).thenReturn(new CompletableFuture<>()); // Never completes
+
+        PhysicalTopologyAwareRaftGroupService svc = startService();
+
+        long startTime = System.currentTimeMillis();
+
+        // With 500ms timeout, should fail within ~600ms (500ms + some 
margin), not 3000ms.
+        CompletableFuture<Object> result = svc.run(testWriteCommand(), 500);
+
+        assertThat(result, 
willThrow(ReplicationGroupUnavailableException.class, 1, TimeUnit.SECONDS));
+
+        long elapsed = System.currentTimeMillis() - startTime;
+        // Should complete much faster than the default response timeout 
(3000ms).
+        assertTrue(elapsed < 1500, "Expected to fail within 1500ms but took " 
+ elapsed + "ms");
+    }
+
+    // ==================== Shutdown Tests ====================

Review Comment:
   done



-- 
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]

Reply via email to