You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by tejasapatil <gi...@git.apache.org> on 2016/07/14 14:51:24 UTC

[GitHub] spark pull request #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend...

GitHub user tejasapatil opened a pull request:

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

    [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to self kill if there is an exception while creating an Executor

    ## What changes were proposed in this pull request?
    
    With the fix from SPARK-13112, I see that `LaunchTask` is always processed after `RegisteredExecutor` is done and so it gets chance to do all retries to startup an executor. There is still a problem that if `Executor` creation itself fails and there is some exception, it gets unnoticed and the executor is killed when it tries to process the `LaunchTask` as `executor` is null : https://github.com/apache/spark/blob/master/core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala#L88 So if one looks at the logs, it does not tell that there was problem during `Executor` creation and thats why it was killed.
    
    This PR explicitly catches exception in `Executor` creation, logs a proper message and then exits the JVM. Also, I have changed the `exitExecutor` method to accept `reason` so that backends can use that reason and do stuff like logging to a DB to get an aggregate of such exits at a cluster level
    
    ## How was this patch tested?
    
    I am relying on existing tests

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

    $ git pull https://github.com/tejasapatil/spark exit_executor_failure

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

    https://github.com/apache/spark/pull/14202.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 #14202
    
----
commit 0c71699894d4b7920388056a1d05d2277a79cf38
Author: Tejas Patil <te...@fb.com>
Date:   2016-07-14T14:36:36Z

    CoarseGrainedExecutorBackend to self kill if there is an exception while creating an Executor

----


---
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 #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend...

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

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


---
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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

Posted by tejasapatil <gi...@git.apache.org>.
Github user tejasapatil commented on the issue:

    https://github.com/apache/spark/pull/14202
  
    Jenkins test 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 #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend...

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

    https://github.com/apache/spark/pull/14202#discussion_r71058847
  
    --- Diff: core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala ---
    @@ -147,7 +148,10 @@ private[spark] class CoarseGrainedExecutorBackend(
        * executor exits differently. For e.g. when an executor goes down,
        * back-end may not want to take the parent process down.
        */
    -  protected def exitExecutor(code: Int): Unit = System.exit(code)
    +  protected def exitExecutor(code: Int, reason: String, throwable: Throwable = null) = {
    --- End diff --
    
    Looks good. 


---
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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    **[Test build #62324 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62324/consoleFull)** for PR 14202 at commit [`0c71699`](https://github.com/apache/spark/commit/0c71699894d4b7920388056a1d05d2277a79cf38).
     * 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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    @tejasapatil Looks pretty good. Just one minor comment.


---
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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    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 #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend...

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

    https://github.com/apache/spark/pull/14202#discussion_r70900025
  
    --- Diff: core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala ---
    @@ -147,7 +148,10 @@ private[spark] class CoarseGrainedExecutorBackend(
        * executor exits differently. For e.g. when an executor goes down,
        * back-end may not want to take the parent process down.
        */
    -  protected def exitExecutor(code: Int): Unit = System.exit(code)
    +  protected def exitExecutor(code: Int, reason: String, throwable: Throwable = null) = {
    --- End diff --
    
    @hbhanawat  FYI, this is going to change `exitExecutor`.


---
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 #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend...

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

    https://github.com/apache/spark/pull/14202#discussion_r71010365
  
    --- Diff: core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala ---
    @@ -147,7 +148,10 @@ private[spark] class CoarseGrainedExecutorBackend(
        * executor exits differently. For e.g. when an executor goes down,
        * back-end may not want to take the parent process down.
        */
    -  protected def exitExecutor(code: Int): Unit = System.exit(code)
    +  protected def exitExecutor(code: Int, reason: String, throwable: Throwable = null) = {
    +    logError(reason, throwable)
    --- End diff --
    
    Ok. Did the 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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

Posted by tejasapatil <gi...@git.apache.org>.
Github user tejasapatil commented on the issue:

    https://github.com/apache/spark/pull/14202
  
    Hi @zsxwing , can you please review this diff ?


---
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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

Posted by tejasapatil <gi...@git.apache.org>.
Github user tejasapatil commented on the issue:

    https://github.com/apache/spark/pull/14202
  
    @zsxwing : I am done with the change(s) you suggested.


---
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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    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 #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend...

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

    https://github.com/apache/spark/pull/14202#discussion_r70899567
  
    --- Diff: core/src/main/scala/org/apache/spark/executor/CoarseGrainedExecutorBackend.scala ---
    @@ -147,7 +148,10 @@ private[spark] class CoarseGrainedExecutorBackend(
        * executor exits differently. For e.g. when an executor goes down,
        * back-end may not want to take the parent process down.
        */
    -  protected def exitExecutor(code: Int): Unit = System.exit(code)
    +  protected def exitExecutor(code: Int, reason: String, throwable: Throwable = null) = {
    +    logError(reason, throwable)
    --- End diff --
    
    @tejasapatil  do you know if all log frameworks can handle `null` throwable here? Perhaps  it's better to handle it by ourselves like:
    ```
    if (throwable != null) {
       logError(reason, throwable)
    } else {
      logError(reason)
    }
    ```


---
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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    **[Test build #62324 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62324/consoleFull)** for PR 14202 at commit [`0c71699`](https://github.com/apache/spark/commit/0c71699894d4b7920388056a1d05d2277a79cf38).


---
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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62324/
    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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    **[Test build #62381 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62381/consoleFull)** for PR 14202 at commit [`a4f80f5`](https://github.com/apache/spark/commit/a4f80f58694127cab846803b8162cfb0a50b7026).


---
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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/62381/
    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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    LGTM. Thanks! Merging to master and 2.0.


---
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 issue #14202: [SPARK-16230] [CORE] CoarseGrainedExecutorBackend to sel...

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

    https://github.com/apache/spark/pull/14202
  
    **[Test build #62381 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62381/consoleFull)** for PR 14202 at commit [`a4f80f5`](https://github.com/apache/spark/commit/a4f80f58694127cab846803b8162cfb0a50b7026).
     * 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