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



##########
File path: core/src/main/scala/org/apache/spark/storage/BlockManager.scala
##########
@@ -620,11 +620,15 @@ private[spark] class BlockManager(
    * Note that this method must be called without any BlockInfo locks held.
    */
   def reregister(): Unit = {
-    // TODO: We might need to rate limit re-registering.
-    logInfo(s"BlockManager $blockManagerId re-registering with master")
-    master.registerBlockManager(blockManagerId, 
diskBlockManager.localDirsString, maxOnHeapMemory,
-      maxOffHeapMemory, storageEndpoint)
-    reportAllBlocks()
+    SparkContext.getActive.map { context =>

Review comment:
       @wankunde The problem here is it's a race condition issue - the 
reregistration request could be sent before the executor is stopped by the 
driver. So this kind of protection can't resolve the issue thoroughly. 
   
   (BTW, I think we only have SparkContext on the driver side.) 
   
   The problem is https://github.com/apache/spark/pull/34536 now is we can't 
handle the case you mentioned 
[there](https://github.com/apache/spark/pull/34536#issuecomment-971211171). The 
reason the fix can't handle is that `HeartbeatReceiver` doesn't know the 
existence of the BlockManager in that case. So I think we can let 
`HeartbeatReceiver` implements `onBlockManagerAdded` to listen on the 
registration of the BlockManager so that `HeartbeatReceiver` knows the 
BlockManager too in that case. 
   
   
   




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



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

Reply via email to