[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68649791 Alright, I've re-scoped SPARK-4787 to only cover the changes here and made it into a subtask of the broader SPARK-4194 issue. So as not to let the perfect be the enemy of the good, I'm going to merge this now and we can address the broader cleanup issues in later PRs. Thanks for the fix! --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68649927 Merged into `master` (1.3.0), `branch-1.2` (1.2.1), `branch-1.1` (1.1.2), and `branch-1.0` (1.0.3). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/3809 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3809#discussion_r22322821
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -329,8 +329,11 @@ class SparkContext(config: SparkConf) extends Logging
with ExecutorAllocationCli
try {
dagScheduler = new DAGScheduler(this)
} catch {
-case e: Exception = throw
- new SparkException(DAGScheduler cannot be initialized due to
%s.format(e.getMessage))
+case e: Exception = {
+ stop()
--- End diff --
Also, do you think this should be in a try-finally block so that we don't
swallow the useful DAGScheduler could not be initialized exception if the
stop() call somehow fails?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user tigerquoll commented on a diff in the pull request:
https://github.com/apache/spark/pull/3809#discussion_r22334486
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -329,8 +329,11 @@ class SparkContext(config: SparkConf) extends Logging
with ExecutorAllocationCli
try {
dagScheduler = new DAGScheduler(this)
} catch {
-case e: Exception = throw
- new SparkException(DAGScheduler cannot be initialized due to
%s.format(e.getMessage))
+case e: Exception = {
+ stop()
--- End diff --
Excellent idea Josh.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68317909 [Test build #24873 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24873/consoleFull) for PR 3809 at commit [`5661e01`](https://github.com/apache/spark/commit/5661e01c2b0aaf900b50fb2444db714f73021aa4). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68322406 [Test build #24873 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24873/consoleFull) for PR 3809 at commit [`5661e01`](https://github.com/apache/spark/commit/5661e01c2b0aaf900b50fb2444db714f73021aa4). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68322408 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24873/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
GitHub user tigerquoll opened a pull request: https://github.com/apache/spark/pull/3809 spark-core - [SPARK-4787] - Stop sparkcontext properly if a DAGScheduler init error occurs [SPARK-4787] Stop SparkContext properly if an exception occurs during DAGscheduler initialization. You can merge this pull request into a Git repository by running: $ git pull https://github.com/tigerquoll/spark SPARK-4787 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/3809.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 #3809 commit 217257879fe7c98673caf14b980790498887581e Author: Dale tigerqu...@outlook.com Date: 2014-12-26T09:33:05Z [SPARK-4787] Stop context properly if an exception occurs during DAGScheduler initialization. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68133474 Can one of the admins verify this patch? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/3809#discussion_r22287446
--- Diff: core/src/main/scala/org/apache/spark/SparkContext.scala ---
@@ -329,8 +329,11 @@ class SparkContext(config: SparkConf) extends Logging
with ExecutorAllocationCli
try {
dagScheduler = new DAGScheduler(this)
} catch {
-case e: Exception = throw
- new SparkException(DAGScheduler cannot be initialized due to
%s.format(e.getMessage))
+case e: Exception = {
+ stop()
+ throw
--- End diff --
Style nit: you can use string interpolation instead of String.format, which
will allow the `new SparkException` to fit on the same line as `throw`:
```scala
throw new SparkException(DAGScheduler cannot be initialized due to
${e.getMessage})
```
However, I'd prefer to call the two-argument constructor which takes the
cause as second argument, since this will lead to more informative stacktraces:
```scala
throw new SparkException(Error while constructing DAGScheduler, e)
```
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68155076 Jenkins, this is ok to test. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68155166 [Test build #24838 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24838/consoleFull) for PR 3809 at commit [`2172578`](https://github.com/apache/spark/commit/217257879fe7c98673caf14b980790498887581e). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68155357 This is a nice fix. Resource leaks when SparkContext's constructor throws exceptions have been a longstanding issue. I first ran across the issue while adding logic to detect whether a SparkContext was already running when attempting to create a new one ([SPARK-4180](https://issues.apache.org/jira/browse/SPARK-4180)). In that case, I ran into some issues because I wanted to effectively make the entire constructor synchronized on a static object, but this was hard because there wasn't an explicit constructor method. We could have tried to wrap the entire implicit constructor in a try-finally block, but this would require us to re-organize a huge amount of code and change many `vals` into `vars`. I had an alternative proposal to move the dependency-creation into the SparkContext companion object and pass in a SparkContextDependencies object into SparkContext's constructors, which would solve this issue more generally (but it's a much larger change). See the PR description at #3121 for more deta ils. Barring a big restructuring of SparkContext's constructor, though, small fixes like this are welcome. Therefore, this looks good to me. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user JoshRosen commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68155681 By the way, I left a [comment over on JIRA](https://issues.apache.org/jira/browse/SPARK-4787?focusedCommentId=14259202page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14259202) about the scope of the SPARK-4787 JIRA. If we merge this PR as-is, without adding more try-catches for other statements that could throw exceptions, then I think we should revise that JIRA to describe only the fix implemented here (error-catching for DAGScheduler errors) and should convert it into a subtask of SPARK-4180. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68158247 [Test build #24838 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/24838/consoleFull) for PR 3809 at commit [`2172578`](https://github.com/apache/spark/commit/217257879fe7c98673caf14b980790498887581e). * 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: spark-core - [SPARK-4787] - Stop sparkcontext ...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/3809#issuecomment-68158250 Test PASSed. Refer to this link for build results (access rights to CI server needed): https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24838/ 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
