FrankYang0529 commented on code in PR #16614: URL: https://github.com/apache/kafka/pull/16614#discussion_r1684321413
########## 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: Hi all, thanks for raising the correctness issue. IMO, we can fix data correctness first, and then improve performance if it doesn't break data correctness. I will rewrite `testLogRecoveryMetrics` with `NoOpScheduler` first and see whether need to improve `LeaderEpochFileCache` performance with its own scheduler. Thank you. -- 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