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


##########
metadata/src/main/java/org/apache/kafka/controller/QuorumFeatures.java:
##########
@@ -63,7 +63,7 @@ public static Map<String, VersionRange> 
defaultFeatureMap(boolean enableUnstable
                     MetadataVersion.latestTesting().featureLevel() :
                     MetadataVersion.latestProduction().featureLevel()));
         for (Feature feature : Feature.PRODUCTION_FEATURES) {
-            short maxVersion = enableUnstable ? feature.latestTesting() : 
feature.latestProduction();
+            short maxVersion = enableUnstable ? feature.latestTesting() : 
feature.defaultLevel(MetadataVersion.LATEST_PRODUCTION);

Review Comment:
   @jolshan : Not sure that I understand this change. The result of 
`defaultFeatureMap` is used for Controller/Broker registration. So, it seems 
that we should pass in the max supported version of each feature, instead of 
the default version, right? In fact, `defaultFeatureMap` should be renamed to 
sth like `supportedFeatureMap`.
   
   > A test was failing since the production version for transaction version is 
now not the same as the default based on the latest production MV.
   
   Hmm, I thought that with https://github.com/apache/kafka/pull/17886, it's ok 
for the latest production version for TV to be different from the default. It 
just needs to be larger.



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

Reply via email to