junrao commented on code in PR #16614:
URL: https://github.com/apache/kafka/pull/16614#discussion_r1683646397


##########
storage/src/main/java/org/apache/kafka/storage/internals/epoch/LeaderEpochFileCache.java:
##########
@@ -348,7 +348,8 @@ public void truncateFromEndAsyncFlush(long endOffset) {
                 // - We still flush the change in #assign synchronously, 
meaning that it's guaranteed that the checkpoint file always has no missing 
entries.
                 //   * Even when stale epochs are restored from the checkpoint 
file after the unclean shutdown, it will be handled by
                 //     another truncateFromEnd call on log loading procedure, 
so it won't be a problem
-                scheduler.scheduleOnce("leader-epoch-cache-flush-" + 
topicPartition, this::writeToFileForTruncation);
+                List<EpochEntry> entries = new ArrayList<>(epochs.values());
+                scheduler.scheduleOnce("leader-epoch-cache-flush-" + 
topicPartition, () -> checkpoint.writeForTruncation(entries));

Review Comment:
   @chia7712 : Yes, you are right that the overwriting issue was already 
introduced in https://github.com/apache/kafka/pull/15993. Moving the flush call 
inside the read lock fixes this issue, but it defeats the original performance 
optimization in https://github.com/apache/kafka/pull/14242. @ocadaruma : What's 
your opinion on this?



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to