Dzeri96 commented on code in PR #3597:
URL: https://github.com/apache/celeborn/pull/3597#discussion_r2787051312
##########
worker/src/main/scala/org/apache/celeborn/service/deploy/worker/Worker.scala:
##########
@@ -284,11 +284,10 @@ private[celeborn] class Worker(
storageManager.updateDiskInfos()
storageManager.startDeviceMonitor()
- // WorkerInfo's diskInfos is a reference to storageManager.diskInfos
- val diskInfos = JavaUtils.newConcurrentHashMap[String, DiskInfo]()
- storageManager.disksSnapshot().foreach { diskInfo =>
- diskInfos.put(diskInfo.mountPoint, diskInfo)
- }
+ private val diskInfos = storageManager
+ .allDisksSnapshot()
+ .map { diskInfo => diskInfo.mountPoint -> diskInfo }
+ .toMap.asJava
Review Comment:
Furthermore, if this were true, the right solution would be to encapsulate
this concern inside of the `WorkerInfo` class, which is exactly what we are
doing now. Copilot is promoting something that goes against object-oriented
principles.
--
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]
