sidharthdaga opened a new pull request, #22585: URL: https://github.com/apache/kafka/pull/22585
KAFKA-7283 removed index sanity checks from broker startup to reduce load time. This left a gap: a corrupt time index on a segment that recoverLog() does not reach is loaded without examination and fails at the next roll. A time index is pre-allocated to its maximum size and trimmed to its real length when the segment closes. If the trim fails (an IOException in close() is swallowed) or the broker crashes before trimming, the file is left at full size with no valid entries. On reload, AbstractIndex positions the buffer at the end of the file, so entries() equals maxEntries() and the index appears full. If the segment is below the recovery point or the shutdown was clean, recoverLog() does not process it, and the corruption persists. Because the index appears full, TimeIndex.isFull() returns true and the segment is eligible to roll. The next append or retention check triggers the roll, and onBecomeInactiveSegment() appends the largest timestamp through maybeAppend(..., skipFullCheck = true). The buffer is at the end of a full index, so the write overruns it and throws BufferOverflowException. This fails the replica fetcher and leaves the partition under-replicated. The fix adds a targeted check to TimeIndex.sanityCheck(): reject an index where entries equals maxEntries and the last entry reads a timestamp of zero. A legitimately full index always ends on a positive timestamp, so this combination identifies a never-written one. LogSegment.sanityCheck() runs this check on load regardless of shutdown type. The resulting CorruptIndexException routes the segment through LogLoader recovery, which rebuilds the index from the log. Only the time index is checked because it is the one appended to on roll. The offset index stays lazy, so this reintroduces only the startup cost needed to close the gap. Tests cover detection (TimeIndexTest), propagation through LogSegment.sanityCheck (LogSegmentTest), and end-to-end recovery on reload (UnifiedLogTest). RemoteLogManagerTest trims the index files its fetch mock serves so they pass the check. Delete this text and replace it with a detailed description of your change. The PR title and body will become the squashed commit message. If you would like to tag individuals, add some commentary, upload images, or include other supplemental information that should not be part of the eventual commit message, please use a separate comment. If applicable, please include a summary of the testing strategy (including rationale) for the proposed change. Unit and/or integration tests are expected for any behavior change and system tests should be considered for larger changes. -- 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]
