kamalcph commented on code in PR #15825:
URL: https://github.com/apache/kafka/pull/15825#discussion_r1606925582


##########
core/src/main/scala/kafka/log/LocalLog.scala:
##########
@@ -370,11 +370,12 @@ class LocalLog(@volatile private var _dir: File,
         throw new OffsetOutOfRangeException(s"Received request for offset 
$startOffset for partition $topicPartition, " +
           s"but we only have log segments upto $endOffset.")
 
-      if (startOffset == maxOffsetMetadata.messageOffset)
+      if (startOffset == maxOffsetMetadata.messageOffset) {
         emptyFetchDataInfo(maxOffsetMetadata, includeAbortedTxns)
-      else if (startOffset > maxOffsetMetadata.messageOffset)
-        emptyFetchDataInfo(convertToOffsetMetadataOrThrow(startOffset), 
includeAbortedTxns)
-      else {
+      } else if (startOffset > maxOffsetMetadata.messageOffset) {
+        // Instead of converting the `startOffset` to metadata, returning 
message-only metadata to avoid potential loop
+        emptyFetchDataInfo(new LogOffsetMetadata(startOffset), 
includeAbortedTxns)

Review Comment:
   With the newly added checks in L376 and L377, the empty fetch-data-info is 
returned and the actual data will be returned only when the `maxOffsetMetadata` 
gets resolved to complete metadata. 
   
   Earlier, the below condition returned `segment.size` for message-only 
maxOffsetMetadata:
   ```
   if (maxOffsetMetadata.segmentBaseOffset == segment.baseOffset) 
maxOffsetMetadata.relativePositionInSegment
               else segment.size
   ```
   so we might be returning the data beyond the allowed maxOffset. 



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