jsancio commented on code in PR #17801:
URL: https://github.com/apache/kafka/pull/17801#discussion_r1900255597
##########
raft/src/test/java/org/apache/kafka/raft/RaftUtilTest.java:
##########
@@ -335,6 +335,27 @@ public void testSingletonFetchRequestForAllVersion(final
FetchRequestTestCase te
assertEquals(testCase.expectedJson, json.toString());
}
+ // Test that the replicaDirectoryId field introduced in version 17 is
ignorable for older versions.
+ // This is done by setting a FetchPartition's replicaDirectoryId
explicitly to a non-zero uuid and
+ // checking that the FetchRequestData can still be written to an older
version specified by
+ // testCase.version.
+ @ParameterizedTest
+ @MethodSource("singletonFetchRequestTestCases")
+ public void testFetchRequestV17Compatibility(final FetchRequestTestCase
testCase) {
+ FetchRequestData fetchRequestData =
RaftUtil.singletonFetchRequest(topicPartition, Uuid.ONE_UUID,
+ partition -> partition
+ .setPartitionMaxBytes(10)
+ .setCurrentLeaderEpoch(5)
+ .setFetchOffset(333)
+ .setLastFetchedEpoch(testCase.lastFetchedEpoch)
+ .setPartition(2)
+ .setReplicaDirectoryId(Uuid.ONE_UUID)
+ .setLogStartOffset(0)
+ );
Review Comment:
Minor formatting issue:
```java
FetchRequestData fetchRequestData = RaftUtil.singletonFetchRequest(
topicPartition,
Uuid.ONE_UUID,
partition -> partition
.setPartitionMaxBytes(10)
.setCurrentLeaderEpoch(5)
.setFetchOffset(333)
.setLastFetchedEpoch(testCase.lastFetchedEpoch)
.setPartition(2)
.setReplicaDirectoryId(Uuid.ONE_UUID)
.setLogStartOffset(0)
);
```
##########
raft/src/test/java/org/apache/kafka/raft/RaftUtilTest.java:
##########
@@ -437,6 +458,33 @@ public void
testSingletonFetchSnapshotRequestForAllVersion(final short version,
assertEquals(expectedJson, json.toString());
}
+ // Test that the replicaDirectoryId field introduced in version 1 is
ignorable for version 0
+ // This is done by setting a FetchPartition's replicaDirectoryId
explicitly to a non-zero uuid and
+ // checking that the FetchSnapshotRequestData can still be written to an
older version specified by
+ // testCase.version.
+ @ParameterizedTest
+ @MethodSource("fetchSnapshotRequestTestCases")
+ public void testSingletonFetchSnapshotRequestV1Compatibility(final short
version,
+ final Uuid
directoryId,
+ final String
expectedJson) {
Review Comment:
Please use this formatting:
```java
public void testSingletonFetchSnapshotRequestV1Compatibility(
short version,
Uuid directoryId,
String expectedJson
) {
```
##########
raft/src/test/java/org/apache/kafka/raft/RaftUtilTest.java:
##########
@@ -437,6 +458,33 @@ public void
testSingletonFetchSnapshotRequestForAllVersion(final short version,
assertEquals(expectedJson, json.toString());
}
+ // Test that the replicaDirectoryId field introduced in version 1 is
ignorable for version 0
+ // This is done by setting a FetchPartition's replicaDirectoryId
explicitly to a non-zero uuid and
+ // checking that the FetchSnapshotRequestData can still be written to an
older version specified by
+ // testCase.version.
+ @ParameterizedTest
+ @MethodSource("fetchSnapshotRequestTestCases")
+ public void testSingletonFetchSnapshotRequestV1Compatibility(final short
version,
+ final Uuid
directoryId,
+ final String
expectedJson) {
+ int epoch = 1;
+ int maxBytes = 1000;
+ int position = 10;
+
+ FetchSnapshotRequestData fetchSnapshotRequestData =
RaftUtil.singletonFetchSnapshotRequest(
+ clusterId,
+ ReplicaKey.of(1, directoryId),
+ topicPartition,
+ epoch,
+ new OffsetAndEpoch(10, epoch),
+ maxBytes,
+ position
+ );
Review Comment:
How about this formatting:
```java
FetchSnapshotRequestData fetchSnapshotRequestData =
RaftUtil.singletonFetchSnapshotRequest(
clusterId,
ReplicaKey.of(1, directoryId),
topicPartition,
epoch,
new OffsetAndEpoch(10, epoch),
maxBytes,
position
);
```
--
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]
