Github user GraceH commented on a diff in the pull request:

    https://github.com/apache/spark/pull/7927#discussion_r36596179
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/BlockManager.scala 
---
    @@ -590,10 +590,21 @@ private[spark] class BlockManager(
       private def doGetRemote(blockId: BlockId, asBlockResult: Boolean): 
Option[Any] = {
         require(blockId != null, "BlockId is null")
         val locations = Random.shuffle(master.getLocations(blockId))
    +    var failTimes = 0
         for (loc <- locations) {
           logDebug(s"Getting remote block $blockId from $loc")
    -      val data = blockTransferService.fetchBlockSync(
    -        loc.host, loc.port, loc.executorId, 
blockId.toString).nioByteBuffer()
    +      val data = try {
    +        blockTransferService.fetchBlockSync(
    +          loc.host, loc.port, loc.executorId, 
blockId.toString).nioByteBuffer()
    +      } catch {
    +        case e: IOException if failTimes < locations.size - 1 =>
    +          // Return null when IOException throw, so we can fetch block
    +          // from another location if there still have locations
    +          failTimes += 1
    +          logWarning(s"Try ${failTimes} times getting remote block 
$blockId from $loc failed:", e)
    --- End diff --
    
    That's fine @squito. Both option (a) and (b) are acceptable. 
    
    BTW, we'd better to add some document to tell the caller, it throws out an 
Exception. In Java, all expected exceptions are displayed explicitly. It is 
easy to understand which kind of exceptions to be catch in the caller. It seems 
most of the caller for ```bm.getRemoteBytes``` or ```bm.get``` replying on the 
returned back ```Option```, and not being aware of any exception. We can tell 
them if getting blocks failed from all remotes, it  is expected one new 
```Exception``` there as Unit test does. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---

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

Reply via email to