KAFKA-10869: Gate topic IDs behind IBP 2.8#9814
Conversation
|
|
||
| private def writeTopicPartitionAssignment(topic: String, replicaAssignment: Map[Int, ReplicaAssignment], isUpdate: Boolean): Unit = { | ||
| private def writeTopicPartitionAssignment(topic: String, replicaAssignment: Map[Int, ReplicaAssignment], | ||
| isUpdate: Boolean, usesTopicId: Boolean = false): Unit = { |
There was a problem hiding this comment.
Why do you set the default value to false? I don't think we should give it a default value since the caller can decide it by caller.
There was a problem hiding this comment.
Some places that eventually call this do not have access to IBP. The controller will eventually pick up and assign topic IDs that need them.
| partitionReplicaAssignment: Map[Int, Seq[Int]], | ||
| validate: Boolean = true): Unit = { | ||
| validate: Boolean = true, | ||
| usesTopicId: Boolean = false): Unit = { |
There was a problem hiding this comment.
So if we use TopicCommand with ZookeeperTopicService to create a topic, we will always not set a topicId?
There was a problem hiding this comment.
Initially yes. The controller will process and add topic IDs to these topics if IBP is at least 2.8 in processTopicChange() using the processTopicIds method.
rajinisivaram
left a comment
rajinisivaram
left a comment
There was a problem hiding this comment.
@jolshan Thanks for the PR, looks good. Left some minor comments.
core/src/test/scala/unit/kafka/controller/ControllerIntegrationTest.scala
Outdated
Show resolved
Hide resolved
core/src/test/scala/unit/kafka/controller/ControllerIntegrationTest.scala
Outdated
Show resolved
Hide resolved
core/src/test/scala/unit/kafka/controller/ControllerIntegrationTest.scala
Outdated
Show resolved
Hide resolved
core/src/test/scala/unit/kafka/controller/ControllerIntegrationTest.scala
Outdated
Show resolved
Hide resolved
core/src/test/scala/unit/kafka/controller/ControllerIntegrationTest.scala
Show resolved
Hide resolved
rajinisivaram
left a comment
rajinisivaram
left a comment
There was a problem hiding this comment.
@jolshan Thanks for the updates, LGTM. Can you confirm that the upgrade and downgrade system tests passed with the PR before I merge?
|
@jolshan Thanks for running the system tests, merging to trunk. |
4 participants
Topics processed by the controller and topics newly created will only be given topic IDs if the inter-broker protocol version on the controller is greater than 2.8. This PR also adds a kafka config to specify whether the IBP is greater or equal to 2.8. System tests have been modified to include topic ID checks for upgrade/downgrade tests.
This PR also adds a new integration test file for requests/responses that are not gated by IBP (ex: metadata) This file can be expanded to check topic IDs are handled correctly when the request/response version uses topic IDs but the brokers are not able to handle them.
Committer Checklist (excluded from commit message)