ahuang98 commented on code in PR #18240:
URL: https://github.com/apache/kafka/pull/18240#discussion_r1915429555
##########
raft/src/test/java/org/apache/kafka/raft/UnattachedStateTest.java:
##########
@@ -77,72 +84,134 @@ public void testElectionTimeout() {
@ParameterizedTest
@ValueSource(booleans = {true, false})
- public void testGrantVote(boolean isLogUpToDate) {
- UnattachedState state = newUnattachedState(Set.of(1, 2, 3),
OptionalInt.empty());
+ public void testGrantVoteWithoutVotedKey(boolean isLogUpToDate) {
+ UnattachedState state = newUnattachedState(OptionalInt.empty(),
Optional.empty());
assertEquals(
isLogUpToDate,
- state.canGrantVote(ReplicaKey.of(1, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
+ state.canGrantVote(voter1Key, isLogUpToDate, true)
);
+ assertEquals(
+ isLogUpToDate,
+ state.canGrantVote(voter1Key, isLogUpToDate, false)
+ );
+
assertEquals(
isLogUpToDate,
state.canGrantVote(ReplicaKey.of(2, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
);
assertEquals(
isLogUpToDate,
- state.canGrantVote(ReplicaKey.of(3, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
+ state.canGrantVote(ReplicaKey.of(2, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, false)
);
assertEquals(
isLogUpToDate,
- state.canGrantVote(ReplicaKey.of(1, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, false)
+ state.canGrantVote(ReplicaKey.of(3, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
);
assertEquals(
isLogUpToDate,
- state.canGrantVote(ReplicaKey.of(2, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, false)
+ state.canGrantVote(ReplicaKey.of(3, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, false)
);
+
assertEquals(
isLogUpToDate,
- state.canGrantVote(ReplicaKey.of(3, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, false)
+ state.canGrantVote(ReplicaKey.of(10, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
+ );
+ assertEquals(
+ isLogUpToDate,
+ state.canGrantVote(ReplicaKey.of(10, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, false)
);
}
- @Test
- void testLeaderEndpoints() {
- UnattachedState state = newUnattachedState(Set.of(1, 2, 3),
OptionalInt.empty());
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testCanGrantVoteWithVotedKey(boolean isLogUpToDate) {
+ UnattachedState state = newUnattachedState(OptionalInt.empty(),
Optional.of(votedKey));
- assertEquals(Endpoints.empty(), state.leaderEndpoints());
+ // Same voterKey
+ // Local can reject PreVote for a replica that local has already
granted a standard vote to if their log is behind
+ assertEquals(
+ isLogUpToDate,
+ state.canGrantVote(votedKey, isLogUpToDate, true)
+ );
+ assertTrue(state.canGrantVote(votedKey, isLogUpToDate, false));
+
+ // Different directoryId
+ // Local can grant PreVote for a replica that local has already
granted a standard vote to if their log is up-to-date,
+ // even if the directoryId is different
+ assertEquals(
+ isLogUpToDate,
+ state.canGrantVote(ReplicaKey.of(votedKey.id(),
Uuid.randomUuid()), isLogUpToDate, true)
+ );
+ assertFalse(state.canGrantVote(ReplicaKey.of(votedKey.id(),
Uuid.randomUuid()), isLogUpToDate, false));
+
+ // Missing directoryId
+ assertEquals(
+ isLogUpToDate,
+ state.canGrantVote(ReplicaKey.of(votedKey.id(),
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, true)
+ );
+ assertFalse(state.canGrantVote(ReplicaKey.of(votedKey.id(),
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, false));
+
+ // Different voterId
+ assertEquals(
+ isLogUpToDate,
+ state.canGrantVote(ReplicaKey.of(2, votedKey.directoryId().get()),
isLogUpToDate, true)
+ );
+ assertEquals(
+ isLogUpToDate,
+ state.canGrantVote(ReplicaKey.of(2, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
+ );
+ assertFalse(state.canGrantVote(ReplicaKey.of(2,
votedKey.directoryId().get()), isLogUpToDate, false));
+ assertFalse(state.canGrantVote(ReplicaKey.of(2,
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, false));
+
+ // Observer
+ assertEquals(
+ isLogUpToDate,
+ state.canGrantVote(ReplicaKey.of(10, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
+ );
+ assertFalse(state.canGrantVote(ReplicaKey.of(10,
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, false));
}
@ParameterizedTest
@ValueSource(booleans = {true, false})
- void testUnattachedWithLeader(boolean isLogUpToDate) {
+ void testGrantVoteWithLeader(boolean isLogUpToDate) {
int leaderId = 3;
- Set<Integer> voters = Set.of(1, 2, leaderId);
-
- UnattachedState state = newUnattachedState(voters,
OptionalInt.of(leaderId));
+ UnattachedState state = newUnattachedState(OptionalInt.of(leaderId),
Optional.empty());
// Check that the leader is persisted if the leader is known
- assertEquals(ElectionState.withElectedLeader(epoch, leaderId, voters),
state.election());
+ assertEquals(ElectionState.withElectedLeader(epoch, leaderId,
Optional.empty(), voters), state.election());
// Check that the replica can grant PreVotes if the log is up-to-date,
even if the last leader is known
// This is because nodes in Unattached have not successfully fetched
from the leader yet
assertEquals(
isLogUpToDate,
- state.canGrantVote(ReplicaKey.of(1, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
+ state.canGrantVote(voter1Key, isLogUpToDate, true)
);
assertEquals(
isLogUpToDate,
state.canGrantVote(ReplicaKey.of(2, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
);
assertEquals(
isLogUpToDate,
- state.canGrantVote(ReplicaKey.of(3, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
+ state.canGrantVote(ReplicaKey.of(leaderId,
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, true)
+ );
+ assertEquals(
+ isLogUpToDate,
+ state.canGrantVote(ReplicaKey.of(10, ReplicaKey.NO_DIRECTORY_ID),
isLogUpToDate, true)
);
// Check that the replica rejects all standard votes request if the
leader is known
assertFalse(state.canGrantVote(ReplicaKey.of(1,
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, false));
assertFalse(state.canGrantVote(ReplicaKey.of(2,
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, false));
- assertFalse(state.canGrantVote(ReplicaKey.of(3,
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, false));
+ assertFalse(state.canGrantVote(ReplicaKey.of(leaderId,
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, false));
+ assertFalse(state.canGrantVote(ReplicaKey.of(10,
ReplicaKey.NO_DIRECTORY_ID), isLogUpToDate, false));
+ }
+
+ @Test
+ public void testLeaderEndpoints() {
+ UnattachedState state = newUnattachedState(OptionalInt.of(3),
Optional.of(this.votedKey));
+
+ assertEquals(Endpoints.empty(), state.leaderEndpoints());
Review Comment:
Clobbered `UnattachedStateWithVoteTest.java` into
`UnattachedStateTest.java`.
I didn't think it was necessary to have a separate file (both having
votedKey state and leaderId state are tested in this one file). I wanted to
prevent introducing a separate WithVoteTest for ProspectiveState as well
--
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]
