holdenk commented on a change in pull request #28911:
URL: https://github.com/apache/spark/pull/28911#discussion_r480269400



##########
File path: core/src/main/scala/org/apache/spark/storage/BlockManager.scala
##########
@@ -212,7 +211,7 @@ private[spark] class BlockManager(
   private val maxOnHeapMemory = memoryManager.maxOnHeapStorageMemory
   private val maxOffHeapMemory = memoryManager.maxOffHeapStorageMemory
 
-  private val externalShuffleServicePort = 
StorageUtils.externalShuffleServicePort(conf)
+  private[spark] val externalShuffleServicePort = 
StorageUtils.externalShuffleServicePort(conf)

Review comment:
       Why did the scope change here?

##########
File path: 
core/src/main/scala/org/apache/spark/network/netty/NettyBlockRpcServer.scala
##########
@@ -113,6 +113,21 @@ class NettyBlockRpcServer(
             s"when there is not sufficient space available to store the 
block.")
           responseContext.onFailure(exception)
         }
+
+      case getLocalDirs: GetLocalDirsForExecutors =>
+        val isIncorrectAppId = getLocalDirs.appId != appId
+        val execNum = getLocalDirs.execIds.length
+        if (isIncorrectAppId || execNum != 1) {
+          val errorMsg = "Invalid GetLocalDirsForExecutors request: " +
+            s"${if (isIncorrectAppId) s"incorrect application id: 
${getLocalDirs.appId};"}" +
+            s"${if (execNum != 1) s"incorrect executor number: $execNum 
(expected 1);"}"
+          responseContext.onFailure(new IllegalStateException(errorMsg))
+        } else {
+          val execId = getLocalDirs.execIds.head

Review comment:
       So this assumes that if we're passed in only one execId it's always the 
execId of the executor to which the block server is related? Would it make 
sense to do a check 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.

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