Dzeri96 commented on code in PR #3597:
URL: https://github.com/apache/celeborn/pull/3597#discussion_r2779120047
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/storage/StorageManager.scala:
##########
@@ -102,19 +102,23 @@ final private[worker] class StorageManager(conf:
CelebornConf, workerSource: Abs
if (diskInfoSet.nonEmpty) Some(diskInfoSet) else None
}
- def disksSnapshot(): List[DiskInfo] = {
+ def localDisksSnapshot(): List[DiskInfo] = {
diskInfos.synchronized {
val disks = new util.ArrayList[DiskInfo](diskInfos.values())
disks.asScala.toList
}
}
- def healthyWorkingDirs(): List[File] =
- disksSnapshot().filter(_.status == DiskStatus.HEALTHY).flatMap(_.dirs)
+ def allDisksSnapshot(): List[DiskInfo] = {
+ localDisksSnapshot() ++ remoteDiskInfos.getOrElse(Nil)
Review Comment:
This is straight up incorrect. First of all, the code compiles and the tests
pass. Second of all, this can be verified easily in the scala shell:
```scala
Welcome to Scala 2.13.17 (OpenJDK 64-Bit Server VM, Java 17.0.16).
Type in expressions for evaluation. Or try :help.
scala> List(1,2,3)
val res0: List[Int] = List(1, 2, 3)
scala> val list = List(1,2,3)
val list: List[Int] = List(1, 2, 3)
scala> Option(Set(5,6))
val res1: Option[scala.collection.immutable.Set[Int]] = Some(Set(5, 6))
scala> val optSome = Option(Set(5,6))
val optSome: Option[scala.collection.immutable.Set[Int]] = Some(Set(5, 6))
scala> val optNone = Option.empty
val optNone: Option[Nothing] = None
scala> list ++ optSome.getOrElse(Nil)
val res2: List[Int] = List(1, 2, 3, 5, 6)
scala> list ++ optNone.getOrElse(Nil)
val res3: List[Int] = List(1, 2, 3)
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
