[GitHub] [incubator-celeborn] waitinfuture commented on a diff in pull request #1236: [CELEBORN-301] Refactor PartitionLocationInfo to use ConcurrentHashMap

["waitinfuture (via GitHub)" <gi...@apache.org>]

waitinfuture commented on code in PR #1236:
URL: https://github.com/apache/incubator-celeborn/pull/1236#discussion_r1107075431


##########
common/src/main/scala/org/apache/celeborn/common/meta/PartitionLocationInfo.scala:
##########
@@ -122,81 +110,42 @@ class PartitionLocationInfo extends Logging {
     removePartitions(shuffleKey, uniqueIds, slavePartitionLocations)
   }
 
-  def getAllMasterLocationsWithMinEpoch(shuffleKey: String): util.List[PartitionLocation] =
-    this.synchronized {
-      getAllMasterLocationsWithExtremeEpoch(shuffleKey, (a, b) => a < b)
-    }
+  def getAllMasterLocationsWithMinEpoch(shuffleKey: String): util.List[PartitionLocation] = {
+    getAllMasterLocationsWithExtremeEpoch(shuffleKey, (a, b) => a < b)
+  }
 
+  // TODO: UT
   def getAllMasterLocationsWithExtremeEpoch(
       shuffleKey: String,
-      order: (Int, Int) => Boolean): util.List[PartitionLocation] = this.synchronized {
-    if (masterPartitionLocations.containsKey(shuffleKey)) {
-      masterPartitionLocations.get(shuffleKey)
-        .values()
-        .asScala
-        .map { list =>
-          var loc = list.get(0)
-          1 until list.size() foreach (ind => {
-            if (order(list.get(ind).getEpoch, loc.getEpoch)) {
-              loc = list.get(ind)
-            }
-          })
-          loc
-        }.toList.asJava
+      order: (Int, Int) => Boolean): util.List[PartitionLocation] = {
+    val partitionMap = masterPartitionLocations.get(shuffleKey)
+    if (partitionMap != null) {
+      val resMap = new util.HashMap[Int, PartitionLocation]()
+      partitionMap.values().asScala.foreach(loc => {

Review Comment:
   Since partitionMap is ConcurrentHashMap, I think it's safe here.



-- 
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: issues-unsubscribe@celeborn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org