[GitHub] [kafka] rondagostino commented on a change in pull request #10039: MINOR: Defer log recovery until LogManager startup

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

rondagostino commented on a change in pull request #10039:
URL: https://github.com/apache/kafka/pull/10039#discussion_r569580157



##########
File path: core/src/main/scala/kafka/log/LogManager.scala
##########
@@ -402,7 +403,26 @@ class LogManager(logDirs: Seq[File],
   /**
    *  Start the background threads to flush logs and do log cleanup
    */
-  def startup(): Unit = {
+  def startup(retrieveTopicNames: () => Set[String]): Unit = {
+    startup(generateTopicLogConfigs(retrieveTopicNames))
+  }
+
+  // visible for testing
+  private[log] def generateTopicLogConfigs(retrieveTopicNames: () => Set[String]): Map[String, LogConfig] = {
+    val topicLogConfigs: mutable.Map[String, LogConfig] = mutable.Map()
+    val defaultProps = currentDefaultConfig.originals()
+    retrieveTopicNames().foreach { topicName =>
+      val overrides = configRepository.topicConfigs(topicName)
+      val logConfig = LogConfig.fromProps(defaultProps, overrides)
+      topicLogConfigs(topicName) = logConfig
+    }

Review comment:
       I wonder about the efficiency of this loop when using ZooKeeper.  Today we ask ZooKeeper for all of the topics, then we ask ZooKeeper for all of the configs for all of those topics.  However, under the hood it does appear that `KafkaZkClient.getTopicConfigs(Set)` is iterating through all of the topics and querying each one individually.  So I raise it just to get another pair of eyes to confirm that this explicit loop will not perform worse than what appears to be a single round-trip but is in fact not.




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