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 2016/02/19 23:05:45 UTC

[GitHub] spark pull request: [SPARK-13408][Core]Ignore errors when it's alr...

GitHub user zsxwing opened a pull request:

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

    [SPARK-13408][Core]Ignore errors when it's already reported

    ## What changes were proposed in this pull request?
    
    `JobWaiter.taskSucceeded` will be called for each task. When `resultHandler` throws an exception, `taskSucceeded` will also throw it for each task. DAGScheduler just catches it and reports it like this:
    ```Scala
                      try {
                        job.listener.taskSucceeded(rt.outputId, event.result)
                      } catch {
                        case e: Exception =>
                          // TODO: Perhaps we want to mark the resultStage as failed?
                          job.listener.jobFailed(new SparkDriverExecutionException(e))
                      }
    ```
    Therefore `JobWaiter.jobFailed` may be called multiple times.
    
    So `JobWaiter.jobFailed` should use `Promise.tryFailure` instead of `Promise.failure` because the latter one doesn't support calling multiple times.
    
    ## How was the this patch tested?
    
    Jenkins tests.
    
    


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

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

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

    https://github.com/apache/spark/pull/11280.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 #11280
    
----
commit d56a91af67bd91503fbe2676ba14c62f4c51a069
Author: Shixiong Zhu <sh...@databricks.com>
Date:   2016-02-19T22:00:48Z

    Ignore errors when it's already reported

----


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186463053
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51579/
    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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186440886
  
    LGTM, pending on tests.


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186493073
  
    **[Test build #51585 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51585/consoleFull)** for PR 11280 at commit [`cbcbf7e`](https://github.com/apache/spark/commit/cbcbf7ecbda84c601fed26ee3e3e7c9852146246).
     * 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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186529434
  
    LGTM, merging this into master, thanks!


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186467905
  
    **[Test build #51585 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51585/consoleFull)** for PR 11280 at commit [`cbcbf7e`](https://github.com/apache/spark/commit/cbcbf7ecbda84c601fed26ee3e3e7c9852146246).


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#discussion_r53525333
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/JobWaiter.scala ---
    @@ -61,7 +63,10 @@ private[spark] class JobWaiter[T](
         }
       }
     
    -  override def jobFailed(exception: Exception): Unit =
    -    jobPromise.failure(exception)
    +  override def jobFailed(exception: Exception): Unit = {
    +    if (!jobPromise.tryFailure(exception)) {
    +      logWarning("Ignore failure", exception)
    --- End diff --
    
    Just log a warning since we cannot report it to Promise.


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186463051
  
    Merged build finished. 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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186429618
  
    cc @davies 


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#discussion_r53528674
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/JobWaiter.scala ---
    @@ -61,7 +63,10 @@ private[spark] class JobWaiter[T](
         }
       }
     
    -  override def jobFailed(exception: Exception): Unit =
    -    jobPromise.failure(exception)
    +  override def jobFailed(exception: Exception): Unit = {
    +    if (!jobPromise.tryFailure(exception)) {
    +      logWarning("Ignore failure", exception)
    --- End diff --
    
    The first exception will be reported to Promise. Other exceptions will just be logged.


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186464722
  
    > Test build #51579 has finished for PR 11280 at commit d56a91a.
    > 
    > This patch fails Spark unit tests.
    > This patch merges cleanly.
    > This patch adds no public classes.
    
    Oops, forgot to set totalTasks. If it's 0, the Promise will be completed at once.


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186493146
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/51585/
    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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186433165
  
    **[Test build #51579 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51579/consoleFull)** for PR 11280 at commit [`d56a91a`](https://github.com/apache/spark/commit/d56a91af67bd91503fbe2676ba14c62f4c51a069).


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186462947
  
    **[Test build #51579 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/51579/consoleFull)** for PR 11280 at commit [`d56a91a`](https://github.com/apache/spark/commit/d56a91af67bd91503fbe2676ba14c62f4c51a069).
     * This patch **fails Spark unit 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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#issuecomment-186493144
  
    Merged build finished. 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-13408][Core]Ignore errors when it's alr...

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

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


---
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-13408][Core]Ignore errors when it's alr...

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

    https://github.com/apache/spark/pull/11280#discussion_r53527501
  
    --- Diff: core/src/main/scala/org/apache/spark/scheduler/JobWaiter.scala ---
    @@ -61,7 +63,10 @@ private[spark] class JobWaiter[T](
         }
       }
     
    -  override def jobFailed(exception: Exception): Unit =
    -    jobPromise.failure(exception)
    +  override def jobFailed(exception: Exception): Unit = {
    +    if (!jobPromise.tryFailure(exception)) {
    +      logWarning("Ignore failure", exception)
    --- End diff --
    
    The exception will be logged once or multiple times if it came from resultHandler?


---
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