satishd commented on code in PR #14034: URL: https://github.com/apache/kafka/pull/14034#discussion_r1451284145
########## core/src/main/scala/kafka/log/LogLoader.scala: ########## @@ -489,16 +488,16 @@ class LogLoader( * * @param segmentsToDelete The log segments to schedule for deletion */ - private def removeAndDeleteSegmentsAsync(segmentsToDelete: Iterable[LogSegment]): Unit = { - if (segmentsToDelete.nonEmpty) { + private def removeAndDeleteSegmentsAsync(segmentsToDelete: java.util.Collection[LogSegment]): Unit = { + if (!segmentsToDelete.isEmpty) { // Most callers hold an iterator into the `params.segments` collection and // `removeAndDeleteSegmentAsync` mutates it by removing the deleted segment. Therefore, // we should force materialization of the iterator here, so that results of the iteration // remain valid and deterministic. We should also pass only the materialized view of the // iterator to the logic that deletes the segments. - val toDelete = segmentsToDelete.toList - info(s"Deleting segments as part of log recovery: ${toDelete.mkString(",")}") - toDelete.foreach { segment => + val toDelete = new util.ArrayList[LogSegment](segmentsToDelete) + info(s"Deleting segments as part of log recovery: ${LocalLog.mkString(toDelete.iterator(), ",")}") Review Comment: Thanks for letting me know about `Utils.join`, I was not aware of it. ########## core/src/test/scala/unit/kafka/log/LocalLogTest.scala: ########## @@ -362,8 +363,8 @@ class LocalLogTest { } assertEquals(5, log.segments.numberOfSegments) assertNotEquals(10L, log.segments.activeSegment.baseOffset) - val expected = log.segments.values.asScala.toVector - val deleted = log.truncateFullyAndStartAt(10L) + val expected = new util.ArrayList(log.segments.values) + val deleted = new util.ArrayList(log.truncateFullyAndStartAt(10L)) Review Comment: They give different types like java `ConcurrentSkipListMap.Values` and `ArrayList`, and the equality check will fail. Wrapped around the same ArrayList instance for their equality checks. Followed similar checks in LocalLogTest and UnifiedLogTest. -- 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