kafka
kafka copied to clipboard
KAFKA-17803: Changing KafkaMetadataLog#read to match MockLog#read behavior + correcting LogSegment#read javadoc
Calling MockLog or KafkaMetadataLog's read method for a given startOffset returns a LogOffsetMetadata object that contains an offset field. In the case of MockLog, this offset field is the base offset of the record batch which contains startOffset.
However, in KafkaMetadataLog, this offset field is set to the given startOffset. If the given startOffset is in the middle of a batch, the returned LogOffsetMetadata will have an offset that does not match the file position of the returned batch. This makes the javadoc for LogSegment#read inaccurate since startOffset is not a lower bound when startOffset is not a batch's baseOffset (the base offset of the batch containing startOffset is the lower bound).
This difference became an issue for https://github.com/apache/kafka/pull/17500/files because checking a whether a snapshotId.offset was batch aligned using the LogOffsetMetadata.offset() returned from read would work for raft tests that rely on MockLog, but this check would always be trivially true for KafkaMetadataLog (snapshotId.offset == startOffset == LogOffsetMetadata.offset()).
This PR will modify LogSegment#read so it returns the batch start offset of the batch that contains startOffset, which is the actual offset being pointed to by the byte position in LogOffsetMetadata. This change will result in KafkaMetadataLog#read's behavior matching MockLog#read.
Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
LGTM, but can we have some kind of test of the MockLog behavior?
Yeah I can add that.
@junrao Thanks for the reviews! Let me know if there's anything else.
@kevin-wu24 : Could you adjust the title of the PR accordingly? The PR didn't change KafkaMetadataLog#read.