chia7712 commented on code in PR #16843:
URL: https://github.com/apache/kafka/pull/16843#discussion_r1758133515
##########
core/src/main/scala/kafka/server/ControllerServer.scala:
##########
@@ -512,13 +500,11 @@ class ControllerServer(
CoreUtils.swallow(controllerApis.close(), this)
if (quotaManagers != null)
CoreUtils.swallow(quotaManagers.shutdown(), this)
- if (controller != null)
- controller.close()
- if (quorumControllerMetrics != null)
- CoreUtils.swallow(quorumControllerMetrics.close(), this)
- CoreUtils.swallow(authorizer.foreach(_.close()), this)
- createTopicPolicy.foreach(policy => CoreUtils.swallow(policy.close(),
this))
- alterConfigPolicy.foreach(policy => CoreUtils.swallow(policy.close(),
this))
+ Utils.closeQuietly(controller, "controller")
+ Utils.closeQuietly(quorumControllerMetrics, "quorum controller metrics")
+ authorizer.foreach(Utils.closeQuietly(_, "authorizer"))
+ createTopicPolicy.foreach(policy => Utils.closeQuietly(policy, "topic
policy"))
Review Comment:
`create topic policy`
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -756,7 +756,7 @@ class BrokerServer(
// Close remote log manager to give a chance to any of its underlying
clients
// (especially in RemoteStorageManager and RemoteLogMetadataManager) to
close gracefully.
- CoreUtils.swallow(remoteLogManagerOpt.foreach(_.close()), this)
+ remoteLogManagerOpt.foreach(Utils.closeQuietly(_, "remote log manager
option"))
Review Comment:
`remote log manager`
##########
core/src/main/scala/kafka/server/ZkAdminManager.scala:
##########
@@ -547,8 +547,8 @@ class ZkAdminManager(val config: KafkaConfig,
def shutdown(): Unit = {
topicPurgatory.shutdown()
- CoreUtils.swallow(createTopicPolicy.foreach(_.close()), this)
- CoreUtils.swallow(alterConfigPolicy.foreach(_.close()), this)
+ createTopicPolicy.foreach(Utils.closeQuietly(_, "topic policy"))
Review Comment:
`create topic policy`
##########
core/src/main/scala/kafka/server/KafkaServer.scala:
##########
@@ -1051,13 +1051,12 @@ class KafkaServer(
// Close remote log manager before stopping processing requests, to
give a chance to any
// of its underlying clients (especially in RemoteStorageManager and
RemoteLogMetadataManager)
// to close gracefully.
- CoreUtils.swallow(remoteLogManagerOpt.foreach(_.close()), this)
+ remoteLogManagerOpt.foreach(Utils.closeQuietly(_, "remote log manager
option"))
Review Comment:
`remote log manager`
##########
core/src/main/scala/kafka/server/KafkaServer.scala:
##########
@@ -1068,8 +1067,7 @@ class KafkaServer(
if (socketServer != null)
CoreUtils.swallow(socketServer.shutdown(), this)
unregisterCurrentControllerIdMetric()
- if (metrics != null)
- CoreUtils.swallow(metrics.close(), this)
+ Utils.closeQuietly(metrics, "metrics")
if (brokerTopicStats != null)
CoreUtils.swallow(brokerTopicStats.close(), this)
Review Comment:
Could you please make `brokerTopicStats` extend `Closeable`?
--
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
