[GitHub] [spark] squito commented on a change in pull request #24554: [SPARK-27622][Core] Avoiding the network when block manager fetches disk persisted RDD blocks from the same host

[GitBox <gi...@apache.org>]

squito commented on a change in pull request #24554: [SPARK-27622][Core] Avoiding the network when block manager fetches disk persisted RDD blocks from the same host
URL: https://github.com/apache/spark/pull/24554#discussion_r282725898
 
 

 ##########
 File path: core/src/main/scala/org/apache/spark/storage/BlockManager.scala
 ##########
 @@ -875,17 +879,24 @@ private[spark] class BlockManager(
   private def getRemoteManagedBuffer(blockId: BlockId): Option[ManagedBuffer] = {
     logDebug(s"Getting remote block $blockId")
     require(blockId != null, "BlockId is null")
-    var runningFailureCount = 0
-    var totalFailureCount = 0
-
     // Because all the remote blocks are registered in driver, it is not necessary to ask
     // all the slave executors to get block status.
-    val locationsAndStatus = master.getLocationsAndStatus(blockId)
-    val blockSize = locationsAndStatus.map { b =>
-      b.status.diskSize.max(b.status.memSize)
-    }.getOrElse(0L)
-    val blockLocations = locationsAndStatus.map(_.locations).getOrElse(Seq.empty)
-
+    val locationsAndStatusOption = master.getLocationsAndStatus(blockId, blockManagerId.host)
+    if (locationsAndStatusOption.isEmpty) {
+      logDebug(s"Block $blockId is unknown by block manager master")
+      return None
+    }
+    val locationsAndStatus = locationsAndStatusOption.get
+
+    val blockSize = locationsAndStatus.status.diskSize.max(locationsAndStatus.status.memSize)
+    if (locationsAndStatus.localDirs.isDefined) {
 
 Review comment:
   its a bit strange this is inside `getRemoteManagedBuffer` -- but probably moving it to another method would only add complexity.  it at least deserves a comment here that you're really still reading from local disk, but owned by another executor, and in the method doc.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

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