Ngone51 commented on a change in pull request #30164:
URL: https://github.com/apache/spark/pull/30164#discussion_r520298144



##########
File path: 
core/src/main/scala/org/apache/spark/storage/BlockManagerMasterEndpoint.scala
##########
@@ -526,6 +548,8 @@ class BlockManagerMasterEndpoint(
 
       blockManagerInfo(id) = new BlockManagerInfo(id, 
System.currentTimeMillis(),
         maxOnHeapMemSize, maxOffHeapMemSize, storageEndpoint, 
externalShuffleServiceBlockStatus)
+
+      addMergerLocation(id)

Review comment:
       I'm thinking about only adding the merger location when we know there're 
no active executors on it. Since we'd prefer active executor locations first 
now, I think it's almost a redundant copy of the active executor locations 
normally.
   
   The idea is we could add it when we find there're no active executors after 
`removeExecutor` and remove it when there're new executors register on the same 
host. This's definitely helpful for static resource allocation, although I'm a 
little bit hesitant about the possible overhead for dynamic resource allocation.
   
   WDYT?
    




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

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to