junrao commented on code in PR #15993:
URL: https://github.com/apache/kafka/pull/15993#discussion_r1617635613
##########
core/src/main/java/kafka/log/remote/RemoteLogManager.java:
##########
@@ -575,9 +575,9 @@ InMemoryLeaderEpochCheckpoint
getLeaderEpochCheckpoint(UnifiedLog log, long star
if (log.leaderEpochCache().isDefined()) {
LeaderEpochFileCache cache =
log.leaderEpochCache().get().writeTo(checkpoint);
if (startOffset >= 0) {
- cache.truncateFromStart(startOffset);
+ cache.truncateFromStart(startOffset, true);
Review Comment:
This makes the code a bit hard to understand. I am wondering if we could
improve it. The existing usage of `InMemoryLeaderEpochCheckpoint` is kind of
awkward. Its only purpose is to get a list of epoch entries from
`LEaderEpochCache` within a specified offset range. But the way that it
achieves the goal is indirect and complicated.
Instead of using `InMemoryLeaderEpochCheckpoint`, perhaps we could add a new
method like `List<EpochEntry> epochEntriesInRange(long startOffset, long
endOffset)` in `LeaderEpochCache` that returns a list of epoch entries within
startOffset and endOffset. Then, we could pull the logic in
`readAsByteBuffer.InMemoryLeaderEpochCheckpoint()` to a static method. This
way, we don't need to add the `flushSync` option in `truncateFromStart()` and
can get rid of `InMemoryLeaderEpochCheckpoint`. What do you think? cc @satishd
--
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]
