chia7712 commented on code in PR #23316:
URL: https://github.com/apache/kafka/pull/23316#discussion_r3982150006
##########
tools/src/main/java/org/apache/kafka/tools/DumpLogSegments.java:
##########
@@ -394,7 +394,7 @@ private static void dumpLog(File file,
printTrailingBytes(fileRecords, validBytes, maxBytes, file);
} finally {
if (fileRecords != null) {
- fileRecords.closeHandlers();
+ fileRecords.close();
Review Comment:
ditto
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/AbstractIndex.java:
##########
@@ -252,12 +241,15 @@ public void renameTo(File f) throws IOException {
}
/**
- * Flush the data in the index to disk
+ * Flush the data in the index and its metadata to disk
*/
- public void flush() {
+ public void flush() throws IOException {
inLock(() -> {
if (mmap != null) {
mmap.force();
+ try (FileChannel channel = FileChannel.open(file.toPath(),
StandardOpenOption.WRITE)) {
Review Comment:
I'm a bit concerned that `flush()` runs outside `UnifiedLog`'s lock, so it
is possible for flush() to encounter a `NoSuchFileException`, like the one
handled in `LocalLog#flush`.
##########
tools/src/main/java/org/apache/kafka/tools/DumpLogSegments.java:
##########
@@ -312,13 +312,13 @@ static void dumpTimeIndex(File file,
}
} finally {
if (fileRecords != null) {
Review Comment:
This seems to be a good use case for `closeAll` after these changes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
