junrao commented on code in PR #23765:
URL: https://github.com/apache/kafka/pull/23765#discussion_r4252213431
##########
storage/src/main/java/org/apache/kafka/storage/internals/log/LogSegment.java:
##########
@@ -524,6 +539,8 @@ public int recover(ProducerStateManager
producerStateManager, LeaderEpochFileCac
// A normally closed segment always appends the biggest timestamp ever
seen into log segment, we do this as well.
timeIndex().maybeAppend(maxTimestampSoFar(),
shallowOffsetOfMaxTimestampSoFar(), true);
timeIndex().trimToValidSize();
+ if (overflowException != null)
+ throw overflowException;
Review Comment:
This is more complicated than I original thought. First, the problem with
this fix is that the caller doesn't get the truncated bytes. During unclean
recovery, we find the first segment with truncated bytes and the caller removes
the remaining segments. By throwing an exception here, we lose tract of the
truncated bytes.
Second, I realize that there is another existing problem. In
loadSegmentFiles(), we call recoverSegment() on previously flushed segments,
which could throw LogSegmentOffsetOverflowException. retryOnOffsetOverflow()
will split the segment on that exception. The problem is that the splitting
logic doesn't rebuild the txn index on the split segments. Since this is a
previously flushed segment, it won't go through the subsequent recovery logic
to build the txn index.
Here is a proposal to fix both issues. Keep retryOnOffsetOverflow over
recoveryLog(). Change LogSegment.recover() to return both truncated bytes and
an indication of offset overflow. recoverLog() will truncate the rest of the
segments if truncatedBytes > 0. It then throws
LogSegmentOffsetOverflowException if the indicator is true.
retryOnOffsetOverflow() will then call recoveryLog() again, which will rebuild
the txn index. Change loadSegmentFiles() so that it doesn't call
recoverSegment() and replaces it by changing hadCleanShutdown to false and
moving recoveryPointCheckpoint to this segment's base offset. We don't wrap
loadSegmentFiles with retryOnOffsetOverflow and simply let the subsequent
recoveryLog() takes care of it.
--
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]
