hachikuji commented on a change in pull request #9676:
URL: https://github.com/apache/kafka/pull/9676#discussion_r538686784
##########
File path: core/src/main/scala/kafka/server/LogDirFailureChannel.scala
##########
@@ -49,6 +49,13 @@ class LogDirFailureChannel(logDirNum: Int) extends Logging {
offlineLogDirQueue.add(logDir)
}
+ /*
+ * Return whether the given log dir is offline.
+ */
+ def logDirIsOffline(logDir: String): Boolean = {
+ offlineLogDirs.containsKey(logDir)
Review comment:
I am not sure if it is really necessary, but since offline dirs are a
rare situation, I'm wondering if makes sense to optimize for the common case to
avoid the lookup. For example, maybe we could leave `offlineLogDirs`
uninitialized until the first log dir failure.
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -1106,6 +1106,13 @@ class Log(@volatile private var _dir: File,
// they are valid, insert them in the log
lock synchronized {
checkIfMemoryMappedBufferClosed()
+
+ // check for offline log dir in case a retry following an
IOException happens before the log dir
Review comment:
Hmm.. In case there is an IOException on an append, we will release the
lock and fail the log dir in `maybeHandleIOException`. There is a window for
another append to sneak by. It looks like it should be possible to pull
`maybeHandleIOException` into the locked section here since
`analyzeAndValidateRecords` does not do any IO.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
