junrao commented on code in PR #23880:
URL: https://github.com/apache/kafka/pull/23880#discussion_r4417662801
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/UnifiedLog.java:
##########
@@ -1815,9 +1815,18 @@ public boolean isEmpty() {
private Optional<FileRecords.TimestampAndOffset>
searchOffsetInLocalLog(long targetTimestamp, long startOffset) throws
IOException {
// Cache to avoid race conditions.
List<LogSegment> segments = logSegments();
- for (LogSegment segment : segments) {
+ // Use base offsets to skip segments before the one containing
startOffset without reading their tails.
+ int firstSegmentIndex = 0;
+ for (int i = 1; i < segments.size() && segments.get(i).baseOffset() <=
startOffset; i++) {
+ firstSegmentIndex = i;
+ }
+ for (LogSegment segment : segments.subList(firstSegmentIndex,
segments.size())) {
if (segment.largestTimestamp() >= targetTimestamp) {
- return segment.findOffsetByTimestamp(targetTimestamp,
startOffset, config().maxDecompressedMessageBytes());
+ Optional<FileRecords.TimestampAndOffset> timestampAndOffset =
+ segment.findOffsetByTimestamp(targetTimestamp,
startOffset, config().maxDecompressedMessageBytes());
Review Comment:
Claude found the following issue.
```
Should fix (here or as a tracked follow-up)
1. Tiered topics have the same bug in two more places. For a tiered topic,
RemoteLogOffsetReader first calls RemoteLogManager.findOffsetByTimestamp, and
only falls back to searchOffsetInLocalLog if that comes back empty
(RemoteLogOffsetReader.java:64-66). Inside it:
Remote segments (RemoteLogManager.java:755): it returns lookupTimestamp(...)
for the first matching remote segment, and lookupTimestamp returns empty when
nothing qualifies. The remote segment holding the log start offset hits this in
the same way: its maxTimestampMs comes from a record below startingOffset. The
method then returns empty and jumps to the local fallback, skipping every later
remote segment. The answer is then a later offset from the local log, or
nothing.
Its own local loop (RemoteLogManager.java:759-760): this is a copy of the
loop this PR fixes, so it needs the same change.
```
Also, a similar issue exists for the ListOffsetsRequest.MAX_TIMESTAMP case.
Do you want to fix it in this PR or a separate one?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
