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

Reply via email to