apoorvmittal10 commented on code in PR #18696:
URL: https://github.com/apache/kafka/pull/18696#discussion_r1935361547


##########
core/src/main/java/kafka/server/share/SharePartition.java:
##########
@@ -641,13 +669,26 @@ public ShareAcquiredRecords acquire(
             // The fetched records are already part of the in-flight records. 
The records might
             // be available for re-delivery hence try acquiring same. The 
request batches could
             // be an exact match, subset or span over multiple already fetched 
batches.
+            long nextBatchStartOffset = baseOffset;
             for (Map.Entry<Long, InFlightBatch> entry : subMap.entrySet()) {
                 // If the acquired count is equal to the max fetch records 
then break the loop.
                 if (acquiredCount >= maxFetchRecords) {
                     break;
                 }
 
                 InFlightBatch inFlightBatch = entry.getValue();
+
+                // If nextBatchStartOffset is less than the key of the entry, 
this means the fetch happened for a gap in the cachedState.
+                // Thus, a new batch needs to be acquired for the gap.
+                if (nextBatchStartOffset < entry.getKey()) {
+                    ShareAcquiredRecords shareAcquiredRecords = 
acquireNewBatchRecords(memberId, fetchPartitionData.records.batches(),
+                        nextBatchStartOffset, entry.getKey() - 1, batchSize, 
maxFetchRecords);
+                    result.addAll(shareAcquiredRecords.acquiredRecords());
+                    acquiredCount += shareAcquiredRecords.count();
+                }
+                // Set nextBatchStartOffset as the last offset of the current 
in-flight batch + 1
+                nextBatchStartOffset = inFlightBatch.lastOffset() + 1;

Review Comment:
   1. Do you need to run this code on all the batches irrespective if the past 
the `initialReadGapOffset`? I think not.
   2. Say the start offset is: 10 and inflight batches are: 15-20, 30-40. And 
Fetch returns 10-20, 30-50. Will the above change try to acquire 21-29 as well 
when there doesn't exist any offsets? It might be fine as client can report us 
such gaps but just confirming if that's the thought process?



-- 
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