ahuang98 commented on code in PR #16637:
URL: https://github.com/apache/kafka/pull/16637#discussion_r1690603037
##########
raft/src/test/java/org/apache/kafka/raft/KafkaRaftClientTest.java:
##########
@@ -2762,79 +2764,357 @@ public void testDescribeQuorumNonLeader(boolean
withKip853Rpc) throws Exception
assertEquals(Errors.NOT_LEADER_OR_FOLLOWER.message(),
partitionData.errorMessage());
}
+ @Test
+ public void testDescribeQuorumWithOnlyStaticVoters() throws Exception {
+ int localId = 0;
+ ReplicaKey local = replicaKey(localId, true);
+ ReplicaKey follower1 = replicaKey(1, true);
+ Set<Integer> voters = Utils.mkSet(localId, follower1.id());
+
+ RaftClientTestContext context = new
RaftClientTestContext.Builder(localId, local.directoryId().get())
+ .withStaticVoters(voters)
+ .withKip853Rpc(true)
+ .build();
+
+ context.becomeLeader();
+ int epoch = context.currentEpoch();
+
+ // Describe quorum response will not include directory ids
+ context.deliverRequest(context.describeQuorumRequest());
+ context.pollUntilResponse();
+ List<ReplicaState> expectedVoterStates = Arrays.asList(
+ new ReplicaState()
+ .setReplicaId(localId)
+ .setReplicaDirectoryId(ReplicaKey.NO_DIRECTORY_ID)
+ .setLogEndOffset(1L)
+ .setLastFetchTimestamp(context.time.milliseconds())
+ .setLastCaughtUpTimestamp(context.time.milliseconds()),
+ new ReplicaState()
+ .setReplicaId(follower1.id())
+ .setReplicaDirectoryId(ReplicaKey.NO_DIRECTORY_ID)
+ .setLogEndOffset(-1L)
+ .setLastFetchTimestamp(-1)
+ .setLastCaughtUpTimestamp(-1));
+ context.assertSentDescribeQuorumResponse(localId, epoch, -1L,
expectedVoterStates, Collections.emptyList());
+ }
+
+
@ParameterizedTest
@ValueSource(booleans = { true, false })
- public void testDescribeQuorum(boolean withKip853Rpc) throws Exception {
- int localId = randomReplicaId();
- ReplicaKey closeFollower = replicaKey(localId + 2, withKip853Rpc);
- ReplicaKey laggingFollower = replicaKey(localId + 1, withKip853Rpc);
- Set<Integer> voters = Utils.mkSet(localId, closeFollower.id(),
laggingFollower.id());
-
- RaftClientTestContext context = new
RaftClientTestContext.Builder(localId, voters)
- .withKip853Rpc(withKip853Rpc)
- .build();
+ public void testDescribeQuorumWithFollowers(boolean withKip853Rpc) throws
Exception {
+ int localId = 0;
+ ReplicaKey local = replicaKey(localId, true);
+ Uuid localDirectoryId = local.directoryId().get();
+ ReplicaKey follower1 = replicaKey(1, true);
+ Uuid followerDirectoryId1 = follower1.directoryId().get();
+ ReplicaKey follower2 = replicaKey(2, false);
+ Set<Integer> voters = Utils.mkSet(localId, follower1.id(),
follower2.id());
+ VoterSet voterSet = VoterSetTest.voterSet(Stream.of(local, follower1,
follower2));
+
+ RaftClientTestContext.Builder builder = new
RaftClientTestContext.Builder(localId, localDirectoryId)
+ .withStaticVoters(voters)
+ .withKip853Rpc(withKip853Rpc);
+
+ if (withKip853Rpc) {
+ builder.withBootstrapSnapshot(Optional.of(voterSet));
Review Comment:
My reasoning was that it shouldn't break to set both (we'll ignore static
when we have a bootstrap checkpoint), but I can be more explicit about it -
i.e. make it either or for all existing tests and add
`testDescribeQuorumIgnoresStaticVoters` which shows we'll ignore static voters
if bootstrap checkpoint exists.
--
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
