ahuang98 commented on code in PR #17807:
URL: https://github.com/apache/kafka/pull/17807#discussion_r1882712890


##########
raft/src/test/java/org/apache/kafka/raft/UnattachedStateWithVoteTest.java:
##########
@@ -83,35 +83,67 @@ public void testElectionTimeout() {
     public void testCanGrantVoteWithoutDirectoryId(boolean isLogUpToDate) {
         UnattachedState state = 
newUnattachedVotedState(ReplicaKey.NO_DIRECTORY_ID);
 
-        assertTrue(
-            state.canGrantVote(ReplicaKey.of(votedId, 
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate)
+        assertEquals(
+            isLogUpToDate,
+            state.canGrantPreVote(ReplicaKey.of(votedId, 
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate)
         );
-        assertTrue(
-            state.canGrantVote(
-                ReplicaKey.of(votedId, Uuid.randomUuid()),
-                isLogUpToDate
-            )
+        assertTrue(state.canGrantVote(ReplicaKey.of(votedId, 
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate));
+
+        assertEquals(
+            isLogUpToDate,
+            state.canGrantPreVote(ReplicaKey.of(votedId, Uuid.randomUuid()), 
isLogUpToDate)
         );
+        assertTrue(state.canGrantVote(ReplicaKey.of(votedId, 
Uuid.randomUuid()), isLogUpToDate));
 
-        assertFalse(
-            state.canGrantVote(ReplicaKey.of(votedId + 1, 
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate)
+        // Can grant PreVote to other replicas even if we have granted a 
standard vote to another replica
+        assertEquals(
+            isLogUpToDate,
+            state.canGrantPreVote(ReplicaKey.of(votedId + 1, 
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate)
         );
+        assertFalse(state.canGrantVote(ReplicaKey.of(votedId + 1, 
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate));
     }
 
-    @Test
-    void testCanGrantVoteWithDirectoryId() {
+    @ParameterizedTest
+    @ValueSource(booleans = {true, false})
+    void testCanGrantVoteWithDirectoryId(boolean isLogUpToDate) {
         Uuid votedDirectoryId = Uuid.randomUuid();
         UnattachedState state = newUnattachedVotedState(votedDirectoryId);
 
-        assertTrue(state.canGrantVote(ReplicaKey.of(votedId, 
votedDirectoryId), false));
+        // Same voterKey
+        // We will not grant PreVote for a replica we have already granted a 
standard vote to if their log is behind
+        assertEquals(
+            isLogUpToDate,
+            state.canGrantPreVote(ReplicaKey.of(votedId, votedDirectoryId), 
isLogUpToDate)
+        );

Review Comment:
   perhaps the caveat "if their log is behind" was missed. I think the 
following reads a bit better -
   ```
           // We can reject PreVote for a replica we have already granted a 
standard vote to if their log is behind
   ```



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to