clolov commented on code in PR #14161:
URL: https://github.com/apache/kafka/pull/14161#discussion_r1290107019
##########
core/src/main/scala/kafka/server/ConfigHandler.scala:
##########
@@ -62,6 +62,12 @@ class TopicConfigHandler(private val logManager: LogManager,
kafkaConfig: KafkaC
topicConfig.asScala.forKeyValue { (key, value) =>
if (!configNamesToExclude.contains(key)) props.put(key, value)
}
+
+ if (!kafkaConfig.remoteLogManagerConfig.enableRemoteStorageSystem()
Review Comment:
I actually prefer that we keep values from KafkaConfig (system-wide Tiered
Storage configuration) and LogConfig (topic-specific Tiered Storage
configuration) logically separate rather than insert one in the other. In other
words, I would prefer that the validation between KafkaConfig(s) and
LogConfig(s) happens in a place where values can be pulled from both and
compared with one another (like what this PR is doing) until we introduce a
mechanism of dependence between configurations from different sources.
Of course, if I have missed a KRaft configuration which appears to have a
similar logic as what we are trying to achieve here and it has been put in the
LogConfig I can be persuaded to change my mind.
My reasoning comes from default values (log.segment.bytes and
log.segment.ms) in KafkaConfig being pulled from more specific configs (for
example the segment.bytes and segment.ms) rather than the values from
KafkaConfig being supplied to the LogConfig and being overwritten there.
--
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
