[GitHub] spark pull request: [SPARK-14357] [CORE] Properly handle the root ...

[jasonmoore2k]

Github user jasonmoore2k commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-214658162
  
I've opened this ticket:  https://issues.apache.org/jira/browse/SPARK-14915


---
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-14357] [CORE] Properly handle the root ...

[jasonmoore2k]

Github user jasonmoore2k commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-214651693
  
Previously, code was in place to treat a CDE in a particular way, but it 
wasn't sufficient (it didn't consider that the CDE was actually nested inside a 
Spark Exception at the point it was being handled).  This PR fixed that, but it 
seems to have highlighted a problem with the originally intended behavior 
(worst case can cause an infinite loop).

Reverting my change could return it to a state where speculation would 
often cause a job to be aborted (but at least finish), which is still not great.

Alternatively, I'm thinking we could put a task that receives a CDE from 
the driver, into a TaskState.FINISHED or some other state to indicated that the 
task shouldn't be resubmitted by the TaskScheduler.  I'd probably need some 
opinions on whether there are other consequences for doing something like this.

It might be worth moving this discussion to a JIRA ticket.  I'll fire one 
up.


---
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-14357] [CORE] Properly handle the root ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-214636865
  
You know more about the semantics than I do, but does this change make the 
behavior change in this way? it seems like just made something caused by a CDE 
be handled like a CDE. Are you suggesting just reverting this or some other 
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 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-14357] [CORE] Properly handle the root ...

[jasonmoore2k]

Github user jasonmoore2k commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-214628237
  
@andrewor14 @srowen

I'm starting to think this change wasn't great to make.  I'm now seeing 
speculated tasks sometimes getting into a cycle of running, being 
(legitimately) denied from committing their tasks, retrying, being denied, 
retrying, etc.  And with the change I made in this PR, this retrying now has no 
limit.  Which is really bad (jobs potentially running forever).  Whereas 
previously they would eventually fail (still not great).

I think the ideal behavior would be to have the speculated task instead be 
considered a success if the commit is denied legitimately (because another task 
for the same partition has already completed), rather than considered a failure 
and retried without limit (to only fail again).  Any thoughts?


---
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-14357] [CORE] Properly handle the root ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207999135
  
SGTM; I checked for other instances of this pattern and I don't see any, 
which is I suppose good news (didn't miss any) and bad (unfortunately don't get 
to simply anything else with this whole new class)


---
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-14357] [CORE] Properly handle the root ...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207932122
  
Merging into master.


---
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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207607724
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55381/
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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207607722
  
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 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-14357] [CORE] Properly handle the root ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207607440
  
