dajac commented on a change in pull request #11036: URL: https://github.com/apache/kafka/pull/11036#discussion_r672250729
########## File path: core/src/main/scala/kafka/log/LogManager.scala ########## @@ -403,27 +411,49 @@ class LogManager(logDirs: Seq[File], * Start the background threads to flush logs and do log cleanup */ def startup(topicNames: Set[String]): Unit = { - startupWithConfigOverrides(fetchTopicConfigOverrides(topicNames)) + // ensure consistency between default config and overrides + val defaultConfig = currentDefaultConfig + startupWithConfigOverrides(defaultConfig, fetchTopicConfigOverrides(defaultConfig, topicNames)) } // visible for testing - private[log] def fetchTopicConfigOverrides(topicNames: Set[String]): Map[String, LogConfig] = { + @nowarn("cat=deprecation") + private[log] def fetchTopicConfigOverrides(defaultConfig: LogConfig, topicNames: Set[String]): Map[String, LogConfig] = { val topicConfigOverrides = mutable.Map[String, LogConfig]() - val defaultProps = currentDefaultConfig.originals() + val defaultProps = defaultConfig.originals() topicNames.foreach { topicName => - val overrides = configRepository.topicConfig(topicName) + var overrides = configRepository.topicConfig(topicName) Review comment: For my own education, could `Properties` by unmodifiable somehow? I pointed that out because the scaladoc of the `ConfigRepository` trait states the following `@return a copy of the configuration for the given resource`. It seems that the two main implementations respect this however `MockConfigRepository` does not. Therefore, I tend to agree with you. -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org