You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by zsxwing <gi...@git.apache.org> on 2014/10/31 06:27:07 UTC

[GitHub] spark pull request: [SPARK-4163][Core][Web UI] Send the fetch fail...

GitHub user zsxwing opened a pull request:

    https://github.com/apache/spark/pull/3032

    [SPARK-4163][Core][Web UI] Send the fetch failure message back to Web UI

    This is a PR to send the fetch failure message back to Web UI.
    Before:
    ![f1](https://cloud.githubusercontent.com/assets/1000778/4856595/1f036c80-60be-11e4-956f-335147fbccb7.png)
    ![f2](https://cloud.githubusercontent.com/assets/1000778/4856596/1f11cbea-60be-11e4-8fe9-9f9b2b35c884.png)
    
    After (Please ignore the meaning of exception, I threw it in the code directly because it's hard to simulate a fetch failure):
    ![e1](https://cloud.githubusercontent.com/assets/1000778/4856600/2657ea38-60be-11e4-9f2d-d56c5f900f10.png)
    ![e2](https://cloud.githubusercontent.com/assets/1000778/4856601/26595008-60be-11e4-912b-2744af786991.png)

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/zsxwing/spark SPARK-4163

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/3032.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #3032
    
----
commit a82e6b678223064b53635fe8a28c52b661b5d4cb
Author: zsxwing <zs...@gmail.com>
Date:   2014-10-30T12:42:08Z

    Send the fetch failure message back to Web UI

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61423147
  
    **[Test build #22764 timed out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22764/consoleFull)**     for PR 3032 at commit [`4e946f7`](https://github.com/apache/spark/commit/4e946f7349db8468375f40fa526c6892f5145759)     after a configured wait of `120m`.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19702059
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/FetchFailedException.scala ---
    @@ -30,13 +31,15 @@ private[spark] class FetchFailedException(
         bmAddress: BlockManagerId,
         shuffleId: Int,
         mapId: Int,
    -    reduceId: Int)
    +    reduceId: Int,
    +    message: String)
       extends Exception {
     
       override def getMessage: String =
         "Fetch failed: %s %d %d %d".format(bmAddress, shuffleId, mapId, reduceId)
     
    -  def toTaskEndReason: TaskEndReason = FetchFailed(bmAddress, shuffleId, mapId, reduceId)
    +  def toTaskEndReason: TaskEndReason =
    --- End diff --
    
    this may fit within 100ch on one line


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61400137
  
    **[Test build #22757 timed out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22757/consoleFull)**     for PR 3032 at commit [`4e946f7`](https://github.com/apache/spark/commit/4e946f7349db8468375f40fa526c6892f5145759)     after a configured wait of `120m`.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19710310
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,18 +53,18 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId, e)
                 case _ =>
                   throw new SparkException(
                     "Failed to get block " + blockId + ", which is not a shuffle block")
    --- End diff --
    
    Sorry. I thought you were talking my modification. Should I revert my changes about adding a overloaded constructor of `FetchFailedException`?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19702114
  
    --- Diff: core/src/test/scala/org/apache/spark/util/JsonProtocolSuite.scala ---
    @@ -107,7 +107,7 @@ class JsonProtocolSuite extends FunSuite {
         testJobResult(jobFailed)
     
         // TaskEndReason
    -    val fetchFailed = FetchFailed(BlockManagerId("With or", "without you", 15), 17, 18, 19)
    +    val fetchFailed = FetchFailed(BlockManagerId("With or", "without you", 15), 17, 18, 19, "Some exception")
    --- End diff --
    
    greater than 100ch


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19705166
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala ---
    @@ -313,14 +319,30 @@ object ShuffleBlockFetcherIterator {
       }
     
       /**
    -   * Result of a fetch from a remote block. A failure is represented as size == -1.
    +   * Result of a fetch from a remote block.
    +   */
    +  trait FetchResult {
    --- End diff --
    
    I added sealed to these classes since they are only used in this file.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19702235
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala ---
    @@ -313,14 +319,30 @@ object ShuffleBlockFetcherIterator {
       }
     
       /**
    -   * Result of a fetch from a remote block. A failure is represented as size == -1.
    +   * Result of a fetch from a remote block.
    +   */
    +  trait FetchResult {
    +    val blockId: BlockId
    +  }
    +
    +  /**
    +   * Result of a fetch from a remote block successfully.
        * @param blockId block id
        * @param size estimated size of the block, used to calculate bytesInFlight.
    -   *             Note that this is NOT the exact bytes. -1 if failure is present.
    -   * @param buf [[ManagedBuffer]] for the content. null is error.
    +   *             Note that this is NOT the exact bytes.
    +   * @param buf [[ManagedBuffer]] for the content.
    +   */
    +  case class SuccessFetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer)
    +    extends FetchResult {
    +    require(buf != null)
    +    require(size >= 0)
    +  }
    +
    +  /**
    +   * Result of a fetch from a remote block unsuccessfully.
    +   * @param blockId block id
    +   * @param e the failure exception
        */
    -  case class FetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer) {
    -    def failed: Boolean = size == -1
    -    if (failed) assert(buf == null) else assert(buf != null)
    +  case class FailureFetchResult(blockId: BlockId, e: Throwable) extends FetchResult {
    --- End diff --
    
    nit: I think it is slightly more idiomatic in Spark to avoid empty braces in case classes which have no body.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61397587
  
      [Test build #22757 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22757/consoleFull) for   PR 3032 at commit [`4e946f7`](https://github.com/apache/spark/commit/4e946f7349db8468375f40fa526c6892f5145759).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19715295
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/FetchFailedException.scala ---
    @@ -46,7 +54,7 @@ private[spark] class MetadataFetchFailedException(
         shuffleId: Int,
         reduceId: Int,
         message: String)
    -  extends FetchFailedException(null, shuffleId, -1, reduceId) {
    +  extends FetchFailedException(null, shuffleId, -1, reduceId, message) {
     
       override def getMessage: String = message
    --- End diff --
    
    we can now delete this method because this is the actual behavior inherently with your change


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19702223
  
    --- Diff: core/src/main/scala/org/apache/spark/util/JsonProtocol.scala ---
    @@ -627,7 +628,9 @@ private[spark] object JsonProtocol {
             val shuffleId = (json \ "Shuffle ID").extract[Int]
             val mapId = (json \ "Map ID").extract[Int]
             val reduceId = (json \ "Reduce ID").extract[Int]
    -        new FetchFailed(blockManagerAddress, shuffleId, mapId, reduceId)
    +        val message = Utils.jsonOption(json \ "Message").map(_.extract[String])
    +        new FetchFailed(blockManagerAddress, shuffleId, mapId, reduceId,
    +          message.getOrElse("Unknown"))
    --- End diff --
    
    Maybe "Unknown reason" instead


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61361054
  
    LGTM, only had superficial comments.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19715279
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/FetchFailedException.scala ---
    @@ -30,13 +31,20 @@ private[spark] class FetchFailedException(
         bmAddress: BlockManagerId,
         shuffleId: Int,
         mapId: Int,
    -    reduceId: Int)
    -  extends Exception {
    +    reduceId: Int,
    +    message: String)
    +  extends Exception(message) {
    +
    +  def this(bmAddress: BlockManagerId, shuffleId: Int, mapId: Int, reduceId: Int, e: Throwable) {
    +    this(bmAddress, shuffleId, mapId, reduceId, e.getMessage)
    +    initCause(e)
    +  }
     
       override def getMessage: String =
    --- End diff --
    
    Having this override the getMessage means that the message parameter does not propagate correctly. We should probably get rid of this method here. Doing so means that the stages page does not show these stats, just the cause message, but the console and the job page show the "TaskEndReason", which includes the address as well as the exception itself. This looks pretty good from a usability perspective.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19718354
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,21 +53,21 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId, e)
    --- End diff --
    
    I will revert to the Utils.exceptionString() way.
    
    Looks `Utils.exceptionString()` does not handle the nested exceptions like this. It would output the outermost exception info. Is it intentional?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61227899
  
      [Test build #22600 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22600/consoleFull) for   PR 3032 at commit [`a3bca65`](https://github.com/apache/spark/commit/a3bca65e2010751cae8e6f7b3e8e7be22bdb35d9).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `  trait FetchResult `
      * `  case class SuccessFetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer)`
      * `  case class FailureFetchResult(blockId: BlockId, e: Throwable) extends FetchResult `



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61446009
  
    Merging into master. Thanks for this!


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61396081
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22742/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61358931
  
      [Test build #22681 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22681/consoleFull) for   PR 3032 at commit [`0c07d1f`](https://github.com/apache/spark/commit/0c07d1fa8f472b88b65653b3471b5afba3ee3546).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `  trait FetchResult `
      * `  case class SuccessFetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer)`
      * `  case class FailureFetchResult(blockId: BlockId, e: Throwable) extends FetchResult `



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19709864
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,18 +53,18 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId, e)
                 case _ =>
                   throw new SparkException(
                     "Failed to get block " + blockId + ", which is not a shuffle block")
    --- End diff --
    
    Sorry. Do you mean `throw e` directly? If so, https://github.com/apache/spark/blob/f55218aeb1e9d638df6229b36a59a15ce5363482/core/src/main/scala/org/apache/spark/executor/Executor.scala#L233 cannot transform such exception to `FetchFailed` by `ffe.toTaskEndReason`.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][Web UI] Send the fetch fail...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61225060
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22594/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19702219
  
    --- Diff: core/src/main/scala/org/apache/spark/TaskEndReason.scala ---
    @@ -69,11 +69,13 @@ case class FetchFailed(
         bmAddress: BlockManagerId,  // Note that bmAddress can be null
    --- End diff --
    
    one day we'll use proper subclassing to get rid of this silly case too


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61376273
  
      [Test build #22696 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22696/consoleFull) for   PR 3032 at commit [`62103fd`](https://github.com/apache/spark/commit/62103fd80a56cb9f2bf072a1b92271be377b5c24).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `  sealed trait FetchResult `
      * `  sealed case class SuccessFetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer)`
      * `  sealed case class FailureFetchResult(blockId: BlockId, e: Throwable) extends FetchResult`



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19715284
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala ---
    @@ -1060,7 +1060,7 @@ class DAGScheduler(
             if (runningStages.contains(failedStage)) {
               logInfo(s"Marking $failedStage (${failedStage.name}) as failed " +
                 s"due to a fetch failure from $mapStage (${mapStage.name})")
    -          markStageAsFinished(failedStage, Some("Fetch failure"))
    +          markStageAsFinished(failedStage, Some("Fetch failure: " + failureMessage))
    --- End diff --
    
    I was wrong here, I didn't realize that the failureMessage includes the "FetchFailedException: ", which makes the prepending of "Fetch failure:" redundant. Please go ahead and remove it again, sorry for making you do extra work!



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61398055
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22753/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61396080
  
      [Test build #22742 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22742/consoleFull) for   PR 3032 at commit [`b88c919`](https://github.com/apache/spark/commit/b88c91959218940898fae3cea59ae9f7c6681166).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `  sealed trait FetchResult `



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61441682
  
    New screenshots for current codes.
    
    Stages page:
    ![stags](https://cloud.githubusercontent.com/assets/1000778/4878803/e2ffede6-6318-11e4-8376-15854e2f6eb3.png)
    
    Detail for stage:
    ![stage](https://cloud.githubusercontent.com/assets/1000778/4878804/ea7f62b8-6318-11e4-8b3e-6939f44c1e56.png)



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][Web UI] Send the fetch fail...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61223601
  
      [Test build #22600 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22600/consoleFull) for   PR 3032 at commit [`a3bca65`](https://github.com/apache/spark/commit/a3bca65e2010751cae8e6f7b3e8e7be22bdb35d9).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19706108
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala ---
    @@ -313,14 +319,29 @@ object ShuffleBlockFetcherIterator {
       }
     
       /**
    -   * Result of a fetch from a remote block. A failure is represented as size == -1.
    +   * Result of a fetch from a remote block.
    +   */
    +  sealed trait FetchResult {
    +    val blockId: BlockId
    +  }
    +
    +  /**
    +   * Result of a fetch from a remote block successfully.
        * @param blockId block id
        * @param size estimated size of the block, used to calculate bytesInFlight.
    -   *             Note that this is NOT the exact bytes. -1 if failure is present.
    -   * @param buf [[ManagedBuffer]] for the content. null is error.
    +   *             Note that this is NOT the exact bytes.
    +   * @param buf [[ManagedBuffer]] for the content.
        */
    -  case class FetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer) {
    -    def failed: Boolean = size == -1
    -    if (failed) assert(buf == null) else assert(buf != null)
    +  sealed case class SuccessFetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer)
    --- End diff --
    
    ah, "sealed" is usually only applied to traits, as case classes themselves are not supposed to be extended.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61418385
  
    Jenkins, retest this please.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19709867
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala ---
    @@ -313,14 +319,29 @@ object ShuffleBlockFetcherIterator {
       }
     
       /**
    -   * Result of a fetch from a remote block. A failure is represented as size == -1.
    +   * Result of a fetch from a remote block.
    +   */
    +  sealed trait FetchResult {
    +    val blockId: BlockId
    +  }
    +
    +  /**
    +   * Result of a fetch from a remote block successfully.
        * @param blockId block id
        * @param size estimated size of the block, used to calculate bytesInFlight.
    -   *             Note that this is NOT the exact bytes. -1 if failure is present.
    -   * @param buf [[ManagedBuffer]] for the content. null is error.
    +   *             Note that this is NOT the exact bytes.
    +   * @param buf [[ManagedBuffer]] for the content.
        */
    -  case class FetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer) {
    -    def failed: Boolean = size == -1
    -    if (failed) assert(buf == null) else assert(buf != null)
    +  sealed case class SuccessFetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer)
    --- End diff --
    
    Already replaced `sealed` with `private[storage]`. Thank you.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61394707
  
      [Test build #22742 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22742/consoleFull) for   PR 3032 at commit [`b88c919`](https://github.com/apache/spark/commit/b88c91959218940898fae3cea59ae9f7c6681166).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19710069
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala ---
    @@ -313,14 +319,30 @@ object ShuffleBlockFetcherIterator {
       }
     
       /**
    -   * Result of a fetch from a remote block. A failure is represented as size == -1.
    +   * Result of a fetch from a remote block.
    +   */
    +  sealed trait FetchResult {
    +    val blockId: BlockId
    +  }
    +
    +  /**
    +   * Result of a fetch from a remote block successfully.
        * @param blockId block id
        * @param size estimated size of the block, used to calculate bytesInFlight.
    -   *             Note that this is NOT the exact bytes. -1 if failure is present.
    -   * @param buf [[ManagedBuffer]] for the content. null is error.
    +   *             Note that this is NOT the exact bytes.
    +   * @param buf [[ManagedBuffer]] for the content.
        */
    -  case class FetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer) {
    -    def failed: Boolean = size == -1
    -    if (failed) assert(buf == null) else assert(buf != null)
    +  private[storage] case class SuccessFetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer)
    --- End diff --
    
    Just to make sure we're on absolutely the same page, `private[storage]` is used to restrict visibility, to ensure that we never accidentally expose this API to a wider audience than necessary. `sealed` is used to restrict inheritance, so that the compiler (and programmer) can check that all cases have been matched on. The attributes are thus orthogonal in nature.
    
    Adding `private[storage]` is good here, and we should also add it to the `trait FetchResult` as well, to make sure it's restricted in both visibility and inheritance.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19715256
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,21 +53,21 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId, e)
    --- End diff --
    
    I tested this patch out locally, and it appears that, sadly, the cause is actually not propagated correctly back to the driver. I think we can revert this to the prior Utils.exceptionString() way, that looks good in the UI.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61441696
  
      [Test build #22800 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22800/consoleFull) for   PR 3032 at commit [`f7e1faf`](https://github.com/apache/spark/commit/f7e1fafaabce45f59ae0b742c233d70f2aacde3d).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61373259
  
    @aarondav thanks for reviewing it. Updated as per your comments.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19706121
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,18 +53,18 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId, e)
                 case _ =>
                   throw new SparkException(
                     "Failed to get block " + blockId + ", which is not a shuffle block")
    --- End diff --
    
    Did you catch my comment here about adding `e` as the cause of this exception?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61396504
  
    How about now?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19710231
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,18 +53,18 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId, e)
                 case _ =>
                   throw new SparkException(
                     "Failed to get block " + blockId + ", which is not a shuffle block")
    --- End diff --
    
    Here, very specifically, I mean
    ```
    case _ =>
      throw new SparkException(
        "Failed to get block " + blockId + ", which is not a shuffle block", e)
    ```
    for this case


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61373383
  
      [Test build #22696 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22696/consoleFull) for   PR 3032 at commit [`62103fd`](https://github.com/apache/spark/commit/62103fd80a56cb9f2bf072a1b92271be377b5c24).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61396730
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22752/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19710088
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala ---
    @@ -313,14 +319,30 @@ object ShuffleBlockFetcherIterator {
       }
     
       /**
    -   * Result of a fetch from a remote block. A failure is represented as size == -1.
    +   * Result of a fetch from a remote block.
    +   */
    +  sealed trait FetchResult {
    +    val blockId: BlockId
    +  }
    +
    +  /**
    +   * Result of a fetch from a remote block successfully.
        * @param blockId block id
        * @param size estimated size of the block, used to calculate bytesInFlight.
    -   *             Note that this is NOT the exact bytes. -1 if failure is present.
    -   * @param buf [[ManagedBuffer]] for the content. null is error.
    +   *             Note that this is NOT the exact bytes.
    +   * @param buf [[ManagedBuffer]] for the content.
        */
    -  case class FetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer) {
    -    def failed: Boolean = size == -1
    -    if (failed) assert(buf == null) else assert(buf != null)
    +  private[storage] case class SuccessFetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer)
    --- End diff --
    
    Got it.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19718379
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala ---
    @@ -1060,7 +1060,7 @@ class DAGScheduler(
             if (runningStages.contains(failedStage)) {
               logInfo(s"Marking $failedStage (${failedStage.name}) as failed " +
                 s"due to a fetch failure from $mapStage (${mapStage.name})")
    -          markStageAsFinished(failedStage, Some("Fetch failure"))
    +          markStageAsFinished(failedStage, Some("Fetch failure: " + failureMessage))
    --- End diff --
    
    If I I revert to the Utils.exceptionString() way, `FetchFailedException` won't appear here because failureMessage will only contain `Utils.exceptionString(e)`. So I will still keep `Fetch failure` here.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61400138
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22757/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19709873
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,18 +53,18 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId, e)
                 case _ =>
                   throw new SparkException(
                     "Failed to get block " + blockId + ", which is not a shuffle block")
    --- End diff --
    
    Oh, sorry, I see. Will update it.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61396600
  
      [Test build #22753 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22753/consoleFull) for   PR 3032 at commit [`316767d`](https://github.com/apache/spark/commit/316767dfb237ab9b1019468e5d039d891c2eef12).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61358019
  
      [Test build #22681 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22681/consoleFull) for   PR 3032 at commit [`0c07d1f`](https://github.com/apache/spark/commit/0c07d1fa8f472b88b65653b3471b5afba3ee3546).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19702216
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/DAGScheduler.scala ---
    @@ -1060,7 +1060,7 @@ class DAGScheduler(
             if (runningStages.contains(failedStage)) {
               logInfo(s"Marking $failedStage (${failedStage.name}) as failed " +
                 s"due to a fetch failure from $mapStage (${mapStage.name})")
    -          markStageAsFinished(failedStage, Some("Fetch failure"))
    +          markStageAsFinished(failedStage, Some(failureMessage))
    --- End diff --
    
    Looking at some of the other error messages that are displayed in the UI's stages view, I think it's safer to add more information -- let's just prepend something like "Fetch failure:" to the exception, because the exception itself may sound totally unrelated (some timeout, file not found, etc.) and it's unclear why the whole stage was finished.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61442041
  
    LGTM, will merge as soon as Jenkins passes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/3032


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19719128
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,21 +53,21 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId, e)
    --- End diff --
    
    That is almost certainly not intentional. I think the method was introduced rather recently, and does not have many users right now. It should be fixed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61398052
  
      [Test build #22753 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22753/consoleFull) for   PR 3032 at commit [`316767d`](https://github.com/apache/spark/commit/316767dfb237ab9b1019468e5d039d891c2eef12).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19702103
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,18 +53,19 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId,
    +                Utils.exceptionString(e))
                 case _ =>
                   throw new SparkException(
                     "Failed to get block " + blockId + ", which is not a shuffle block")
    --- End diff --
    
    maybe we could put `e` as the cause for this guy, while we're here.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61418515
  
      [Test build #22764 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22764/consoleFull) for   PR 3032 at commit [`4e946f7`](https://github.com/apache/spark/commit/4e946f7349db8468375f40fa526c6892f5145759).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][Web UI] Send the fetch fail...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61220886
  
      [Test build #22594 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22594/consoleFull) for   PR 3032 at commit [`c261d23`](https://github.com/apache/spark/commit/c261d235a4a55d082a8d788135e48f9367b5f93f).
     * This patch merges cleanly.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by zsxwing <gi...@git.apache.org>.
Github user zsxwing commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19719301
  
    --- Diff: core/src/main/scala/org/apache/spark/shuffle/hash/BlockStoreShuffleFetcher.scala ---
    @@ -52,21 +53,21 @@ private[hash] object BlockStoreShuffleFetcher extends Logging {
             (address, splits.map(s => (ShuffleBlockId(shuffleId, s._1, reduceId), s._2)))
         }
     
    -    def unpackBlock(blockPair: (BlockId, Option[Iterator[Any]])) : Iterator[T] = {
    +    def unpackBlock(blockPair: (BlockId, Try[Iterator[Any]])) : Iterator[T] = {
           val blockId = blockPair._1
           val blockOption = blockPair._2
           blockOption match {
    -        case Some(block) => {
    +        case Success(block) => {
               block.asInstanceOf[Iterator[T]]
             }
    -        case None => {
    +        case Failure(e) => {
               blockId match {
                 case ShuffleBlockId(shufId, mapId, _) =>
                   val address = statuses(mapId.toInt)._1
    -              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId)
    +              throw new FetchFailedException(address, shufId.toInt, mapId.toInt, reduceId, e)
    --- End diff --
    
    I will try to fix it in another PR.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][Web UI] Send the fetch fail...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61225058
  
      [Test build #22594 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/22594/consoleFull) for   PR 3032 at commit [`c261d23`](https://github.com/apache/spark/commit/c261d235a4a55d082a8d788135e48f9367b5f93f).
     * This patch **passes all tests**.
     * This patch merges cleanly.
     * This patch adds the following public classes _(experimental)_:
      * `  trait FetchResult `
      * `  case class SuccessFetchResult(blockId: BlockId, size: Long, buf: ManagedBuffer)`
      * `  case class FailureFetchResult(blockId: BlockId, e: Throwable) extends FetchResult `



---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61376279
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22696/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by aarondav <gi...@git.apache.org>.
Github user aarondav commented on a diff in the pull request:

    https://github.com/apache/spark/pull/3032#discussion_r19702231
  
    --- Diff: core/src/main/scala/org/apache/spark/storage/ShuffleBlockFetcherIterator.scala ---
    @@ -313,14 +319,30 @@ object ShuffleBlockFetcherIterator {
       }
     
       /**
    -   * Result of a fetch from a remote block. A failure is represented as size == -1.
    +   * Result of a fetch from a remote block.
    +   */
    +  trait FetchResult {
    --- End diff --
    
    We should probably make this a sealed trait, and all associated classes should probably be `private[storage]`


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61423148
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22764/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61358932
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22681/
    Test FAILed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-4163][Core][WebUI] Send the fetch failu...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/3032#issuecomment-61227902
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/22600/
    Test PASSed.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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