junrao commented on code in PR #15825:
URL: https://github.com/apache/kafka/pull/15825#discussion_r1608986312
##########
core/src/main/scala/kafka/log/LocalLog.scala:
##########
@@ -370,11 +370,17 @@ 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 ||
+ maxOffsetMetadata.messageOffsetOnly() ||
+ maxOffsetMetadata.segmentBaseOffset < segmentOpt.get().baseOffset()) {
+ // We need to be careful before reading the segment as
`maxOffsetMetadata` may not be a complete metadata:
+ // 1. If maxOffsetMetadata is message-offset-only, then return empty
fetchDataInfo since
+ // maxOffsetMetadata.offset is not on local log segments.
+ // 2. If maxOffsetMetadata.segmentBaseOffset is smaller than
segment.baseOffset, then return empty fetchDataInfo.
Review Comment:
Looking at the code again. @showuon is correct.
`convertToOffsetMetadataOrThrow(startOffset)` uses nextOffset, which always has
the offset metadata. So, we can keep the
`convertToOffsetMetadataOrThrow(startOffset)` call.
--
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
