junrao commented on code in PR #18685:
URL: https://github.com/apache/kafka/pull/18685#discussion_r1983832557
##########
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
Review Comment:
Hmm, is this because the KRaft client needs to serve ApiVersionRequest
during bootstrap? MV won't be ready during bootstrap, but ApiVersionResponse
needs to include finalized features that depend on MV being available.
##########
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:
Hmm, this doesn't seem correct. Within the same KRaft leader's life time
(i.e. the same epoch), multiple rounds of feature updates could have happened.
It would be incorrect to have the same FinalizedFeaturesEpoch after each round
of update.
--
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
