[GitHub] spark pull request: [SPARK-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157786997
  
LGTM. Merging to master and branch 1.6. I will try to cherry-pick it to 
1.5. If there is no major conflict, we will not need a 1.5 specific pr.


---
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-11195][CORE] Use correct classloader fo...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157793454
  
@choochootrain #9779 has been merged. Can you close this 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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157787067
  
Thank you 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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157817239
  
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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain closed the pull request at:

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


---
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157638488
  
**[Test build #46192 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46192/consoleFull)**
 for PR 9779 at commit 
[`8bd78af`](https://github.com/apache/spark/commit/8bd78afe09821c21f233df4715b698d2589e33f1).


---
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157676646
  
**[Test build #46192 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46192/consoleFull)**
 for PR 9779 at commit 
[`8bd78af`](https://github.com/apache/spark/commit/8bd78afe09821c21f233df4715b698d2589e33f1).
 * 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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157676778
  
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157676783
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46192/
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157336863
  
  [Test build #2073 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2073/consoleFull)
 for   PR 9367 at commit 
[`fadf2ca`](https://github.com/apache/spark/commit/fadf2ca57e21d5f2600d4ec3e504e20db596c35c).


---
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157369352
  
  [Test build #2073 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2073/console)
 for   PR 9367 at commit 
[`fadf2ca`](https://github.com/apache/spark/commit/fadf2ca57e21d5f2600d4ec3e504e20db596c35c).
 * 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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157421084
  
  [Test build #2079 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2079/console)
 for   PR 9367 at commit 
[`fadf2ca`](https://github.com/apache/spark/commit/fadf2ca57e21d5f2600d4ec3e504e20db596c35c).
 * This patch **fails PySpark 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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157422891
  
  [Test build #2080 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2080/console)
 for   PR 9367 at commit 
[`fadf2ca`](https://github.com/apache/spark/commit/fadf2ca57e21d5f2600d4ec3e504e20db596c35c).
 * 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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157378559
  
  [Test build #2080 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2080/consoleFull)
 for   PR 9367 at commit 
[`fadf2ca`](https://github.com/apache/spark/commit/fadf2ca57e21d5f2600d4ec3e504e20db596c35c).


---
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157378317
  
  [Test build #2079 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2079/consoleFull)
 for   PR 9367 at commit 
[`fadf2ca`](https://github.com/apache/spark/commit/fadf2ca57e21d5f2600d4ec3e504e20db596c35c).


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r45093937
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +123,64 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  /**
+   * SPARK-11195
+
+   * Make sure we are using the context classloader when deserializing 
failed TaskResults instead
+   * of the Spark classloader.
+
+   * This test compiles a jar containing an exception and tests that when 
it is thrown on the
+   * executor, enqueueFailedTask can correctly deserialize the failure and 
identify the thrown
+   * exception as the cause.
+
+   * Before this fix, enqueueFailedTask would throw a 
ClassNotFoundException when deserializing
+   * the exception, resulting in an UnknownReason for the TaskEndResult.
+   */
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// load the exception from the jar
+val originalClassLoader = Thread.currentThread.getContextClassLoader
+val loader = new MutableURLClassLoader(new Array[URL](0), 
originalClassLoader)
+loader.addURL(jarFile.toURI.toURL)
+Thread.currentThread().setContextClassLoader(loader)
+val excClass: Class[_] = Utils.classForName("repro.MyException")
+
+// NOTE: we must run the cluster with "local" so that the executor can 
load the compiled
+// jar.
+sc = new SparkContext("local", "test", conf)
+val rdd = sc.parallelize(Seq(1), 1).map(x => {
+  val exc = excClass.newInstance().asInstanceOf[Exception]
+  throw exc
+})
+
+// the driver should not have any problems resolving the exception 
class and determining
+// why the task failed.
+val exceptionMessage = intercept[SparkException] {
+  rdd.collect()
+}.getMessage
+
+val expectedFailure = """(?s).*Lost task.*: repro.MyException.*""".r
+val unknownFailure = """(?s).*Lost task.*: UnknownReason.*""".r
+
+assert(expectedFailure.findFirstMatchIn(exceptionMessage).isDefined)
+assert(unknownFailure.findFirstMatchIn(exceptionMessage).isEmpty)
+
+// reset the classloader to the default value
+Thread.currentThread.setContextClassLoader(originalClassLoader)
--- End diff --

Can we use the following pattern?
```
// Get the original classloader
try {
// do our test
} finally {
// reset our classloader
}
```
So, we will not mess up the classloader even if the test somehow 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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157453097
  
@choochootrain Please open a pr for master branch (this is our typical 
workflow). So, we can fix it in master and branch-1.6. I'd prefer fixing master 
and branch 1.6 first and then backport the fix to 1.5.


---
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157554750
  
**[Test build #46124 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46124/consoleFull)**
 for PR 9779 at commit 
[`93939c3`](https://github.com/apache/spark/commit/93939c3f485cd8af9a544a0458a505042a54084c).


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157552644
  
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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157485843
  
sounds good, i'll go ahead and squash the commits and submit a pr to master 
and branch 1.6


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r45110137
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +123,64 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  /**
+   * SPARK-11195
+
+   * Make sure we are using the context classloader when deserializing 
failed TaskResults instead
+   * of the Spark classloader.
+
+   * This test compiles a jar containing an exception and tests that when 
it is thrown on the
+   * executor, enqueueFailedTask can correctly deserialize the failure and 
identify the thrown
+   * exception as the cause.
+
+   * Before this fix, enqueueFailedTask would throw a 
ClassNotFoundException when deserializing
+   * the exception, resulting in an UnknownReason for the TaskEndResult.
+   */
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// load the exception from the jar
+val originalClassLoader = Thread.currentThread.getContextClassLoader
+val loader = new MutableURLClassLoader(new Array[URL](0), 
originalClassLoader)
+loader.addURL(jarFile.toURI.toURL)
+Thread.currentThread().setContextClassLoader(loader)
+val excClass: Class[_] = Utils.classForName("repro.MyException")
+
+// NOTE: we must run the cluster with "local" so that the executor can 
load the compiled
+// jar.
+sc = new SparkContext("local", "test", conf)
+val rdd = sc.parallelize(Seq(1), 1).map(x => {
+  val exc = excClass.newInstance().asInstanceOf[Exception]
+  throw exc
+})
+
+// the driver should not have any problems resolving the exception 
class and determining
+// why the task failed.
+val exceptionMessage = intercept[SparkException] {
+  rdd.collect()
+}.getMessage
+
+val expectedFailure = """(?s).*Lost task.*: repro.MyException.*""".r
+val unknownFailure = """(?s).*Lost task.*: UnknownReason.*""".r
+
+assert(expectedFailure.findFirstMatchIn(exceptionMessage).isDefined)
+assert(unknownFailure.findFirstMatchIn(exceptionMessage).isEmpty)
+
+// reset the classloader to the default value
+Thread.currentThread.setContextClassLoader(originalClassLoader)
--- End 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 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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157578249
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46124/
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157578149
  
**[Test build #46124 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46124/consoleFull)**
 for PR 9779 at commit 
[`93939c3`](https://github.com/apache/spark/commit/93939c3f485cd8af9a544a0458a505042a54084c).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`class HashingTF(override val uid: String)`\n  * `class Interaction 
@Since(\"1.6.0\") (override val uid: String) extends Transformer`\n  * `class 
Normalizer(override val uid: String)`\n  * `class SQLTransformer 
@Since(\"1.6.0\") (override val uid: String) extends Transformer with Writable 
`\n  * `class Tokenizer(override val uid: String)`\n  * `  case class 
ArrayConversion(elementConversion: JDBCConversion) extends JDBCConversion`\n  * 
`abstract class JdbcDialect extends Serializable `\n


---
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157578248
  
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-11195][CORE] Use correct classloader fo...

[andrewor14]

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

https://github.com/apache/spark/pull/9779#discussion_r45154648
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +123,66 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  /**
+   * SPARK-11195
+
+   * Make sure we are using the context classloader when deserializing 
failed TaskResults instead
+   * of the Spark classloader.
+
+   * This test compiles a jar containing an exception and tests that when 
it is thrown on the
+   * executor, enqueueFailedTask can correctly deserialize the failure and 
identify the thrown
+   * exception as the cause.
+
+   * Before this fix, enqueueFailedTask would throw a 
ClassNotFoundException when deserializing
+   * the exception, resulting in an UnknownReason for the TaskEndResult.
+   */
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// ensure we reset the classloader after the test completes
+val originalClassLoader = Thread.currentThread.getContextClassLoader
+try {
+  // load the exception from the jar
+  val loader = new MutableURLClassLoader(new Array[URL](0), 
originalClassLoader)
+  loader.addURL(jarFile.toURI.toURL)
+  Thread.currentThread().setContextClassLoader(loader)
+  val excClass: Class[_] = Utils.classForName("repro.MyException")
+
+  // NOTE: we must run the cluster with "local" so that the executor 
can load the compiled
+  // jar.
+  sc = new SparkContext("local", "test", conf)
+  val rdd = sc.parallelize(Seq(1), 1).map(x => {
+val exc = excClass.newInstance().asInstanceOf[Exception]
+throw exc
+  })
--- End diff --

style
```
.map { x =>
  val exc = ...
  throw exc
}
```


---
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-11195][CORE] Use correct classloader fo...

[andrewor14]

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

https://github.com/apache/spark/pull/9779#discussion_r45154563
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +123,66 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  /**
+   * SPARK-11195
+
+   * Make sure we are using the context classloader when deserializing 
failed TaskResults instead
+   * of the Spark classloader.
+
+   * This test compiles a jar containing an exception and tests that when 
it is thrown on the
+   * executor, enqueueFailedTask can correctly deserialize the failure and 
identify the thrown
+   * exception as the cause.
+
+   * Before this fix, enqueueFailedTask would throw a 
ClassNotFoundException when deserializing
+   * the exception, resulting in an UnknownReason for the TaskEndResult.
+   */
+  test("failed task deserialized with the correct classloader") {
--- End diff --

I would put the `(SPARK-11195)` in the test name to be consistent with 
other tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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-11195][CORE] Use correct classloader fo...

[andrewor14]

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

https://github.com/apache/spark/pull/9779#discussion_r45154680
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +123,66 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  /**
+   * SPARK-11195
+
+   * Make sure we are using the context classloader when deserializing 
failed TaskResults instead
+   * of the Spark classloader.
+
+   * This test compiles a jar containing an exception and tests that when 
it is thrown on the
+   * executor, enqueueFailedTask can correctly deserialize the failure and 
identify the thrown
+   * exception as the cause.
+
+   * Before this fix, enqueueFailedTask would throw a 
ClassNotFoundException when deserializing
+   * the exception, resulting in an UnknownReason for the TaskEndResult.
+   */
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// ensure we reset the classloader after the test completes
+val originalClassLoader = Thread.currentThread.getContextClassLoader
+try {
+  // load the exception from the jar
+  val loader = new MutableURLClassLoader(new Array[URL](0), 
originalClassLoader)
+  loader.addURL(jarFile.toURI.toURL)
+  Thread.currentThread().setContextClassLoader(loader)
+  val excClass: Class[_] = Utils.classForName("repro.MyException")
+
+  // NOTE: we must run the cluster with "local" so that the executor 
can load the compiled
+  // jar.
+  sc = new SparkContext("local", "test", conf)
+  val rdd = sc.parallelize(Seq(1), 1).map(x => {
+val exc = excClass.newInstance().asInstanceOf[Exception]
+throw exc
+  })
+
+  // the driver should not have any problems resolving the exception 
class and determining
+  // why the task failed.
+  val exceptionMessage = intercept[SparkException] {
+rdd.collect()
+  }.getMessage
+
+  val expectedFailure = """(?s).*Lost task.*: repro.MyException.*""".r
+  val unknownFailure = """(?s).*Lost task.*: UnknownReason.*""".r
+
+  assert(expectedFailure.findFirstMatchIn(exceptionMessage).isDefined)
+  assert(unknownFailure.findFirstMatchIn(exceptionMessage).isEmpty)
+} finally {
+  Thread.currentThread.setContextClassLoader(originalClassLoader)
--- End diff --

do we need to clean up the dirs and jars?


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9779#discussion_r45168978
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +123,66 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  /**
+   * SPARK-11195
+
+   * Make sure we are using the context classloader when deserializing 
failed TaskResults instead
+   * of the Spark classloader.
+
+   * This test compiles a jar containing an exception and tests that when 
it is thrown on the
+   * executor, enqueueFailedTask can correctly deserialize the failure and 
identify the thrown
+   * exception as the cause.
+
+   * Before this fix, enqueueFailedTask would throw a 
ClassNotFoundException when deserializing
+   * the exception, resulting in an UnknownReason for the TaskEndResult.
+   */
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// ensure we reset the classloader after the test completes
+val originalClassLoader = Thread.currentThread.getContextClassLoader
+try {
+  // load the exception from the jar
+  val loader = new MutableURLClassLoader(new Array[URL](0), 
originalClassLoader)
+  loader.addURL(jarFile.toURI.toURL)
+  Thread.currentThread().setContextClassLoader(loader)
+  val excClass: Class[_] = Utils.classForName("repro.MyException")
+
+  // NOTE: we must run the cluster with "local" so that the executor 
can load the compiled
+  // jar.
+  sc = new SparkContext("local", "test", conf)
+  val rdd = sc.parallelize(Seq(1), 1).map(x => {
+val exc = excClass.newInstance().asInstanceOf[Exception]
+throw exc
+  })
+
+  // the driver should not have any problems resolving the exception 
class and determining
+  // why the task failed.
+  val exceptionMessage = intercept[SparkException] {
+rdd.collect()
+  }.getMessage
+
+  val expectedFailure = """(?s).*Lost task.*: repro.MyException.*""".r
+  val unknownFailure = """(?s).*Lost task.*: UnknownReason.*""".r
+
+  assert(expectedFailure.findFirstMatchIn(exceptionMessage).isDefined)
+  assert(unknownFailure.findFirstMatchIn(exceptionMessage).isEmpty)
+} finally {
+  Thread.currentThread.setContextClassLoader(originalClassLoader)
--- End diff --

i don't think so - `createTempDir` adds a shutdown hook to delete the 
directory


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9779#discussion_r45168868
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +123,66 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  /**
+   * SPARK-11195
+
+   * Make sure we are using the context classloader when deserializing 
failed TaskResults instead
+   * of the Spark classloader.
+
+   * This test compiles a jar containing an exception and tests that when 
it is thrown on the
+   * executor, enqueueFailedTask can correctly deserialize the failure and 
identify the thrown
+   * exception as the cause.
+
+   * Before this fix, enqueueFailedTask would throw a 
ClassNotFoundException when deserializing
+   * the exception, resulting in an UnknownReason for the TaskEndResult.
+   */
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// ensure we reset the classloader after the test completes
+val originalClassLoader = Thread.currentThread.getContextClassLoader
+try {
+  // load the exception from the jar
+  val loader = new MutableURLClassLoader(new Array[URL](0), 
originalClassLoader)
+  loader.addURL(jarFile.toURI.toURL)
+  Thread.currentThread().setContextClassLoader(loader)
+  val excClass: Class[_] = Utils.classForName("repro.MyException")
+
+  // NOTE: we must run the cluster with "local" so that the executor 
can load the compiled
+  // jar.
+  sc = new SparkContext("local", "test", conf)
+  val rdd = sc.parallelize(Seq(1), 1).map(x => {
+val exc = excClass.newInstance().asInstanceOf[Exception]
+throw exc
+  })
--- End 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 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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9779#discussion_r45168854
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +123,66 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  /**
+   * SPARK-11195
+
+   * Make sure we are using the context classloader when deserializing 
failed TaskResults instead
+   * of the Spark classloader.
+
+   * This test compiles a jar containing an exception and tests that when 
it is thrown on the
+   * executor, enqueueFailedTask can correctly deserialize the failure and 
identify the thrown
+   * exception as the cause.
+
+   * Before this fix, enqueueFailedTask would throw a 
ClassNotFoundException when deserializing
+   * the exception, resulting in an UnknownReason for the TaskEndResult.
+   */
+  test("failed task deserialized with the correct classloader") {
--- End 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 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-11195][CORE] Use correct classloader fo...

[choochootrain]

GitHub user choochootrain opened a pull request:

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

[SPARK-11195][CORE] Use correct classloader for TaskResultGetter

Make sure we are using the context classloader when deserializing failed 
TaskResults instead of the Spark classloader.

The issue is that `enqueueFailedTask` was using the incorrect classloader 
which results in `ClassNotFoundException`.

Adds a test in TaskResultGetterSuite that compiles a custom exception, 
throws it on the executor, and asserts that Spark handles the TaskResult 
deserialization instead of returning `UnknownReason`.

See #9367 for previous comments
See SPARK-11195 for a full repro

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

$ git pull https://github.com/choochootrain/spark spark-11195-master

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

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


commit 93939c3f485cd8af9a544a0458a505042a54084c
Author: Hurshal Patel 
Date:   2015-10-30T01:05:44Z

[SPARK-11195][CORE] Use correct classloader for TaskResultGetter

Make sure we are using the context classloader when deserializing failed
TaskResults instead of the Spark classloader.

Adds a test in TaskResultGetterSuite that compiles a custom exception,
throws it on the executor, and asserts that Spark handles the
TaskResult deserialization properly.




---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157522927
  
@yhuai any other comments?


---
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9779#issuecomment-157523161
  
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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r44965129
  
--- Diff: core/src/main/scala/org/apache/spark/TestUtils.scala ---
@@ -130,7 +131,7 @@ private[spark] object TestUtils {
 val fileName = className + ".class"
 val result = new File(fileName)
 assert(result.exists(), "Compiled file not found: " + 
result.getAbsolutePath())
-val out = new File(destDir, fileName)
+val out = new File(destDir, result.getName)
--- End diff --

whoops, leftover from the previous changes


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r44965069
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -17,8 +17,13 @@
 
 package org.apache.spark.scheduler
 
+import java.io.File
+import java.net.URL
 import java.nio.ByteBuffer
 
+import org.apache.spark.TestUtils.JavaSourceFromString
+import org.apache.spark.util.{MutableURLClassLoader, Utils}
--- End 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 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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r44967313
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +124,47 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  // SPARK-11195
+  // Make sure we are using the context classloader when deserializing 
failed TaskResults instead of
+  // the Spark classloader.
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// load the exception from the jar
+val loader = new MutableURLClassLoader(new Array[URL](0), 
Thread.currentThread.getContextClassLoader)
+loader.addURL(jarFile.toURI.toURL)
+Thread.currentThread().setContextClassLoader(loader)
+val excClass: Class[_] = Class.forName("repro.MyException", false, 
loader)
+
+sc = new SparkContext("local", "test", conf)
--- End 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 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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r44964392
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +124,47 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  // SPARK-11195
+  // Make sure we are using the context classloader when deserializing 
failed TaskResults instead of
+  // the Spark classloader.
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// load the exception from the jar
+val loader = new MutableURLClassLoader(new Array[URL](0), 
Thread.currentThread.getContextClassLoader)
+loader.addURL(jarFile.toURI.toURL)
+Thread.currentThread().setContextClassLoader(loader)
--- End 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 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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r44964408
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +124,47 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  // SPARK-11195
+  // Make sure we are using the context classloader when deserializing 
failed TaskResults instead of
+  // the Spark classloader.
+  test("failed task deserialized with the correct classloader") {
--- End 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 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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r44967864
  
--- Diff: core/src/main/scala/org/apache/spark/TestUtils.scala ---
@@ -78,15 +79,15 @@ private[spark] object TestUtils {
   }
 
   /**
-   * Create a jar file that contains this set of files. All files will be 
located at the root
-   * of the jar.
+   * Create a jar file that contains this set of files. All files will be 
located in the specified
+   * directory or at the root of the jar.
*/
-  def createJar(files: Seq[File], jarFile: File): URL = {
+  def createJar(files: Seq[File], jarFile: File, directoryPrefix: 
Option[String] = None): URL = {
--- End diff --

afaik you cannot import a class from a jar unless it has declared a package 
and the `.class` resides in the corresponding directory in the jar.

without `directoryPrefix`, `createJar` will place all of your classes into 
the root directory of the jar so you cannot actually import anything from them. 
it looks like this is the first use of `createJar` in the tests that goes on to 
load classes from the jar so there was no need for this before.

in the spirit of keeping this change as minimal as possible, i added a 
`directoryPrefix` option but i am open to other suggestions.


---
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157153668
  
  [Test build #46008 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46008/consoleFull)
 for   PR 9367 at commit 
[`fadf2ca`](https://github.com/apache/spark/commit/fadf2ca57e21d5f2600d4ec3e504e20db596c35c).


---
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157201464
  
**[Test build #46008 timed 
out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/46008/console)**
 for PR 9367 at commit 
[`fadf2ca`](https://github.com/apache/spark/commit/fadf2ca57e21d5f2600d4ec3e504e20db596c35c)
 after a configured wait of `175m`.


---
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157201590
  
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-157201592
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/46008/
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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r44880055
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +124,47 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  // SPARK-11195
+  // Make sure we are using the context classloader when deserializing 
failed TaskResults instead of
+  // the Spark classloader.
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// load the exception from the jar
+val loader = new MutableURLClassLoader(new Array[URL](0), 
Thread.currentThread.getContextClassLoader)
+loader.addURL(jarFile.toURI.toURL)
+Thread.currentThread().setContextClassLoader(loader)
--- End diff --

Can we set the original loader back?


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r44880126
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +124,47 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  // SPARK-11195
+  // Make sure we are using the context classloader when deserializing 
failed TaskResults instead of
+  // the Spark classloader.
+  test("failed task deserialized with the correct classloader") {
--- End diff --

Can we add comments to outline how this fix is tested?


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r44880215
  
--- Diff: core/src/main/scala/org/apache/spark/TestUtils.scala ---
@@ -78,15 +79,15 @@ private[spark] object TestUtils {
   }
 
   /**
-   * Create a jar file that contains this set of files. All files will be 
located at the root
-   * of the jar.
+   * Create a jar file that contains this set of files. All files will be 
located in the specified
+   * directory or at the root of the jar.
*/
-  def createJar(files: Seq[File], jarFile: File): URL = {
+  def createJar(files: Seq[File], jarFile: File, directoryPrefix: 
Option[String] = None): URL = {
--- End diff --

Do we need to add `directoryPrefix`?


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r44880369
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -119,5 +124,47 @@ class TaskResultGetterSuite extends SparkFunSuite with 
BeforeAndAfter with Local
 // Make sure two tasks were run (one failed one, and a second retried 
one).
 assert(scheduler.nextTaskId.get() === 2)
   }
+
+  // SPARK-11195
+  // Make sure we are using the context classloader when deserializing 
failed TaskResults instead of
+  // the Spark classloader.
+  test("failed task deserialized with the correct classloader") {
+// compile a small jar containing an exception that will be thrown on 
an executor.
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+val excSource = new JavaSourceFromString(new File(srcDir, 
"MyException").getAbsolutePath,
+  """package repro;
+|
+|public class MyException extends Exception {
+|}
+  """.stripMargin)
+val excFile = TestUtils.createCompiledClass("MyException", srcDir, 
excSource, Seq.empty)
+val jarFile = new File(tempDir, 
"testJar-%s.jar".format(System.currentTimeMillis()))
+TestUtils.createJar(Seq(excFile), jarFile, directoryPrefix = 
Some("repro"))
+
+// load the exception from the jar
+val loader = new MutableURLClassLoader(new Array[URL](0), 
Thread.currentThread.getContextClassLoader)
+loader.addURL(jarFile.toURI.toURL)
+Thread.currentThread().setContextClassLoader(loader)
+val excClass: Class[_] = Class.forName("repro.MyException", false, 
loader)
+
+sc = new SparkContext("local", "test", conf)
--- End diff --

I think we need to add a comment to say we need to use `local` in order to 
make this test work. Otherwise, that jar will not be loaded at the executor 
side.


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r44880391
  
--- Diff: core/src/main/scala/org/apache/spark/TestUtils.scala ---
@@ -130,7 +131,7 @@ private[spark] object TestUtils {
 val fileName = className + ".class"
 val result = new File(fileName)
 assert(result.exists(), "Compiled file not found: " + 
result.getAbsolutePath())
-val out = new File(destDir, fileName)
+val out = new File(destDir, result.getName)
--- End diff --

why?


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r44880250
  
--- Diff: 
core/src/test/scala/org/apache/spark/scheduler/TaskResultGetterSuite.scala ---
@@ -17,8 +17,13 @@
 
 package org.apache.spark.scheduler
 
+import java.io.File
+import java.net.URL
 import java.nio.ByteBuffer
 
+import org.apache.spark.TestUtils.JavaSourceFromString
+import org.apache.spark.util.{MutableURLClassLoader, Utils}
--- End diff --

Let's order imports according to 
https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide#SparkCodeStyleGuide-Imports.


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-156603526
  
should be less invasive now :)
note that i still have to make some minor changes to `TestUtils` in order 
for the jar to be in the correct format.


---
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-156604554
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/45913/
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-156604552
  
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-156604550
  
  [Test build #45913 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45913/console)
 for   PR 9367 at commit 
[`71f8df9`](https://github.com/apache/spark/commit/71f8df97e6cf3c477101c7fce8c195ae8e0c829f).
 * This patch **fails Scala style 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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-156604441
  
  [Test build #45913 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/45913/consoleFull)
 for   PR 9367 at commit 
[`71f8df9`](https://github.com/apache/spark/commit/71f8df97e6cf3c477101c7fce8c195ae8e0c829f).


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-155865241
  
@srowen @andrewor14 @yhuai

it seems like the SPARK-8489 approach would be less invasive, but also less 
maintainable. any preferences? i'd like to stick with one and get this out asap.


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-155879290
  
I'd like to go with 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-11195][CORE] Use correct classloader fo...

[brkyvz]

Github user brkyvz commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-155869908
  
My vote is with (3). I feel it requires the least amount of new code that 
you have to write and is more maintainable.


---
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-11195][CORE] Use correct classloader fo...

[andrewor14]

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

https://github.com/apache/spark/pull/9367#discussion_r7058
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

We should go with the simplest option that reproduces the issue. In other 
`SparkSubmitSuite` tests we used (2) but only out of necessity, where we just 
prepackage a jar and put it in the test resources dir. This makes it a little 
hard to maintain, e.g. it doesn't work with scala-2.11.

In this case, maybe (3) is the simplest and most maintainable. It's 
unlikely that we'll ever have to modify `MyException`, but the reproduction 
code itself should be kept flexible. Could you give it a try?


---
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-11195][CORE] Use correct classloader fo...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-155353765
  
@yhuai @choochootrain how is this one going? it does seem desirable to test 
this as much as possible. If we're having to introduce very complex mechanisms 
to do so and they aren't working, is there anything simpler even if less 
effective we can do to test it?


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r43942556
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

all this patch does is make `TaskResult` deserialization use 
`Utils.getContextOrSparkClassLoader` (the classloader which loaded the 
`spark-submit`ed jar) instead of `Utils.getSparkClassLoader` (this is 
`AppClassLoader` which only has spark classes in it). without this patch, a 
failed task would not be able to deserialize an exception if it did not exist 
in `Utils.getSparkClassLoader`.

in order to reproduce this issue, i set up a situation where 
`Utils.getContextOnSparkClassLoader` contains `MyException` but 
`Utils.getSparkClassLoader` does not (see 
https://issues.apache.org/jira/browse/SPARK-11195). this is easy to manually 
test with `spark-submit` and a user defined exception, but turning this into an 
automated test is proving to be much trickier. here are the 3 options:

* :x: if i place all of the code into `SparkSubmitSuite`, the bug won't be 
hit because `MyException` will be in the root classloader and my patch makes no 
difference.

* :grey_question: if i place all of the code into an external jar and run 
`spark-submit`, i can set up the same situation as my repro which found this 
bug. the issue i am running into is that i need a spark classpath in order to 
compile my jar. i can use the assembled jar, but this changes depending on the 
maven profiles that are enabled and so on.

* :grey_question: i can try @brkyvz & @yhuai's hybrid approach of putting 
only the exception into a jar and the rest of the code into `SparkSubmitSuite`. 
i will have to do the following in order to repro this issue:
  * load the jar with `MyException` in a new classloader whose parent is 
the root classloader
  * somehow allow this classloader to be used by the driver and the 
executor *without* changing `Utils.getSparkClassLoader`.

 at this point am i not reimplementing `spark-submit`? :)

the final approach is certainly worth trying, i'll take a look at it later 
today.


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

[GitHub] spark pull request: [SPARK-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r43936635
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

Maybe you can compile a jar that just include your jar and put your app 
below. In your app, you use reflection to create an instance of your 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

[GitHub] spark pull request: [SPARK-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r43922931
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

@JoshRosen @andrewor14 any idea on how to test 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 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-11195][CORE] Use correct classloader fo...

[brkyvz]

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

https://github.com/apache/spark/pull/9367#discussion_r43927722
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

Or maybe I didn't understand the issue that this patch is trying to solve 
very well


---
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-11195][CORE] Use correct classloader fo...

[brkyvz]

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

https://github.com/apache/spark/pull/9367#discussion_r43927641
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

Why do you need to compile using Spark? Can't you just create your own 
exception, compile it, pass that to Spark Submit. Then move all of the code 
here down to something like `SimpleApplicationTest`, where you create your 
exception through reflection and then throw it?


---
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153543844
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44987/
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153543840
  
  [Test build #44987 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44987/console)
 for   PR 9367 at commit 
[`c63ca09`](https://github.com/apache/spark/commit/c63ca091714c464f22e9e28b8605aa8c551eaa87).
 * This patch **fails Scala style 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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153543299
  
Merged build started.


---
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153543842
  
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153543543
  
  [Test build #44987 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44987/consoleFull)
 for   PR 9367 at commit 
[`c63ca09`](https://github.com/apache/spark/commit/c63ca091714c464f22e9e28b8605aa8c551eaa87).


---
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153543287
  
 Merged build triggered.


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153550645
  
whoops, fixed the style error.


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r43836051
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

the only remaining issue is that this jar is hardcoded for the maven 
profiles that i am using. i was experimenting with putting the entire maven 
`target/classes` directory on the classpath but that seems equally janky. any 
suggestions 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 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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153550807
  
Merged build started.


---
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153551172
  
  [Test build #44991 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44991/consoleFull)
 for   PR 9367 at commit 
[`53f7c4c`](https://github.com/apache/spark/commit/53f7c4cda8216ce6db726ae88fbf89776da62b27).


---
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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153572051
  
  [Test build #44991 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44991/console)
 for   PR 9367 at commit 
[`53f7c4c`](https://github.com/apache/spark/commit/53f7c4cda8216ce6db726ae88fbf89776da62b27).
 * 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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r43840784
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

I probably missed something. Why do we need to put spark's assembly jar at 
here? When we run test, spark's classes are already in class path.


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r43844348
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

in order to test this issue, i need to submit an external jar to 
`spark-submit`. i can compile my job it using `TestUtils`, but i need to put 
(some version) of spark on the classpath so that `javac` can resolve `RDD` and 
so on. 


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r43840823
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

This is what our jenkins does.


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

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

https://github.com/apache/spark/pull/9367#discussion_r43840818
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

btw, when you unit test `SparkSubmitSuite`, you have to do 
`assembly/assembly` before you can run any of these tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153572088
  
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153572089
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44991/
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153550780
  
 Merged build triggered.


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

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

https://github.com/apache/spark/pull/9367#discussion_r43844511
  
--- Diff: 
core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala ---
@@ -366,6 +367,72 @@ class SparkSubmitSuite
 }
   }
 
+  // SPARK-11195
+  test("classes are correctly loaded when tasks fail") {
+// Compile a simple jar that throws a user defined exception on the 
driver
+val tempDir = Utils.createTempDir()
+val srcDir = new File(tempDir, "repro/")
+srcDir.mkdirs()
+// scalastyle:off line.size.limit
+val mainSource = new JavaSourceFromString(new File(srcDir, 
"MyJob").getAbsolutePath,
+  """package repro;
+|
+|import java.util.*;
+|import java.util.regex.*;
+|import org.apache.spark.*;
+|import org.apache.spark.api.java.*;
+|import org.apache.spark.api.java.function.*;
+|
+|public class MyJob {
+|  public static class MyException extends Exception {
+|  }
+|
+|  public static void main(String[] args) {
+|SparkConf conf = new SparkConf();
+|JavaSparkContext sc = new JavaSparkContext(conf);
+|
+|JavaRDD rdd = sc.parallelize(Arrays.asList(new Integer[]{1}), 
1).map(new Function() {
+|  public Boolean call(Integer x) throws MyException {
+|throw new MyException();
+|  }
+|});
+|
+|try {
+|  rdd.collect();
+|
+|  assert(false); // should be unreachable
+|} catch (Exception e) {
+|  // the driver should not have any problems resolving the 
exception class and determining
+|  // why the task failed.
+|
+|  Pattern unknownFailure = Pattern.compile(".*Lost task.*: 
UnknownReason.*", Pattern.DOTALL);
+|  Pattern expectedFailure = Pattern.compile(".*Lost task.*: 
repro.MyJob\\$MyException.*", Pattern.DOTALL);
+|
+|  assert(!unknownFailure.matcher(e.getMessage()).matches());
+|  assert(expectedFailure.matcher(e.getMessage()).matches());
+|}
+|  }
+|}
+  """.stripMargin)
+// scalastyle:on line.size.limit
+val sparkJar = 
"../assembly/target/scala-2.10/spark-assembly-1.5.1-hadoop2.2.0.jar"
--- End diff --

so the test `classes are correctly loaded when tasks fail` is failing 
because of the above issue: 
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44991/consoleFull

i'm more than happy to take another approach if i'm going about this the 
wrong way.


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153244789
  
@yhuai yep this should also be in master. should I also submit a pr on 
master when this one looks good or will a maintainer be able to cherry-pick the 
commit?


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153248208
  
@choochootrain It will be great if you can submit a pr against the master. 
Our merge script only cherry-pick commit from master to a branch.


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153191848
  
i'm writing the test right now, is there an easy way to get the relative 
path to the assembled spark jar so I can compile my job against it?


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153192578
  
@choochootrain I think you can write your app in SparkSubmitSuite 
(https://github.com/apache/spark/blob/master/core/src/test/scala/org/apache/spark/deploy/SparkSubmitSuite.scala#L590-L614
 is an example).


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153237759
  
@choochootrain I just noticed that this PR is for branch-1.5. Should we 
also fix it in 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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-153221384
  
@yhuai i can't write the test directly in `SparkSubmitSuite` because this 
is a classloader issue that only repros (as far as I can tell) when an external 
jar is loaded. I'm adding a test that uses `TestUtils.createCompiledClass` to 
compile and submit my external jar. 


---
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-11195][CORE] Use correct classloader fo...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-152822504
  
@choochootrain are you able to put together a small test like what @yhuai 
mentions? then I think this is good to go.


---
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-11195][CORE] Use correct classloader fo...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-152530948
  
This looks good as I'm not otherwise clear why these two blocks would use 
different classloaders.


---
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-11195][CORE] Use correct classloader fo...

[choochootrain]

GitHub user choochootrain opened a pull request:

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

[SPARK-11195][CORE] Use correct classloader for TaskResultGetter

Make sure we are using the context classloader when deserializing failed 
TaskResults instead of the Spark classloader.

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

$ git pull https://github.com/choochootrain/spark spark-11195

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

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


commit 90c47aa133de83dd49dea00b0949088e8600ab23
Author: Hurshal Patel 
Date:   2015-10-30T01:05:44Z

[SPARK-11195][CORE] Use correct classloader for TaskResultGetter

Make sure we are using the context classloader when deserializing failed
TaskResults instead of the Spark classloader.




---
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-11195][CORE] Use correct classloader fo...

[JoshRosen]

Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-152372506
  
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-11195][CORE] Use correct classloader fo...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-152372707
  
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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-152372967
  
I have a manual test that exhibits this behavior in 
https://issues.apache.org/jira/browse/SPARK-11195 and I am working on adding a 
test to the repo.

In order to test this I basically want to mirror the classloader hierarchy 
created by `spark-submit` - are there any conventions or existing tests which 
do something like this that I can look at?


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-152396959
  
Will `SparkSubmitSuite` help?


---
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-11195][CORE] Use correct classloader fo...

[yhuai]

Github user yhuai commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-152402475
  
How about this. In the main method, you can catch the exception and if it 
is the expected type, let the main method finish successfully. Otherwise, throw 
an exception, which causes a non-zero exit code. In `runSparkSubmit`, we will 
find that the exit code is not 0 and fail the 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-11195][CORE] Use correct classloader fo...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-152415520
  
  [Test build #44655 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/44655/console)
 for   PR 9367 at commit 
[`90c47aa`](https://github.com/apache/spark/commit/90c47aa133de83dd49dea00b0949088e8600ab23).
 * 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-11195][CORE] Use correct classloader fo...

[choochootrain]

Github user choochootrain commented on the pull request:

https://github.com/apache/spark/pull/9367#issuecomment-152398230
  
`SparkSubmitSuite` is helpful, but I want to catch and assert the type of 
exception that is thrown when the job fails - calling into `doMain` in 
`SparkSubmit.scala` would be closer?


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