junrao commented on code in PR #23303:
URL: https://github.com/apache/kafka/pull/23303#discussion_r3896953259
##########
clients/src/main/java/org/apache/kafka/common/record/internal/FileRecords.java:
##########
@@ -215,8 +215,10 @@ public void close() throws IOException {
return;
}
- flush();
trim();
+ // flush() must run after trim() so the truncated file length is
included in the fsync.
+ // A flush before trim only persists message data, not the smaller
size set by truncate().
Review Comment:
truncate() => trim() ?
##########
clients/src/test/java/org/apache/kafka/common/record/internal/FileRecordsTest.java:
##########
@@ -388,6 +388,25 @@ public void testPreallocateFalse() throws IOException {
assertEquals(0, temp.length());
}
+ /**
+ * Closing a preallocated file must fsync after trimming so the truncated
length is durable.
+ */
+ @Test
+ public void testCloseFlushesAfterTrim() throws IOException {
+ FileChannel channelMock = mock(FileChannel.class);
+
+ when(channelMock.size()).thenReturn(1024L);
+ when(channelMock.isOpen()).thenReturn(true);
+ when(channelMock.truncate(anyLong())).thenReturn(channelMock);
+ when(channelMock.position(anyLong())).thenReturn(channelMock);
+
+ FileRecords records = new FileRecords(tempFile(), channelMock, 100);
+ records.close();
+
+ verify(channelMock, times(1)).force(true);
+ verify(channelMock).truncate(100L);
Review Comment:
How do we verify that force() is called after truncate() ? Perhaps sth like
the following.
```
InOrder inOrder = inOrder(channelMock);
inOrder.verify(channelMock).truncate(100L);
inOrder.verify(channelMock).force(true);
```
--
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]
