hachikuji commented on code in PR #12181:
URL: https://github.com/apache/kafka/pull/12181#discussion_r894150026
##########
core/src/main/scala/kafka/server/ApiVersionManager.scala:
##########
@@ -73,18 +73,35 @@ class DefaultApiVersionManager(
) extends ApiVersionManager {
override def apiVersionResponse(throttleTimeMs: Int): ApiVersionsResponse = {
+ val metadataVersion = metadataCache.metadataVersion()
val supportedFeatures = features.supportedFeatures
val finalizedFeatures = metadataCache.features()
val controllerApiVersions =
forwardingManager.flatMap(_.controllerApiVersions)
- ApiVersionsResponse.createApiVersionsResponse(
+ val response = ApiVersionsResponse.createApiVersionsResponse(
throttleTimeMs,
- metadataCache.metadataVersion().highestSupportedRecordVersion,
+ metadataVersion.highestSupportedRecordVersion,
supportedFeatures,
finalizedFeatures.features.map(kv => (kv._1,
kv._2.asInstanceOf[java.lang.Short])).asJava,
finalizedFeatures.epoch,
controllerApiVersions.orNull,
- listenerType)
+ listenerType
+ )
+
+ // In ZK mode if the deployed software of the controller uses version 2.8
or above
+ // but the IBP is below 2.8, the controller does not assign topic ids. In
this case,
+ // it should not advertise the AlterPartition API version 2 and above.
+ val alterPartitionApiVersion =
response.apiVersion(ApiKeys.ALTER_PARTITION.id)
+ if (alterPartitionApiVersion != null) {
+ alterPartitionApiVersion.setMaxVersion(
+ if (metadataVersion.isTopicIdsSupported)
+ alterPartitionApiVersion.maxVersion()
+ else
+ 1.toShort
+ )
+ }
Review Comment:
I recall there were some tricky cases when doing the upgrade to use
TopicIds. It is possible for the controller to initialize on one of the nodes
with the updated IBP and create TopicIds for all topics, but then change to a
new node with a lower IBP. How does the controller handle the existence of
TopicIds in the zk metadata if the IBP is below 2.8? At a quick glance, it
looks like it would still parse it and load it into the `ControllerContext`. It
seems ok in this scenario for the controller to accept `AlterPartition` with
TopicIds. Are there any other scenarios we need to worry about? Perhaps
`upgrade_test.py` is sufficient to test this scenario?
--
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]
