showuon commented on a change in pull request #11246:
URL: https://github.com/apache/kafka/pull/11246#discussion_r693292145
##########
File path: core/src/main/scala/kafka/server/KafkaServer.scala
##########
@@ -611,7 +619,7 @@ class KafkaServer(
// ignore and try again
}
}
- if (!shutdownSucceeded) {
+ if (!shutdownSucceeded && remainingRetries > 0) {
Thread.sleep(config.controlledShutdownRetryBackoffMs)
warn("Retrying controlled shutdown after the previous attempt
failed...")
Review comment:
As we expected that even there's no error returned, the shutdown might
still not complete because there are some partitions remaining to move, not
"failure" mentioned here in the warning message. Maybe we just need to log with
`info` with `s"Retrying controlled shutdown with $remainingRetries retries
left`.
##########
File path: core/src/main/scala/kafka/server/KafkaServer.scala
##########
@@ -594,13 +594,21 @@ class KafkaServer(
val clientResponse =
NetworkClientUtils.sendAndReceive(networkClient, request, time)
val shutdownResponse =
clientResponse.responseBody.asInstanceOf[ControlledShutdownResponse]
- if (shutdownResponse.error == Errors.NONE &&
shutdownResponse.data.remainingPartitions.isEmpty) {
+ if (shutdownResponse.error != Errors.NONE) {
+ info(s"Controlled shutdown request returned after
${clientResponse.requestLatencyMs}ms " +
+ s"with error ${shutdownResponse.error}")
Review comment:
Should we log as error level or warn level?
--
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
