You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by davies <gi...@git.apache.org> on 2015/02/12 23:30:56 UTC
[GitHub] spark pull request: [SPARK-5788] [PySpark] capture the exception i...
GitHub user davies opened a pull request:
https://github.com/apache/spark/pull/4577
[SPARK-5788] [PySpark] capture the exception in python write thread
The exception in Python writer thread will shutdown executor.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/davies/spark exception
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4577.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 #4577
----
commit 139b0db4ad8997c01cabb225383e627ab4779b79
Author: Davies Liu <da...@databricks.com>
Date: 2015-02-12T22:28:55Z
capture the exception in python write thread
----
---
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-5788] [PySpark] capture the exception i...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74605785
lgtm too. josh can u merge 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-5788] [PySpark] capture the exception i...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74605982
I've merged this into `master` (1.4.0), `branch-1.3` (1.3.0), and `branch-1.2` (1.2.2). 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-5788] [PySpark] capture the exception i...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/4577#discussion_r24783568
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala ---
@@ -202,7 +202,7 @@ private[spark] class PythonRDD(
this.interrupt()
}
- override def run(): Unit = Utils.logUncaughtExceptions {
+ override def run(): Unit = Utils.tryLog {
--- End diff --
@rxin, is the concern that a user might write something like
```scala
def foo(): Unit = Utils.tryLog {
return someCallThatThrowsAnException()
}
```
and thereby skip the exception-handling logic?
---
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-5788] [PySpark] capture the exception i...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74596415
@rxin @JoshRosen 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-5788] [PySpark] capture the exception i...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74167914
[Test build #27389 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27389/consoleFull) for PR 4577 at commit [`139b0db`](https://github.com/apache/spark/commit/139b0db4ad8997c01cabb225383e627ab4779b79).
* 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-5788] [PySpark] capture the exception i...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74178783
[Test build #27389 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27389/consoleFull) for PR 4577 at commit [`139b0db`](https://github.com/apache/spark/commit/139b0db4ad8997c01cabb225383e627ab4779b79).
* 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-5788] [PySpark] capture the exception i...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/4577#discussion_r24632726
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala ---
@@ -202,7 +202,7 @@ private[spark] class PythonRDD(
this.interrupt()
}
- override def run(): Unit = Utils.logUncaughtExceptions {
+ override def run(): Unit = Utils.tryLog {
--- End diff --
you can still call return even if the value is Unit
---
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-5788] [PySpark] capture the exception i...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74605385
LGTM.
---
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-5788] [PySpark] capture the exception i...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74604646
[Test build #27599 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27599/consoleFull) for PR 4577 at commit [`eb0ceff`](https://github.com/apache/spark/commit/eb0ceffa87ec1b3ca2b78b8d4a938899ad37c0e5).
* 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-5788] [PySpark] capture the exception i...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/4577#discussion_r24630445
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala ---
@@ -202,7 +202,7 @@ private[spark] class PythonRDD(
this.interrupt()
}
- override def run(): Unit = Utils.logUncaughtExceptions {
+ override def run(): Unit = Utils.tryLog {
--- End diff --
Instead of doing a big tryLog (which is sort of dangerous because of the potential of users adding a return statement that would break this code), can we add an explicit try around the two `worker.shutdownOutput()` calls?
---
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-5788] [PySpark] capture the exception i...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/4577#discussion_r24632471
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala ---
@@ -202,7 +202,7 @@ private[spark] class PythonRDD(
this.interrupt()
}
- override def run(): Unit = Utils.logUncaughtExceptions {
+ override def run(): Unit = Utils.tryLog {
--- End diff --
The `run` can not have return value. The InterruptedException could happen in any place (when the task is canceled), so I think it's better to do it 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-5788] [PySpark] capture the exception i...
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74167827
cc @aarondav
---
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-5788] [PySpark] capture the exception i...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74594740
[Test build #27599 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27599/consoleFull) for PR 4577 at commit [`eb0ceff`](https://github.com/apache/spark/commit/eb0ceffa87ec1b3ca2b78b8d4a938899ad37c0e5).
* 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-5788] [PySpark] capture the exception i...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/4577
---
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-5788] [PySpark] capture the exception i...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/4577#discussion_r24632779
--- Diff: core/src/main/scala/org/apache/spark/api/python/PythonRDD.scala ---
@@ -202,7 +202,7 @@ private[spark] class PythonRDD(
this.interrupt()
}
- override def run(): Unit = Utils.logUncaughtExceptions {
+ override def run(): Unit = Utils.tryLog {
--- End diff --
btw this happened a few times in the past in Spark. somebody added a return in a giant block that is wrapped in a closure. Code reviewers couldn't catch it because it didn't show up in the 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 pull request: [SPARK-5788] [PySpark] capture the exception i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74178788
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27389/
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-5788] [PySpark] capture the exception i...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4577#issuecomment-74604653
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27599/
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