Re: [PR] KAFKA-16452: Don't throw OOORE when converting the offset to metadata [kafka]

[via GitHub]

kamalcph commented on code in PR #15825:
URL: https://github.com/apache/kafka/pull/15825#discussion_r1611559594


##########
storage/src/main/java/org/apache/kafka/storage/internals/log/LogOffsetMetadata.java:
##########
@@ -51,23 +50,24 @@ public LogOffsetMetadata(long messageOffset,
 
     // check if this offset is already on an older segment compared with the 
given offset
     public boolean onOlderSegment(LogOffsetMetadata that) {
-        if (messageOffsetOnly())
-            throw new KafkaException(this + " cannot compare its segment info 
with " + that + " since it only has message offset info");
-
+        if (messageOffsetOnly() || that.messageOffsetOnly())
+            return false;
         return this.segmentBaseOffset < that.segmentBaseOffset;
     }
 
     // check if this offset is on the same segment with the given offset
-    private boolean onSameSegment(LogOffsetMetadata that) {
+    public boolean onSameSegment(LogOffsetMetadata that) {
+        if (messageOffsetOnly() || that.messageOffsetOnly())
+            return false;
         return this.segmentBaseOffset == that.segmentBaseOffset;
     }
 
     // compute the number of bytes between this offset to the given offset
     // if they are on the same segment and this offset precedes the given 
offset
     public int positionDiff(LogOffsetMetadata that) {
-        if (messageOffsetOnly())
+        if (messageOffsetOnly() || that.messageOffsetOnly())
             throw new KafkaException(this + " cannot compare its segment 
position with " + that + " since it only has message offset info");
-        if (!onSameSegment(that))
+        if (this.segmentBaseOffset != that.segmentBaseOffset)

Review Comment:
   We already performed the `messageOffsetOnly` check, so tried to avoid it. 
(micro-optimization, reverted it as the cost is trivial)



-- 
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: jira-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org