junrao commented on code in PR #15673:
URL: https://github.com/apache/kafka/pull/15673#discussion_r1624752626
##########
server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java:
##########
@@ -185,6 +185,7 @@ public void testFromVersionString() {
assertEquals(IBP_3_7_IV4,
MetadataVersion.fromVersionString("3.7-IV4"));
assertEquals(IBP_3_8_IV0,
MetadataVersion.fromVersionString("3.8-IV0"));
+ assertEquals(IBP_3_8_IV1,
MetadataVersion.fromVersionString("3.8-IV1"));
Review Comment:
Should we add a comment that IBP_3_8_IV1 is the latest product version of
3.8?
##########
server-common/src/main/java/org/apache/kafka/server/common/TestFeatureVersion.java:
##########
@@ -24,7 +24,7 @@ public enum TestFeatureVersion implements FeatureVersion {
// TEST_1 released right before MV 3.7-IVO was released, and it has no
dependencies
TEST_1(1, MetadataVersion.IBP_3_7_IV0, Collections.emptyMap()),
// TEST_2 released right before MV 3.8-IVO was released, and it depends on
this metadata version
- TEST_2(2, MetadataVersion.IBP_3_8_IV0,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_3_8_IV0.featureLevel()));
+ TEST_2(2, MetadataVersion.IBP_4_0_IVO,
Collections.singletonMap(MetadataVersion.FEATURE_NAME,
MetadataVersion.IBP_4_0_IVO.featureLevel()));
Review Comment:
@jolshan : Should we keep the MV for TEST_2 fixed at IBP_3_8_IV0?
##########
server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java:
##########
@@ -297,6 +298,7 @@ public void testVersion() {
assertEquals("3.7-IV3", IBP_3_7_IV3.version());
assertEquals("3.7-IV4", IBP_3_7_IV4.version());
assertEquals("3.8-IV0", IBP_3_8_IV0.version());
+ assertEquals("3.8-IV1", IBP_3_8_IV1.version());
Review Comment:
1. Should we add `IBP_3_8_IV1` in `testShortVersion()` too?
2. Should we change `testIsElrSupported()` to use `IBP_4_0_IVO`?
3. Should we update
`FeatureCommandTest.testDescribeWithKRaftAndBootstrapControllers` from
`metadataVersion = MetadataVersion.IBP_3_7_IV4` to `metadataVersion =
MetadataVersion.IBP_3_8_IV1`?
##########
clients/src/main/resources/common/message/ListOffsetsRequest.json:
##########
@@ -34,7 +34,9 @@
// Version 7 enables listing offsets by max timestamp (KIP-734).
//
// Version 8 enables listing offsets by local log start offset (KIP-405).
- "validVersions": "0-8",
+ //
+ // Version 9 enables listing offsets by last tiered offset (KIP-1005).
Review Comment:
To let the AdminClient use this, we need to add a new type of OffsetSpec and
a way to set oldestAllowedVersion in ListOffsetsRequest.Build to version 9,
right? See https://github.com/apache/kafka/pull/10760/files
##########
metadata/src/test/java/org/apache/kafka/controller/PartitionChangeBuilderTest.java:
##########
@@ -125,7 +125,7 @@ private static MetadataVersion
metadataVersionForPartitionChangeRecordVersion(sh
case (short) 1:
return MetadataVersion.IBP_3_7_IV2;
case (short) 2:
- return MetadataVersion.IBP_3_8_IV0;
+ return MetadataVersion.IBP_3_8_IV1;
Review Comment:
We need to change it to return `IBP_4_0_IVO` for version 2 of
PartitionChangeRecord since the version 2 change is for ELR.
--
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]
