[GitHub] [incubator-celeborn] AngersZhuuuu commented on a diff in pull request #1468: [CELEBORN-560][FOLLOWUP] Release slots may call twice for same shuffle key, add comment for this

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

AngersZhuuuu commented on code in PR #1468:
URL: https://github.com/apache/incubator-celeborn/pull/1468#discussion_r1178812995


##########
client/src/main/scala/org/apache/celeborn/client/LifecycleManager.scala:
##########
@@ -625,13 +625,13 @@ class LifecycleManager(appId: String, val conf: CelebornConf) extends RpcEndpoin
       }
     }
 
-    if (shuffleResourceExists(shuffleId)) {
-      logWarning(s"Partition exists for shuffle $shuffleId, " +
-        "maybe caused by task rerun or speculative.")
-      requestReleaseSlots(
-        rssHARetryClient,
-        ReleaseSlots(appId, shuffleId, List.empty.asJava, List.empty.asJava))
-    }
+    logWarning(s"Partition may exists for shuffle $shuffleId, " +
+      "maybe caused by task rerun or speculative.")
+    // ReleaseSlots only remove the master side workers meta's shuffle key related slots.
+    // For one shuffle key may request twice, but won't impact the cluster.
+    requestReleaseSlots(
+      rssHARetryClient,
+      ReleaseSlots(appId, shuffleId, List.empty.asJava, List.empty.asJava))

Review Comment:
   IMO, we can remove these lines, WDYT? cc @waitinfuture @RexXiong 



-- 
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