junrao commented on a change in pull request #11351:
URL: https://github.com/apache/kafka/pull/11351#discussion_r713478871
##########
File path: core/src/main/scala/kafka/server/BrokerServer.scala
##########
@@ -499,12 +499,13 @@ class BrokerServer(
if (clientToControllerChannelManager != null)
CoreUtils.swallow(clientToControllerChannelManager.shutdown(), this)
- if (logManager != null)
- CoreUtils.swallow(logManager.shutdown(), this)
- // be sure to shutdown scheduler after log manager
+ // be sure to shutdown scheduler before log manager
Review comment:
(1) Could we add some comment to explain why we want to shut down the
log manager later?
(2) If we are shutting down, it makes sense to stop the scheduler first
since there is no guarantee any asynchronously scheduled task will complete. If
this is the case, should we shut down the scheduler before any other component
(e.g. ReplicaManger also uses scheduler)?
(3) Once the scheduler is shut down, scheduling new tasks causes
IllegalStateException. That's probably the exception that
https://github.com/apache/kafka/pull/10538 tries to fix. To avoid polluting the
log, perhaps we could change KafkaScheduler such that it avoids throwing an
exception once it's shutdown.
--
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
