junrao commented on code in PR #16347:
URL: https://github.com/apache/kafka/pull/16347#discussion_r1644855151
##########
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##########
@@ -331,7 +340,7 @@ public boolean isDirectoryAssignmentSupported() {
}
public boolean isElrSupported() {
- return this.isAtLeast(IBP_3_8_IV0);
+ return this.isAtLeast(IBP_3_9_IV0);
Review Comment:
Since 3.9 is a short release, will ELR be supported in 3.9?
##########
core/src/test/scala/unit/kafka/server/ApiVersionsRequestTest.scala:
##########
@@ -47,7 +47,7 @@ object ApiVersionsRequestTest {
List(ClusterConfig.defaultBuilder()
.setTypes(java.util.Collections.singleton(Type.ZK))
.setServerProperties(serverProperties)
- .setMetadataVersion(MetadataVersion.IBP_4_0_IV0)
+ .setMetadataVersion(MetadataVersion.IBP_3_9_IV0)
Review Comment:
Could we add some comments on which MV this test should set? Is it the first
unstable MV? Could we add sth like MetadataVersion.earliestTesting() to avoid
having to keep changing it?
##########
clients/src/main/resources/common/message/ListOffsetsRequest.json:
##########
@@ -34,9 +34,12 @@
// 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).
+ "validVersions": "0-9",
"deprecatedVersions": "0",
"flexibleVersions": "6+",
+ "latestVersionUnstable": true,
Review Comment:
@clolov : Is this change ok with you for 3.9?
@cmccabe : I guess this part won't be included in 3.8?
##########
server-common/src/test/java/org/apache/kafka/server/common/MetadataVersionTest.java:
##########
@@ -184,8 +184,11 @@ public void testFromVersionString() {
assertEquals(IBP_3_7_IV3,
MetadataVersion.fromVersionString("3.7-IV3"));
assertEquals(IBP_3_7_IV4,
MetadataVersion.fromVersionString("3.7-IV4"));
+ assertEquals(IBP_3_8_IV0, MetadataVersion.fromVersionString("3.8"));
Review Comment:
Should we change the following comment accordingly?
`// 3.7-IV4 is the latest production version in the 3.7 line`
##########
metadata/src/test/java/org/apache/kafka/metadata/PartitionRegistrationTest.java:
##########
@@ -371,7 +371,7 @@ public void
testPartitionRegistrationToRecord_ElrShouldBeNullIfEmpty() {
setPartitionEpoch(0);
List<UnwritableMetadataException> exceptions = new ArrayList<>();
ImageWriterOptions options = new ImageWriterOptions.Builder().
- setMetadataVersion(MetadataVersion.IBP_3_8_IV0).
+ setMetadataVersion(MetadataVersion.IBP_3_9_IV0).
Review Comment:
We have the following code in this class. Should we change IBP_3_8_IV0 to
IBP_3_9_IV0?
```
private static Stream<Arguments>
metadataVersionsForTestPartitionRegistration() {
return Stream.of(
MetadataVersion.IBP_3_7_IV1,
MetadataVersion.IBP_3_7_IV2,
MetadataVersion.IBP_3_8_IV0
).map(mv -> Arguments.of(mv));
}
```
##########
core/src/test/scala/integration/kafka/zk/ZkMigrationIntegrationTest.scala:
##########
@@ -73,7 +73,7 @@ object ZkMigrationIntegrationTest {
MetadataVersion.IBP_3_7_IV2,
MetadataVersion.IBP_3_7_IV4,
MetadataVersion.IBP_3_8_IV0,
- MetadataVersion.IBP_4_0_IV0
+ MetadataVersion.IBP_3_9_IV0
Review Comment:
(1) I guess we don't support ZK migration from 4.0. Could we add a comment?
(2) We have the following code in this class. Should we bump IBP_3_8_IV0 to
IBP_3_9_IV0?
`@ClusterTest(types = Array(Type.ZK), brokers = 3, metadataVersion =
MetadataVersion.IBP_3_8_IV0, serverProperties = Array(
`
##########
tools/src/test/java/org/apache/kafka/tools/FeatureCommandTest.java:
##########
@@ -321,4 +321,4 @@ public void testHandleDisableDryRun() {
"Can not disable metadata.version. Can't downgrade below 4%n" +
"quux can be disabled."), disableOutput);
}
-}
\ No newline at end of file
+}
Review Comment:
Is this change needed?
##########
server-common/src/test/java/org/apache/kafka/server/common/FeaturesTest.java:
##########
@@ -110,7 +110,7 @@ public void
testLatestProductionMapsToLatestMetadataVersion(Features features) {
@EnumSource(MetadataVersion.class)
public void testDefaultTestVersion(MetadataVersion metadataVersion) {
short expectedVersion;
- if (!metadataVersion.isLessThan(MetadataVersion.IBP_3_8_IV0)) {
+ if (!metadataVersion.isLessThan(MetadataVersion.IBP_3_9_IV0)) {
Review Comment:
Should we change 3.7.-IV4 in the following code to IBP_3_8_0?
`@ClusterTest(types = {Type.KRAFT}, metadataVersion =
MetadataVersion.IBP_3_7_IV4)`
` "SupportedMaxVersion: 4.0-IV0\tFinalizedVersionLevel:
3.7-IV4\t", outputWithoutEpoch(features.get(1)));
`
--
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
