apoorvmittal10 commented on code in PR #18696: URL: https://github.com/apache/kafka/pull/18696#discussion_r1928982420
########## core/src/main/java/kafka/server/share/SharePartition.java: ########## @@ -462,6 +468,10 @@ public CompletableFuture<Void> maybeInitialize() { // in the cached state are not missed findNextFetchOffset.set(true); endOffset = cachedState.lastEntry().getValue().lastOffset(); + // initialReadGapOffset is not required, if there are no gaps in the read state response + if (isGapPresentInCachedState()) { Review Comment: Rather iterating in the metod again, though it's a low cost thing, should we have a variable in earlier for loop in the method to check if gap exists, wdyt? ########## core/src/main/java/kafka/server/share/SharePartition.java: ########## @@ -1212,7 +1282,11 @@ private ShareAcquiredRecords acquireNewBatchRecords( if (cachedState.firstKey() == firstAcquiredOffset) { startOffset = firstAcquiredOffset; } - endOffset = lastAcquiredOffset; + + if (lastAcquiredOffset > endOffset) { + endOffset = lastAcquiredOffset; + } Review Comment: Can we write comment here reqarding when this can happen given we are acquirung new batch. -- 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