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