junrao commented on code in PR #15621:
URL: https://github.com/apache/kafka/pull/15621#discussion_r1594333143


##########
core/src/test/scala/unit/kafka/log/LogValidatorTest.scala:
##########
@@ -404,9 +414,7 @@ class LogValidatorTest {
     assertEquals(now + 1, validatingResults.maxTimestampMs,
       s"Max timestamp should be ${now + 1}")
 
-    val expectedOffsetOfMaxTimestamp = 1
-    assertEquals(expectedOffsetOfMaxTimestamp, 
validatingResults.offsetOfMaxTimestampMs,
-      s"Offset of max timestamp should be 1")
+    assertEquals(2, validatingResults.shallowOffsetOfMaxTimestamp)

Review Comment:
   > yep, that is a "TYPO" but it does not change the test.
   
   The issue is that the test is testing the wrong expected value. For magic of 
1, the offset should be 1 instead of 2.
   
   > However, I do observe a potential bug.
   
   Yes, this can lead to inaccurate LogAppendInfo.sourceCompression. But it 
doesn't seem to have real impact now. LogAppendInfo.sourceCompression is only 
used in LogValidator, which is only called by the leader. In the leader, 
currently, we expect only 1 batch per producer.



-- 
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

Reply via email to