chia7712 commented on code in PR #15863:
URL: https://github.com/apache/kafka/pull/15863#discussion_r1605365153
##########
core/src/main/scala/kafka/log/LogCleaner.scala:
##########
@@ -182,6 +183,27 @@ class LogCleaner(initialConfig: CleanerConfig,
cleanerManager.removeMetrics()
Review Comment:
> We can removed the cleanerManager.removeMetrics() then we should also
removed
[this](https://github.com/apache/kafka/blob/fafa3c76dc93f3258b2cea49dfd1dc7a724a213c/core/src/test/scala/unit/kafka/log/LogCleanerTest.scala#L96)
in the testRemoveMetricsOnClose().
My point was - we don't need to re-create the metrics after reconfiguration
- i.e in reconfiguration we SHOULD NOT remove the metrics. For example,
```scala
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 shutdownCleaners()
finally removeMetrics()
info("Shutting down the log cleaner.")
}
```
With above changes, in `reconfigure` we call `shutdownCleaners` instead of
`shutdown`. WDYT?
--
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
