chia7712 commented on code in PR #23316:
URL: https://github.com/apache/kafka/pull/23316#discussion_r3925340988
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/AbstractIndex.java:
##########
@@ -252,12 +243,18 @@ 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();
+ if (dirtyMetadata) {
+ try (FileChannel channel = FileChannel.open(file.toPath(),
StandardOpenOption.WRITE)) {
+ channel.force(true);
+ }
+ dirtyMetadata = false;
Review Comment:
I’m fine with removing the dirty flag if my nitpick introduces any risk.
However, I may be missing something in Jun’s comment:
> Since dirtyMetadata is cleared, the second flush() won't flush the
metadata.
In close(), maybeAppend only writes index content through the mmap and does
not change the file size, so mmap.force() covers it.
If the file size does change, trimToValidSize sets the dirty flag again, so
the second flush() would still force the metadata. Am I missing a case?
--
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]
