jsancio commented on code in PR #23185:
URL: https://github.com/apache/kafka/pull/23185#discussion_r3814530926
##########
raft/src/main/java/org/apache/kafka/raft/RaftUtil.java:
##########
@@ -715,6 +715,14 @@ static boolean hasValidTopicPartition(FetchResponseData
data, TopicPartition top
data.responses().get(0).partitions().get(0).partitionIndex() ==
topicPartition.partition();
}
+ static boolean hasValidTopicPartition(FetchSnapshotRequestData data) {
+ return data.topics().size() == 1 &&
data.topics().get(0).partitions().size() == 1;
+ }
+
+ static boolean hasValidTopicPartition(FetchSnapshotResponseData data) {
Review Comment:
Thanks for the fix @kevin-wu24 . Let's include unit tests for these new
methods.
##########
raft/src/test/java/org/apache/kafka/raft/KafkaRaftClientSnapshotTest.java:
##########
@@ -767,6 +767,51 @@ public void
testFetchSnapshotRequestUnknownPartition(boolean withKip853Rpc) thro
assertEquals(Errors.UNKNOWN_TOPIC_OR_PARTITION,
Errors.forCode(response.errorCode()));
}
+ @ParameterizedTest
+ @ValueSource(booleans = { false, true })
+ public void testFetchSnapshotRequestWithoutTopics(boolean withKip853Rpc)
throws Exception {
+ int localId = randomReplicaId();
+ Set<Integer> voters = Set.of(localId, localId + 1);
+
+ RaftClientTestContext context = new
RaftClientTestContext.Builder(localId, voters)
+ .withUnknownLeader(3)
+ .withKip853Rpc(withKip853Rpc)
Review Comment:
Is this relevant for this test? Why not pick the latest RPC version? This
comment also applies to the other test added to this suite.
`withKip853Rpc` was deprecated in favor of `withRaftProtocol`.
--
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]
