shantanukawale-stripe commented on code in PR #23880:
URL: https://github.com/apache/kafka/pull/23880#discussion_r4422990400
##########
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:
Thanks, Jun. I'd prefer to keep this PR focused on
`UnifiedLog.searchOffsetInLocalLog` and address these in two separate follow-up
PRs: one covering both early-return paths in `RemoteLogManager`, and another
for the `MAX_TIMESTAMP` case, each with regression tests. I'll link the
follow-up PRs here so both issues stay tracked.
--
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]
