chia7712 commented on code in PR #23316:
URL: https://github.com/apache/kafka/pull/23316#discussion_r3897237193


##########
storage/src/main/java/org/apache/kafka/storage/internals/log/AbstractIndex.java:
##########
@@ -294,8 +288,9 @@ public int sizeInBytes() {
     }
 
     public void close() throws IOException {
-        flush(); // Ensure the index content is flushed to disk as 
LogSegment.close may append an entry
-        trimToValidSize(true);
+        flush(false); // Ensure the index content is flushed to disk as 
LogSegment.close may append an entry

Review Comment:
   `trimToValidSize` replaces `mmap` with a new mapping, so the first `flush` 
ensures `force()` is invoked on the previous `mmap` before it gets swapped out.
   
   The javadoc of `MappedByteBuffer.force` only guarantees that "all changes 
made to the buffer since it was created" are written to the device, so changes 
made through the previous buffer are not strictly covered by calling `force()` 
on the new one.
   
   WDYT?



-- 
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]

Reply via email to