chia7712 commented on code in PR #19371:
URL: https://github.com/apache/kafka/pull/19371#discussion_r2100943381
##########
core/src/main/scala/kafka/raft/KafkaMetadataLog.scala:
##########
@@ -589,8 +589,9 @@ object KafkaMetadataLog extends Logging {
): KafkaMetadataLog = {
val props = new Properties()
props.setProperty(TopicConfig.MAX_MESSAGE_BYTES_CONFIG,
config.maxBatchSizeInBytes.toString)
- props.setProperty(TopicConfig.SEGMENT_BYTES_CONFIG,
config.logSegmentBytes.toString)
- props.setProperty(TopicConfig.SEGMENT_MS_CONFIG,
config.logSegmentMillis.toString)
+ // Since MetadataLogConfig validates the log segment size using a minimum
allowed value,
+ // we can safely use `internal.segment.bytes` to configure it instead of
relying on `segment.bytes`.
+ props.setProperty(LogConfig.INTERNAL_SEGMENT_BYTES_CONFIG,
config.logSegmentBytes.toString)
Review Comment:
> This will fail LogConfig.validate(props) if a test passes in a small log
segment value, right?
This won't happen because this PR removes support for configuring small
segments for Kraft topics. The minimum Kraft topic size is now 8MB, which is
large enough to pass LogConfig.validate(props).
We've removed the configuration directly to streamline the PR, as there are
no existing tests in code base for small segment Kraft topics.
If configuring small segments for kraft topic becomes necessary, we will
need to reintroduce broker-level internal log configuration, as topic-level
configuration is not available for kraft topic.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
