[GitHub] [kafka] hachikuji commented on a change in pull request #10282: KAFKA-12426: Missing logic to create partition.metadata files in RaftReplicaManager

[GitBox <gi...@apache.org>]

hachikuji commented on a change in pull request #10282:
URL: https://github.com/apache/kafka/pull/10282#discussion_r605822416



##########
File path: core/src/main/scala/kafka/log/LogManager.scala
##########
@@ -842,6 +842,20 @@ class LogManager(logDirs: Seq[File],
 
         log
       }
+      // When running a ZK controller, we may get a log that does not have a topic ID. Assign it here.
+      if (log.topicId.isEmpty) {
+        topicId.foreach(log.assignTopicId)
+      }
+
+      // Ensure topic IDs are consistent
+      topicId.foreach { topicId =>
+        log.topicId.foreach { logTopicId =>
+          if (topicId != logTopicId)
+            throw new IllegalStateException(s"Tried to assign topic ID $topicId to log for topic partition $topicPartition," +

Review comment:
       Perhaps we may as well use InconsistentTopicIdException?

##########
File path: core/src/main/scala/kafka/server/ReplicaManager.scala
##########
@@ -1369,7 +1377,10 @@ class ReplicaManager(val config: KafkaConfig,
               val requestLeaderEpoch = partitionState.leaderEpoch
               val requestTopicId = topicIds.get(topicPartition.topic)

Review comment:
       Could we simplify this by taking advantage of `getTopicId`?

##########
File path: core/src/main/scala/kafka/server/ReplicaManager.scala
##########
@@ -1326,6 +1326,14 @@ class ReplicaManager(val config: KafkaConfig,
             s"epoch ${leaderAndIsrRequest.controllerEpoch}")
         }
       val topicIds = leaderAndIsrRequest.topicIds()
+      def getTopicId(topicName: String): Option[Uuid] = {

Review comment:
       nit: how about `requestTopicId` instead of `getTopicId` so that it's clear that this comes from the LeaderAndIsr request?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org