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


##########
core/src/main/scala/kafka/server/metadata/KRaftMetadataCache.scala:
##########
@@ -522,10 +522,14 @@ class KRaftMetadataCache(
     if (kraftVersionLevel > 0) {
       finalizedFeatures.put(KRaftVersion.FEATURE_NAME, kraftVersionLevel)
     }
+    var metadataVersion = MetadataVersion.MINIMUM_VERSION
+    if (!image.features().metadataVersion().isEmpty) {
+      metadataVersion = image.features().metadataVersionOrThrow()
+    }
     new FinalizedFeatures(
-      image.features().metadataVersionOrThrow(),
+      metadataVersion,
       finalizedFeatures,
-      image.highestOffsetAndEpoch().offset)
+      image.highestOffsetAndEpoch().epoch())

Review Comment:
   @chia7712 : The description of FinalizedFeaturesEpoch is the following.
   
   `The monotonically increasing epoch for the finalized features information`
   
   In the client, the way that FinalizedFeaturesEpoch is used is to decide 
which finalized features returned in ApiResponse is the latest.  So, if a 
feature is updated, FinalizedFeaturesEpoch needs to go up. The reverse doesn't 
need to be true, i.e. if FinalizedFeaturesEpoch goes up, it doesn't necessarily 
need to have a corresponding feature update.
   
   In the ZK implementation, the reverse happens to be true. In the KRaft 
implementation, the reverse is not true, but this is ok.
   
   



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