hachikuji commented on a change in pull request #10021:
URL: https://github.com/apache/kafka/pull/10021#discussion_r591870272
##########
File path: core/src/main/scala/kafka/raft/KafkaMetadataLog.scala
##########
@@ -35,21 +34,14 @@ import scala.compat.java8.OptionConverters._
final class KafkaMetadataLog private (
log: Log,
+ config: LogConfig,
Review comment:
I'm slightly more inclined to pass through the configuration value as a
parameter in the constructor here rather than getting in the habit of relying
on `LogConfig`. We're in a an awkward state at the moment in regard to
`LogConfig`. We depend on some of the configurations, but others we ignore. It
relates to this JIRA I filed:
https://issues.apache.org/jira/browse/KAFKA-12423. My thinking for now is
perhaps to try and keep the coupling with `LogConfig` from getting too tight.
Maybe we could do something like this in `apply`:
```scala
val metadataLog = new KafkaMetadataLog(
log,
recoverSnapshots(log),
topicPartition,
maxFetchSizeInBytes,
defaultLogConfig.fileDeleteDelayMs
)
```
This is probably borderline overkill, but the point is to try and keep the
configurations that we rely on in this layer separable.
----------------------------------------------------------------
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:
us...@infra.apache.org
