chia7712 commented on code in PR #15863:
URL: https://github.com/apache/kafka/pull/15863#discussion_r1610083521
##########
core/src/main/scala/kafka/log/LogCleaner.scala:
##########
@@ -224,7 +232,7 @@ class LogCleaner(initialConfig: CleanerConfig,
throttler.updateDesiredRatePerSec(maxIoBytesPerSecond)
}
- shutdown()
+ shutdownCleaners()
Review Comment:
Please add comments to say why we call `shutdownCleaners` instead of
`shutdown`
##########
core/src/main/scala/kafka/log/LogCleaner.scala:
##########
@@ -164,13 +164,21 @@ class LogCleaner(initialConfig: CleanerConfig,
/**
* Stop the background cleaner threads
*/
- def shutdown(): Unit = {
+ private[this] def shutdownCleaners(): Unit = {
info("Shutting down the log cleaner.")
+ cleaners.foreach(_.shutdown())
+ cleaners.clear()
+ }
+
+ /**
+ * Stop the background cleaner threads
+ */
+ def shutdown(): Unit = {
try {
- cleaners.foreach(_.shutdown())
- cleaners.clear()
+ shutdownCleaners()
} finally {
removeMetrics()
+ info("Shutting down the log cleaner.")
Review Comment:
this is duplicate to line#168
##########
core/src/test/scala/unit/kafka/log/LogCleanerTest.scala:
##########
@@ -1942,6 +1963,7 @@ class LogCleanerTest extends Logging {
logs = new Pool[TopicPartition, UnifiedLog](),
logDirFailureChannel = new LogDirFailureChannel(1),
time = time)
+ logCleaner.startup()
Review Comment:
This is unnecessary now, right?
--
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
