chia7712 commented on code in PR #13024:
URL: https://github.com/apache/kafka/pull/13024#discussion_r1053182775
##########
storage/src/main/java/org/apache/kafka/server/log/internals/AbstractIndex.java:
##########
@@ -484,27 +478,35 @@ private static MappedByteBuffer
createMappedBuffer(RandomAccessFile raf, boolean
}
/**
- * Lookup lower and upper bounds for the given target.
+ * Lookup lower or upper bounds for the given target.
*/
- private BinarySearchResult indexSlotRangeFor(ByteBuffer idx, long target,
IndexSearchType searchEntity) {
+ private int indexSlotRangeFor(ByteBuffer idx, long target, IndexSearchType
searchEntity,
+ SearchType searchType) {
// check if the index is empty
if (entries == 0)
- return new BinarySearchResult(-1, -1);
+ return -1;
int firstHotEntry = Math.max(0, entries - 1 - warmEntries());
// check if the target offset is in the warm section of the index
if (compareIndexEntry(parseEntry(idx, firstHotEntry), target,
searchEntity) < 0) {
- return binarySearch(idx, target, searchEntity, firstHotEntry,
entries - 1);
+ return binarySearch(idx, target, searchEntity, searchType,
firstHotEntry, entries - 1);
}
// check if the target offset is smaller than the least offset
- if (compareIndexEntry(parseEntry(idx, 0), target, searchEntity) > 0)
- return new BinarySearchResult(-1, 0);
+ if (compareIndexEntry(parseEntry(idx, 0), target, searchEntity) > 0) {
+ switch (searchType) {
+ case SMALLEST_UPPER_BOUND:
+ return -1;
+ case LARGEST_LOWER_BOUND:
+ return 0;
+ }
Review Comment:
How about throwing `IllegalStateException` from the default label for
consistency?
##########
storage/src/main/java/org/apache/kafka/server/log/internals/AbstractIndex.java:
##########
@@ -484,27 +478,35 @@ private static MappedByteBuffer
createMappedBuffer(RandomAccessFile raf, boolean
}
/**
- * Lookup lower and upper bounds for the given target.
+ * Lookup lower or upper bounds for the given target.
*/
- private BinarySearchResult indexSlotRangeFor(ByteBuffer idx, long target,
IndexSearchType searchEntity) {
+ private int indexSlotRangeFor(ByteBuffer idx, long target, IndexSearchType
searchEntity,
+ SearchType searchType) {
// check if the index is empty
if (entries == 0)
- return new BinarySearchResult(-1, -1);
+ return -1;
int firstHotEntry = Math.max(0, entries - 1 - warmEntries());
// check if the target offset is in the warm section of the index
if (compareIndexEntry(parseEntry(idx, firstHotEntry), target,
searchEntity) < 0) {
- return binarySearch(idx, target, searchEntity, firstHotEntry,
entries - 1);
+ return binarySearch(idx, target, searchEntity, searchType,
firstHotEntry, entries - 1);
}
// check if the target offset is smaller than the least offset
- if (compareIndexEntry(parseEntry(idx, 0), target, searchEntity) > 0)
- return new BinarySearchResult(-1, 0);
+ if (compareIndexEntry(parseEntry(idx, 0), target, searchEntity) > 0) {
+ switch (searchType) {
+ case SMALLEST_UPPER_BOUND:
+ return -1;
Review Comment:
It seems to me `SMALLEST_UPPER_BOUND` should return 0 and
`LARGEST_LOWER_BOUND` should return -1
##########
storage/src/main/java/org/apache/kafka/server/log/internals/AbstractIndex.java:
##########
@@ -517,13 +519,19 @@ private BinarySearchResult binarySearch(ByteBuffer idx,
long target, IndexSearch
else if (compareResult < 0)
lo = mid;
else
- return new BinarySearchResult(mid, mid);
+ return mid;
+ }
+ switch (searchType) {
+ case SMALLEST_UPPER_BOUND:
Review Comment:
ditto. maybe we get the incorrect returned value.
--
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]
