junrao commented on a change in pull request #11351:
URL: https://github.com/apache/kafka/pull/11351#discussion_r714188175
##########
File path: core/src/main/scala/kafka/server/BrokerServer.scala
##########
@@ -482,6 +482,20 @@ class BrokerServer(
}
metadataSnapshotter.foreach(snapshotter =>
CoreUtils.swallow(snapshotter.close(), this))
+ /**
+ * We must shutdown the scheduler early because otherwise, the scheduler
could touch other
+ * resources that might have been shutdown and cause exceptions.
+ * For example, if we didn't shutdown the scheduler first, when
LogManager was closing
+ * partitions one by one, the scheduler might concurrently delete old
segments due to
+ * retention. However, the old segments could have been closed by the
LogManager, which would
+ * cause an exception and subsequently mark logdir as offline. As a
result, the broker would
+ * not flush the remaining partitions or write the clean shutdown
marker. Ultimately, the
+ * broker would have to take hours to recover the log during restart and
are subject to
+ * potential data loss.
Review comment:
I don't think unclean shutdown will cause data loss if acks = all is
used. So, we can just remove that statement.
--
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
