junrao commented on code in PR #16347:
URL: https://github.com/apache/kafka/pull/16347#discussion_r1645128202


##########
tools/src/test/java/org/apache/kafka/tools/FeatureCommandTest.java:
##########
@@ -146,7 +146,7 @@ public void 
testDowngradeMetadataVersionWithKRaft(ClusterInstance cluster) {
         );
         // Change expected message to reflect possible MetadataVersion range 
1-N (N increases when adding a new version)
         assertEquals("Could not disable metadata.version. Invalid update 
version 0 for feature " +
-                "metadata.version. Local controller 3000 only supports 
versions 1-21", commandOutput);
+                "metadata.version. Local controller 3000 only supports 
versions 1-23", commandOutput);

Review Comment:
   Should we change IBP_3_7_IV4 and 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)));
   ```



##########
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.latestTesting())

Review Comment:
   Should we change the MV in the following to the new production MV?
   
   ```
     def testApiVersionsRequestValidationV0Template(): 
java.util.List[ClusterConfig] = {
       val serverProperties: java.util.HashMap[String, String] = 
controlPlaneListenerProperties()
       serverProperties.put("unstable.api.versions.enable", "false")
       serverProperties.put("unstable.feature.versions.enable", "false")
       List(ClusterConfig.defaultBuilder()
         .setTypes(java.util.Collections.singleton(Type.ZK))
         .setMetadataVersion(MetadataVersion.IBP_3_7_IV4)
         .build()).asJava
     }
   ```
   
   ```
     @ClusterTest(types = Array(Type.KRAFT, Type.CO_KRAFT), metadataVersion = 
MetadataVersion.IBP_3_7_IV4, serverProperties = Array(
         new ClusterConfigProperty(key = "unstable.api.versions.enable", value 
= "false"),
         new ClusterConfigProperty(key = "unstable.feature.versions.enable", 
value = "false"),
     ))
   
   ```



##########
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_9_IV1;

Review Comment:
   Should we add 3.8-IV0 to the following? The intention seems to be testing 
the latest production MV for each minor release. Then, I don't understand why 
we include 3.6-IV0 instead of 3.6-IV2. Is there a way to avoid manually add the 
latest production MV in the future?
   
   ```
       @ValueSource(strings = {"3.6-IV0", "3.7-IV4"})
       public void testNoLeaderEpochBumpOnIsrShrink(String 
metadataVersionString) {
   ```
   
   ```
       @ValueSource(strings = {"3.6-IV0", "3.7-IV4"})
       public void testLeaderEpochBumpOnIsrShrinkWithZkMigration(String 
metadataVersionString) {
   ```
   
   ```
       @ValueSource(strings = {"3.4-IV0", "3.5-IV2", "3.6-IV0", "3.7-IV4"})
       public void testNoLeaderEpochBumpOnIsrExpansion(String 
metadataVersionString) {
   ```
   
   ```
       @ValueSource(strings = {"3.4-IV0", "3.5-IV2", "3.6-IV0", "3.7-IV4"})
       public void testNoLeaderEpochBumpOnIsrExpansionDuringMigration(String 
metadataVersionString) {
   ```
   
   ```
       @ValueSource(strings = {"3.4-IV0", "3.5-IV2", "3.6-IV0", "3.7-IV4"})
       public void testLeaderEpochBumpOnNewReplicaSetDisjoint(String 
metadataVersionString) {
   ```
   
   ```
       @ValueSource(strings = {"3.4-IV0", "3.5-IV2", "3.6-IV0", "3.7-IV4"})
       public void testNoLeaderEpochBumpOnEmptyTargetIsr(String 
metadataVersionString) {
   ```



##########
server-common/src/main/java/org/apache/kafka/server/common/MetadataVersion.java:
##########
@@ -331,7 +343,7 @@ public boolean isDirectoryAssignmentSupported() {
     }
 
     public boolean isElrSupported() {
-        return this.isAtLeast(IBP_3_8_IV0);
+        return this.isAtLeast(IBP_3_9_IV0);

Review Comment:
   This needs to be IBP_3_9_IV1 now.



-- 
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

Reply via email to