showuon commented on code in PR #16085:
URL: https://github.com/apache/kafka/pull/16085#discussion_r1619753582


##########
core/src/test/scala/unit/kafka/log/LogSegmentsTest.scala:
##########
@@ -238,5 +245,25 @@ class LogSegmentsTest {
     assertEquals(Int.MaxValue, LogSegments.sizeInBytes(asList(logSegment)))
     assertEquals(largeSize, LogSegments.sizeInBytes(asList(logSegment, 
logSegment)))
     assertTrue(UnifiedLog.sizeInBytes(asList(logSegment, logSegment)) > 
Int.MaxValue)
+
+    val logSegments: LogSegments = new LogSegments(topicPartition)
+    logSegments.add(logSegment)
+    assertEquals(Int.MaxValue, logSegments.sizeInBytes())
+  }
+
+  @Test
+  def testUpdateDir(): Unit = {
+    val seg1 = createSegment(1)

Review Comment:
   seg1 should be closed.



##########
core/src/test/scala/unit/kafka/log/LogSegmentsTest.scala:
##########
@@ -105,8 +109,6 @@ class LogSegmentsTest {
     assertFalse(segments.nonEmpty)
     assertEquals(0, segments.numberOfSegments)
     assertFalse(segments.contains(offset1))
-
-    segments.close()

Review Comment:
   You're right. So it is a bug here. Now, I think what we have to do is 
manually close seg1, seg2, seg3 here, otherwise, there will be resource leak.



##########
core/src/test/scala/unit/kafka/log/LogSegmentTest.scala:
##########
@@ -632,6 +632,15 @@ class LogSegmentTest {
     Utils.delete(tempDir)
   }
 
+  @Test
+  def testGetFirstBatchTimestamp(): Unit = {
+    val segment = createSegment(1)

Review Comment:
   ditto.



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