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


##########
core/src/main/scala/kafka/log/UnifiedLog.scala:
##########
@@ -1424,11 +1424,18 @@ class UnifiedLog(@volatile var logStartOffset: Long,
 
   /**
     * Given a message offset, find its corresponding offset metadata in the 
log.
-    * If the message offset is out of range, throw an OffsetOutOfRangeException
+    * 1. If the message offset is lesser than the log-start-offset, then throw 
an OffsetOutOfRangeException
+    * 2. If the message offset is lesser than the local-log-start-offset, then 
it returns the message-only metadata
+    * 3. If the message offset is greater than the log-end-offset, then it 
returns the message-only metadata
     */
-  private def convertToOffsetMetadataOrThrow(offset: Long): LogOffsetMetadata 
= {
+  private[log] def convertToOffsetMetadataOrThrow(offset: Long): 
LogOffsetMetadata = {

Review Comment:
   I don't think we need to change it because it won't cause infinite recursion 
in `LocalLog.read()`:
   ```
   else if (startOffset > maxOffsetMetadata.messageOffset)
           emptyFetchDataInfo(convertToOffsetMetadataOrThrow(startOffset), 
includeAbortedTxns)
   ```
   
   The reason is when in `LocalLog#convertToOffsetMetadataOrThrow`, the 
`maxOffsetMetadata` will be set to `nextOffsetMetadata`, which is the 
`logEndOffset`.
   
   ```
   read(offset,
         maxLength = 1,
         minOneMessage = false,
         maxOffsetMetadata = nextOffsetMetadata,
         includeAbortedTxns = false)
   ```
   So, even it enters the `LocalLog.read()` again, but this time, it'll enter 
different if/else block. It won't enter the `else if (startOffset > 
maxOffsetMetadata.messageOffset)` block this time because the check earlier 
will catch it!
   ```
         if (startOffset > endOffset || !segmentOpt.isPresent)
           throw new OffsetOutOfRangeException(s"Received request for offset 
$startOffset for partition $topicPartition, " +  s"but we only have log 
segments upto $endOffset.")
   ```
   
   
   



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