hachikuji commented on a change in pull request #9676:
URL: https://github.com/apache/kafka/pull/9676#discussion_r534502282
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -1219,6 +1219,9 @@ class Log(@volatile private var _dir: File,
appendInfo.logAppendTime = duplicate.timestamp
appendInfo.logStartOffset = logStartOffset
case None =>
+ if (logDirFailureChannel.logDirIsFailed(parentDir)) {
Review comment:
Perhaps we could move this to somewhere near the top? I don't think we
get much benefit by delaying the check since duplicates would be a rare case.
We probably don't want to have to trust the producer state anyway after an
append failure.
##########
File path: core/src/test/scala/unit/kafka/log/LogTest.scala
##########
@@ -2818,6 +2818,21 @@ class LogTest {
new SimpleRecord(RecordBatch.NO_TIMESTAMP, "key".getBytes,
"value".getBytes)), leaderEpoch = 0)
}
+ @Test
+ def testAppendToLogInFailedLogDir(): Unit = {
+ val log = createLog(logDir, LogConfig())
+ log.appendAsLeader(TestUtils.singletonRecords(value = null), leaderEpoch =
0)
+ assertEquals(0, readLog(log, 0,
4096).records.records.iterator.next().offset)
+ log.logDirFailureChannel.maybeAddOfflineLogDir(logDir.getParent,
"Simulating failed log dir", new IOException("Test failure"))
+ try {
+ log.appendAsLeader(TestUtils.singletonRecords(value = null), leaderEpoch
= 0)
+ fail()
Review comment:
nit: we can use `assertThrows` or `intercept`
##########
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 logDirIsFailed(logDir: String): Boolean = {
Review comment:
Maybe `isOffline`?
----------------------------------------------------------------
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