**[Test build #55381 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55381/consoleFull)**
 for PR 12228 at commit 
[`d5ebd55`](https://github.com/apache/spark/commit/d5ebd5585fe7a259cdafb6bb4bd427d567ad93cc).
 * 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-14357] [CORE] Properly handle the root ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207565637
  
**[Test build #55381 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55381/consoleFull)**
 for PR 12228 at commit 
[`d5ebd55`](https://github.com/apache/spark/commit/d5ebd5585fe7a259cdafb6bb4bd427d567ad93cc).


---
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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207565240
  
Fixed. 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 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-14357] [CORE] Properly handle the root ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/12228#discussion_r59066989
  
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -321,7 +321,7 @@ private[spark] class Executor(
   logInfo(s"Executor killed $taskName (TID $taskId)")
   execBackend.statusUpdate(taskId, TaskState.KILLED, 
ser.serialize(TaskKilled))
 
-case cDE: CommitDeniedException =>
+case CausedBy(cDE: CommitDeniedException) =>
--- End diff --

Oh I missed that, nice one. Yeah I agree this is a good one then, certainly 
if it can replace similar patterns elsewhere.


---
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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207540334
  
Oh never mind, looks like Jenkins is broken at the moment: 
https://amplab.cs.berkeley.edu/jenkins/view/Spark%20QA%20Compile/job/spark-master-compile-maven-hadoop-2.3/.
 I think @JoshRosen is fixing it ATM.


---
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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207539353
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55372/
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 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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207539352
  
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 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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207538976
  
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 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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/12228#discussion_r59065155
  
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -321,7 +321,7 @@ private[spark] class Executor(
   logInfo(s"Executor killed $taskName (TID $taskId)")
   execBackend.statusUpdate(taskId, TaskState.KILLED, 
ser.serialize(TaskKilled))
 
-case cDE: CommitDeniedException =>
+case CausedBy(cDE: CommitDeniedException) =>
--- End diff --

it's recursive right? We call `unapply` inside `unapply`.


---
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-14357] [CORE] Properly handle the root ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/12228#discussion_r59064976
  
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -321,7 +321,7 @@ private[spark] class Executor(
   logInfo(s"Executor killed $taskName (TID $taskId)")
   execBackend.statusUpdate(taskId, TaskState.KILLED, 
ser.serialize(TaskKilled))
 
-case cDE: CommitDeniedException =>
+case CausedBy(cDE: CommitDeniedException) =>
--- End diff --

This also only handles 1 level, note. If it can be reused to improve other 
code that would be convincing. 


---
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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207537481
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55370/
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 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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207537478
  
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 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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207536042
  
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 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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/12228#discussion_r59063855
  
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -321,7 +321,7 @@ private[spark] class Executor(
   logInfo(s"Executor killed $taskName (TID $taskId)")
   execBackend.statusUpdate(taskId, TaskState.KILLED, 
ser.serialize(TaskKilled))
 
-case cDE: CommitDeniedException =>
+case CausedBy(cDE: CommitDeniedException) =>
--- End diff --

@srowen this is useful in other places too, so I don't think it's overkill. 
What you suggested handles only 1 level of nesting and is less robust. I would 
prefer to leave this the way it is.


---
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-14357] [CORE] Properly handle the root ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/12228#discussion_r59009601
  
--- Diff: core/src/main/scala/org/apache/spark/executor/Executor.scala ---
@@ -321,7 +321,7 @@ private[spark] class Executor(
   logInfo(s"Executor killed $taskName (TID $taskId)")
   execBackend.statusUpdate(taskId, TaskState.KILLED, 
ser.serialize(TaskKilled))
 
-case cDE: CommitDeniedException =>
+case CausedBy(cDE: CommitDeniedException) =>
--- End diff --

It's tidy, but seems like overkill if this is the only place in the code 
that checks if "T, or its immediate cause, is a FooException". `case t: 
Throwable if t.getCause.isInstanceOf[CommitDeniedException] =>` is sufficient 
to handle the additional case, at the cost of repeating the line of code.

Alternatively, if there are really a few instances of this pattern in the 
code that we can clean up with this pattern, then it seems worthwhile.


---
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-14357] [CORE] Properly handle the root ...

[jasonmoore2k]

Github user jasonmoore2k commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207196510
  
@andrewor14, think it's safe to just retest?  I don't think the fatal error 
was because of these changes.   I don't have the permissions to start a test 
run.


---
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-14357] [CORE] Properly handle the root ...

[jasonmoore2k]

Github user jasonmoore2k commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207194131
  
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 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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207193145
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55293/
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 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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207193142
  
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 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-14357] [CORE] Properly handle the root ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207193064
  
**[Test build #55293 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55293/consoleFull)**
 for PR 12228 at commit 
[`d5ebd55`](https://github.com/apache/spark/commit/d5ebd5585fe7a259cdafb6bb4bd427d567ad93cc).
 * 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 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-14357] [CORE] Properly handle the root ...

[jasonmoore2k]

Github user jasonmoore2k commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207188334
  
Flaky test?  The second test that is running has passed that one.


---
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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207184857
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55288/
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 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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207184850
  
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 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-14357] [CORE] Properly handle the root ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207184154
  
**[Test build #55288 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55288/consoleFull)**
 for PR 12228 at commit 
[`64a4499`](https://github.com/apache/spark/commit/64a44998e4ae412645ed38d368639555d3fe1f66).
 * 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 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-14357] [CORE] Properly handle the root ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207160003
  
**[Test build #55293 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55293/consoleFull)**
 for PR 12228 at commit 
[`d5ebd55`](https://github.com/apache/spark/commit/d5ebd5585fe7a259cdafb6bb4bd427d567ad93cc).


---
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-14357] [CORE] Properly handle the root ...

[jasonmoore2k]

Github user jasonmoore2k commented on a diff in the pull request:

https://github.com/apache/spark/pull/12228#discussion_r58967842
  
--- Diff: core/src/test/scala/org/apache/spark/util/CausedBySuite.scala ---
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.util
+
+import org.apache.spark.SparkFunSuite
+
+class CausedBySuite extends SparkFunSuite {
+
+  test("For an error without a cause, should return the error") {
+
+// Arrange
+val error = new Exception
+
+// Act
+val causedBy = error match {
+  case CausedBy(e) => e
+}
+
+// Assert
--- End diff --

Sure thing, 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 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-14357] [CORE] Properly handle the root ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207150989
  
**[Test build #55288 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55288/consoleFull)**
 for PR 12228 at commit 
[`64a4499`](https://github.com/apache/spark/commit/64a44998e4ae412645ed38d368639555d3fe1f66).


---
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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/12228#discussion_r58967525
  
--- Diff: core/src/test/scala/org/apache/spark/util/CausedBySuite.scala ---
@@ -0,0 +1,67 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.util
+
+import org.apache.spark.SparkFunSuite
+
+class CausedBySuite extends SparkFunSuite {
+
+  test("For an error without a cause, should return the error") {
+
+// Arrange
+val error = new Exception
+
+// Act
+val causedBy = error match {
+  case CausedBy(e) => e
+}
+
+// Assert
--- End diff --

can you remove all these comments and blank lines? They're not super 
necessary


---
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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/12228#discussion_r58967540
  
--- Diff: core/src/main/scala/org/apache/spark/util/CausedBy.scala ---
@@ -0,0 +1,38 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.util
+
+/**
+ * Extractor Object for pulling out the root cause of an error.
+ * If the error contains no cause, it will return the error itself.
+ *
+ * Usage:
+ * try
+ * {
+ *   ...
+ * }
+ * catch {
--- End diff --

style:
```
try {
} catch {
}
```


---
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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207150584
  
Looks great. Representing it in `unapply` is quite elegant.


---
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-14357] [CORE] Properly handle the root ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-207150346
  
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-14357] [CORE] Properly handle the root ...

[jasonmoore2k]

Github user jasonmoore2k commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-206658369
  
Should the matching for FetchFailedException, TaskKilledException & 
InterruptedException get similar treatment?


---
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-14357] [CORE] Properly handle the root ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/12228#issuecomment-206658231
  
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-14357] [CORE] Properly handle the root ...

[jasonmoore2k]

GitHub user jasonmoore2k opened a pull request:

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

[SPARK-14357] [CORE] Properly handle the root cause being a commit denied 
exception

## What changes were proposed in this pull request?

When deciding whether a CommitDeniedException caused a task to fail, 
consider the root cause of the Exception.

## How was this patch tested?

Added a test suite for the component that extracts the root cause of the 
error.
Made a distribution after cherry-picking this commit to branch-1.6 and used 
to run our Spark application that would quite often fail due to the 
CommitDeniedException.


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

$ git pull https://github.com/jasonmoore2k/spark SPARK-14357

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

https://github.com/apache/spark/pull/12228.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 #12228


commit 64a44998e4ae412645ed38d368639555d3fe1f66
Author: Jason Moore 
Date:   2016-04-05T06:26:11Z

[SPARK-14357] [CORE] Properly handle the root cause being a commit denied 
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 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