Yunyung commented on code in PR #19579:
URL: https://github.com/apache/kafka/pull/19579#discussion_r2080661082
##########
core/src/main/scala/kafka/server/BrokerServer.scala:
##########
@@ -709,7 +709,7 @@ class BrokerServer(
None
}
- val rlm = new RemoteLogManager(config.remoteLogManagerConfig,
config.brokerId, config.logDirs.head, clusterId, time,
+ val rlm = new RemoteLogManager(config.remoteLogManagerConfig,
config.brokerId, config.logDirs.asScala.head, clusterId, time,
Review Comment:
Thanks. I’m using get(0) instead, since List.getFirst() is available
starting with Java 21.
##########
core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala:
##########
@@ -214,7 +214,7 @@ class ReplicaManagerTest {
partition.createLogIfNotExists(isNew = false, isFutureReplica = false,
new LazyOffsetCheckpoints(rm.highWatermarkCheckpoints.asJava), None)
rm.checkpointHighWatermarks()
- config.logDirs.map(s => Paths.get(s,
ReplicaManager.HighWatermarkFilename))
+ config.logDirs.asScala.map(s => Paths.get(s,
ReplicaManager.HighWatermarkFilename))
.foreach(checkpointFile => assertTrue(Files.exists(checkpointFile),
Review Comment:
Nice. Thanks.
##########
core/src/test/scala/integration/kafka/server/KRaftClusterTest.scala:
##########
@@ -1551,7 +1551,7 @@ class KRaftClusterTest {
// Copy foo-0 to targetParentDir
// This is so that we can rename the main replica to a future down
below
val parentDir = log.parentDir
- val targetParentDir = broker0.config.logDirs.filter(_ !=
parentDir).head
+ val targetParentDir = broker0.config.logDirs.asScala.filter(_ !=
parentDir).head
val targetDirFile = new File(targetParentDir, log.dir.getName)
Review Comment:
Nice. Thanks.
##########
core/src/test/scala/unit/kafka/server/AlterReplicaLogDirsRequestTest.scala:
##########
@@ -91,10 +91,10 @@ class AlterReplicaLogDirsRequestTest extends
BaseRequestTest {
@ParameterizedTest
@ValueSource(strings = Array("kraft"))
def testAlterReplicaLogDirsRequestErrorCode(quorum: String): Unit = {
- val offlineDir = new
File(brokers.head.config.logDirs.tail.head).getAbsolutePath
- val validDir1 = new File(brokers.head.config.logDirs(1)).getAbsolutePath
- val validDir2 = new File(brokers.head.config.logDirs(2)).getAbsolutePath
- val validDir3 = new File(brokers.head.config.logDirs(3)).getAbsolutePath
+ val offlineDir = new
File(brokers.head.config.logDirs.asScala.tail.head).getAbsolutePath
Review Comment:
I checked, tail.head means getting the second element (since tail removes
the first element), So get(1) gets the same element we originally wanted.
But I'm not sure if the original test was wrong, because `val validDir1 =
new File(brokers.head.config.logDirs(1)).getAbsolutePath` also takes the second
element...
I'll track this to see whether it's a bug.
##########
core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala:
##########
@@ -1165,7 +1165,7 @@ class KafkaConfigTest {
assertEquals(1, config.brokerId)
assertEquals(Seq("PLAINTEXT://127.0.0.1:1122"),
config.effectiveAdvertisedBrokerListeners.map(JTestUtils.endpointToString))
assertEquals(Map("127.0.0.1" -> 2, "127.0.0.2" -> 3),
config.maxConnectionsPerIpOverrides)
- assertEquals(List("/tmp1", "/tmp2"), config.logDirs)
+ assertEquals(List("/tmp1", "/tmp2"), config.logDirs.asScala.toList)
Review Comment:
Nice. Done.
##########
core/src/test/scala/unit/kafka/server/DescribeLogDirsRequestTest.scala:
##########
@@ -42,8 +42,8 @@ class DescribeLogDirsRequestTest extends BaseRequestTest {
@ParameterizedTest
@ValueSource(strings = Array("kraft"))
def testDescribeLogDirsRequest(quorum: String): Unit = {
- val onlineDir = new File(brokers.head.config.logDirs.head).getAbsolutePath
- val offlineDir = new
File(brokers.head.config.logDirs.tail.head).getAbsolutePath
+ val onlineDir = new
File(brokers.head.config.logDirs.asScala.head).getAbsolutePath
+ val offlineDir = new
File(brokers.head.config.logDirs.asScala.tail.head).getAbsolutePath
Review Comment:
> I checked, tail.head means getting the second element (since tail removes
the first element)...
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
