KAFKA-10545: Create topic IDs in ZooKeeper and Controller#9473
KAFKA-10545: Create topic IDs in ZooKeeper and Controller#9473rajinisivaram merged 10 commits intoapache:trunkfrom
Conversation
rajinisivaram
left a comment
rajinisivaram
left a comment
There was a problem hiding this comment.
@jolshan Thanks for the PR, looks good overall. Left some minor comments.
| if (topicIds.get(topic)!= None) { | ||
| topicNames.remove(topicIds.get(topic).get) | ||
| topicIds.remove(topic) | ||
| } |
There was a problem hiding this comment.
We could do:
topicIds.remove(topic).foreach { topicId =>
topicNames.remove(topicId)
}
There was a problem hiding this comment.
I was thinking about this some more, and I don't think we will have a case where the topic ID is not in the map (the if conditional is not needed.) However, the suggestion you gave is still good because it prevents the extra lookup.
… getTopicIdsForTopics throws IllegalStateException if one of the topics does not have a topic ID.
| var epochZkVersion: Int = KafkaController.InitialControllerEpochZkVersion | ||
|
|
||
| val allTopics = mutable.Set.empty[String] | ||
| var topicIds = mutable.Map.empty[String, UUID] |
There was a problem hiding this comment.
Should these 2 names be more self-explanatory?
There was a problem hiding this comment.
What part of the names is unclear? The name describes values of the map, so is it not clear what the keys should be?
There was a problem hiding this comment.
Well, topicIds and topicNames may already enough self-explanatory.
rajinisivaram
left a comment
rajinisivaram
left a comment
There was a problem hiding this comment.
@jolshan Thanks for the updates, LGTM
|
@jolshan Thanks for the updates, merging to trunk. |
4 participants
Topic IDs must be created for all new topics and all existing topics that do not yet have a topic ID. In ZooKeeper, the ID is written to the TopicZNode, and in the controller, it is stored in a map.
This is a preliminary change before the second part of KAFKA-10545, which will propagate these IDs to brokers.
Committer Checklist (excluded from commit message)