hachikuji commented on a change in pull request #9676:
URL: https://github.com/apache/kafka/pull/9676#discussion_r544611285
##########
File path: core/src/main/scala/kafka/log/Log.scala
##########
@@ -1556,6 +1572,8 @@ class Log(@volatile private var _dir: File,
done = fetchDataInfo != null || segmentEntry == null
}
+ checkForLogDirFailure()
Review comment:
Seems more intuitive to move this check before the segment read. I don't
think we can totally avoid race conditions with a failure in append since we
don't have the lock here. Perhaps we could even move this check to
`maybeHandleIOException` so that we handle all cases?
##########
File path: core/src/test/scala/unit/kafka/log/LogTest.scala
##########
@@ -2818,6 +2818,22 @@ class LogTest {
new SimpleRecord(RecordBatch.NO_TIMESTAMP, "key".getBytes,
"value".getBytes)), leaderEpoch = 0)
}
+ @Test
+ def testAppendToOrReadFromLogInFailedLogDir(): 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)
+ try {
+ log.maybeHandleIOException("Simulating failed log dir") {
Review comment:
Another way to trigger an IO exception is to rename the log file. This
trick is used in `testAppendToTransactionIndexFailure`. Then we don't need to
expose `maybeHandleIOException` for testing.
----------------------------------------------------------------
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
