You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/12/09 09:27:24 UTC

[GitHub] [spark] Ngone51 commented on a change in pull request #34629: [SPARK-37355][CORE]Avoid Block Manager registrations when Executor is shutting down

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: reviews-unsubscribe@spark.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org