junrao commented on code in PR #23258:
URL: https://github.com/apache/kafka/pull/23258#discussion_r3883823259
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/AbstractIndex.java:
##########
@@ -216,6 +225,9 @@ public boolean resize(int newSize) throws IOException {
mmap =
raf.getChannel().map(FileChannel.MapMode.READ_WRITE, 0, roundedNewSize);
this.maxEntries = mmap.limit() / entrySize();
mmap.position(position);
+ if (sync) {
Review Comment:
It seems the rolled inactive segment is not covered by this. When we roll a
segment, LogSegment.onBecomeInactiveSegment() calls
offsetIndex().trimToValidSize(false). This leaves the index file length
metadata unflushed and set AbstractIndex.length to roundedNewSize. This has a
couple of problems.
(1) During a clean shutdown, we will call close on a rolled segment. Since
its AbstractIndex.length == roundedNewSize, it won't trigger the flushing of
the file length metadata.
(2) During a periodic flush, we call flush() on a rolled segment, which
won't flush the file length metadata for the index file. If the broke has a
hard failure, that rolled segment may not have the right file length and it
won't be recovered since its offset is below the recovery offset.
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/AbstractIndex.java:
##########
@@ -261,13 +275,13 @@ public boolean deleteIfExists() throws IOException {
}
/**
- * Trim this segment to fit just the valid entries, deleting all trailing
unwritten bytes from
- * the file.
+ * Trim this index to fit just the valid entries, deleting all trailing
unwritten bytes from the file.
+ * @param sync if true, fsync the file after resizing to ensure both
content and size are durable
Review Comment:
This comment is a bit mis-leading. If sync is true, this call only makes the
file length metadata durable. The caller needs to ensure the data is durable.
--
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]
