[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-40284736 Jenkins, retest this please. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-40284773 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-40284781 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-40285726 All automated tests passed. Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14078/ --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-40285724 Merged build finished. All automated tests 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-40298968 Thanks for the fix - merged into master and 1.0 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/322 --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39976847 @pwendell: Do you plan to pick this up for 1.0? Is there anything more I need to do? --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39699481 Okay I think the broader issue is that @ueshin you said before that `Class.forName()` method with `null` for 3rd argument tries to load class from bootstrap class loader, which doesn't know the class `org.apache.spark.serializer.JavaSerializer`. But I think in this case we'd expect the bootstrap classloader to know about `JavaSerializer` (this should be on the classpath when the executor starts), right? I'm still not sure why it would fail in this case. I don't see why `MesosExecutorDriver` could be on the java classpath but `JavaSerializer` isn't. @manku-timma I looked more and the reason this doesn't work is that it looks like other parts of the code don't directly use the `classLoader` form the executor. I can look more tomorrow and see how we can best clean this up. The current approach works but it's a bit of a hack. There might be a nicer way to clean this up. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39699792 Ah I see, @ueshin you are right. It's the bootstrap class loader and it won't have any spark definitions. I was mixing this up with the system class loader. ``` ./bin/spark-shell scala Class.forName(org.apache.spark.serializer.JavaSerializer) res7: Class[_] = class org.apache.spark.serializer.JavaSerializer scala Class.forName(org.apache.spark.serializer.JavaSerializer, true, null) java.lang.ClassNotFoundException: org/apache/spark/serializer/JavaSerializer at java.lang.Class.forName0(Native Method) at java.lang.Class.forName(Class.java:270) at $iwC$$iwC$$iwC$$iwC.init(console:11) at $iwC$$iwC$$iwC.init(console:16) at $iwC$$iwC.init(console:18) at $iwC.init(console:20) ``` We should definitely clean this up. The behavior we want in every case is to either use the context class loader (if present) and if not use the classloader that loads spark classes (e.g. the system 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user ueshin commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39700076 @pwendell Yes, the bootstrap class loader knows only core Java APIs and the Spark classes (specified by `-cp` java command argument) are loaded by the system class loader. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39720333 So the current fix looks fine? --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39674063 Hey @ueshin @manku-timma - as a simpler fix, would it be possible to just change the way SparkEnv captures the class loader? I think it was probably just an oversight when that was added. If there is not a context class loader, then it should just load the bootstrap class loader: ``` val classLoader = Option(Thread.currentThread.getContextClassLoader).getOrElse(getClass.getClassLoader) ``` --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request:
https://github.com/apache/spark/pull/322#issuecomment-39676303
@pwendell: I tested your fix to SparkEnv.scala (after reverting my earlier
change). It does not work. SparkEnv's loader turns out to be
`sun.misc.Launcher$AppClassLoader@12360be0` and it fails with the following
error.
```
java.lang.ClassNotFoundException: org/apache/spark/io/LZFCompressionCodec
at java.lang.Class.forName0(Native Method)
at java.lang.Class.forName(Class.java:249)
at
org.apache.spark.io.CompressionCodec$.createCodec(CompressionCodec.scala:45)
at
org.apache.spark.io.CompressionCodec$.createCodec(CompressionCodec.scala:41)
at
org.apache.spark.broadcast.HttpBroadcast$.initialize(HttpBroadcast.scala:110)
at
org.apache.spark.broadcast.HttpBroadcastFactory.initialize(HttpBroadcast.scala:71)
at
org.apache.spark.broadcast.BroadcastManager.initialize(Broadcast.scala:82)
at
org.apache.spark.broadcast.BroadcastManager.init(Broadcast.scala:69)
at org.apache.spark.SparkEnv$.create(SparkEnv.scala:195)
at org.apache.spark.executor.Executor.init(Executor.scala:105)
at
org.apache.spark.executor.MesosExecutorBackend.registered(MesosExecutorBackend.scala:56)
```
My patch to `SparkEnv.scala`:
```
@@ -142,7 +142,10 @@ object SparkEnv extends Logging {
conf.set(spark.driver.port, boundPort.toString)
}
-val classLoader = Thread.currentThread.getContextClassLoader
+val classLoader = Option(Thread.currentThread.getContextClassLoader)
+ .getOrElse(getClass.getClassLoader)
```
---
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.
---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39676705 Another way of putting my question. Currently there is a line: ``` Thread.currentThread.setContextClassLoader(getClass.getClassLoader) ``` What is the actual classloader being set here... isn't it just the `sun.misc.Launcher` 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39689262 @pwendell: You are right. Actually `sun.misc.Launcher$AppClassLoader@12360be0` is the classloader even in the earlier code. Looks like classes directly loaded by SparkEnv get to use the right classloader since it is passed as an argument. But classes loaded at the second level (like in BroadcastManager above) still use the null classloader and fail. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39641219 Let me know if there is any other change I need to make. I have tested after merging from master and things look fine. This is good to be merged from my end. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user ueshin commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39643866 LGTM about the class loader issue. But I'm not sure the last fix i.e. Java7 API may be used or not. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39655348 I see that PR 334 made the java 6 change. So I reverted mine. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39541428 I can confirm that the third line is needed. Without that line I see the same failure as earlier. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39543041 Which exact failure is it? Could you post the stack trace? --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user ueshin commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39543185 Hi, I think `MesosExecutorBackend` should have own `ContextClassLoader` and it should be set to the class loader of `MesosExecutorBackend` class before it creates `Executor` object. I mistakenly thought that `MesosExecutorBackend` has own context class loader, but it doesn't. While creating `SparkEnv` in the `Executor`'s constructor, `Thread.currentThread.getContextClassLoader` returns `null` because `MesosExecutorBackend` doesn't have context class loader. `Class.forName()` method with `null` for 3rd argument tries to load class from bootstrap class loader, which doesn't know the class `org.apache.spark.serializer.JavaSerializer`. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39543328 @ueshin - ah cool. Do you mind giving a code snippet of what that would look like? Then @manku-timma can see if it fixes the problem. Probably is a better solution... --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39543590 java.lang.ClassNotFoundException: org/apache/spark/serializer/JavaSerializer at java.lang.Class.forName0(Native Method) at java.lang.Class.forName(Class.java:249) at org.apache.spark.SparkEnv$.instantiateClass$1(SparkEnv.scala:173) at org.apache.spark.SparkEnv$.create(SparkEnv.scala:184) at org.apache.spark.executor.Executor.init(Executor.scala:115) at org.apache.spark.executor.MesosExecutorBackend.registered(MesosExecutorBackend.scala:56) Exception in thread Thread-0 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user ueshin commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39546729 Put the following line at the beginning of `registered` method (at [line 53](https://github.com/manku-timma/spark-1/blob/89109d7bf2ce35e58a252bd7144438fc720ee362/core/src/main/scala/org/apache/spark/executor/MesosExecutorBackend.scala#L53)) ``` Thread.currentThread.setContextClassLoader(getClass.getClassLoader) ``` --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user ueshin commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39550942 @manku-timma Yes, the standalone backend `org.apache.spark.executor.CoarseGrainedExecutorBackend` and the local-mode backend `org.apache.spark.scheduler.local.LocalActor` have their own context class loader set by `ActorSystem`. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user ueshin commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39551604 BTW, we might have to restore the context class loader of `MesosExecutorBackend` to bootstrap class loader. We should restore if there are 2 or more threads to handle `MesosExecutorBackend`, but I don't know how many threads there are. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39552826 @ueshin, your one line change works for me. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user ueshin commented on the pull request:
https://github.com/apache/spark/pull/322#issuecomment-39560562
@manku-timma, I just wondered that we should restore the class loader in
`registered` method like:
```scala
val cl = Thread.currentThread.getContextClassLoader
try {
Thread.currentThread.setContextClassLoader(getClass.getClassLoader)
logInfo(Registered with Mesos as executor ID +
executorInfo.getExecutorId.getValue)
this.driver = driver
val properties = Utils.deserialize[Array[(String,
String)]](executorInfo.getData.toByteArray)
executor = new Executor(
executorInfo.getExecutorId.getValue,
slaveInfo.getHostname,
properties)
} finally {
Thread.currentThread.setContextClassLoader(cl)
}
```
I think this is better than before.
The http based class loader for `TaskRunner` is correctly set at [this
point](https://github.com/manku-timma/spark-1/blob/89109d7bf2ce35e58a252bd7144438fc720ee362/core/src/main/scala/org/apache/spark/executor/Executor.scala#L180).
The flow you wrote would be:
- when `registered` method is called
- `MesosExecutorBackend` starts off
- `MesosExecutorBackend` sets classloader
- `MesosExecutorBackend` creates `Executor`
- `Executor` creates `SparkEnv`
- `MesosExecutorBackend` restore classlaoder
- when `launchTask` method is called
- `MesosExecutorBackend` calls `Executor.launchTask` method
- `Executor` creates `TaskRunner`
- `Executor` submits the task runner
- when the task runner is run
- `TaskRunner` sets classloader to the http based one (so that later
classes can be downloaded from the 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.
---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user ueshin commented on a diff in the pull request:
https://github.com/apache/spark/pull/322#discussion_r11318644
--- Diff:
core/src/main/scala/org/apache/spark/executor/MesosExecutorBackend.scala ---
@@ -50,13 +50,18 @@ private[spark] class MesosExecutorBackend
executorInfo: ExecutorInfo,
frameworkInfo: FrameworkInfo,
slaveInfo: SlaveInfo) {
-logInfo(Registered with Mesos as executor ID +
executorInfo.getExecutorId.getValue)
-this.driver = driver
-val properties = Utils.deserialize[Array[(String,
String)]](executorInfo.getData.toByteArray)
-executor = new Executor(
- executorInfo.getExecutorId.getValue,
- slaveInfo.getHostname,
- properties)
+val cl = Thread.currentThread.getContextClassLoader
+try {
--- End diff --
You need one more line here
```scala
Thread.currentThread.setContextClassLoader(getClass.getClassLoader)
```
---
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.
---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39627690 Oops. Added that line. I am facing this error in the current git tree ``` [error] /home/vagrant/spark2/sql/core/src/main/scala/org/apache/spark/sql/parque t/ParquetRelation.scala:106: value getGlobal is not a member of object java.util .logging.Logger [error] logger.setParent(Logger.getGlobal) [error] ^ [error] one error found ``` --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
GitHub user manku-timma opened a pull request: https://github.com/apache/spark/pull/322 [SPARK-1403] Move the class loader creation back to where it was in 0.9.0 [SPARK-1403] I investigated why spark 0.9.0 loads fine on mesos while spark 1.0.0 fails. What I found was that in SparkEnv.scala, while creating the SparkEnv object, the current thread's classloader is null. But in 0.9.0, at the same place, it is set to org.apache.spark.repl.ExecutorClassLoader . I saw that https://github.com/apache/spark/commit/7edbea41b43e0dc11a2de156be220db8b7952d01 moved it to it current place. I moved it back and saw that 1.0.0 started working fine on mesos. I just created a minimal patch that allows me to run spark on mesos correctly. It seems like SecurityManager's creation needs to be taken into account for a correct fix. Also moving the creation of the serializer out of SparkEnv might be a part of the right solution. PTAL. You can merge this pull request into a Git repository by running: $ git pull https://github.com/manku-timma/spark-1 spark-1403 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/322.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 #322 commit 89109d7bf2ce35e58a252bd7144438fc720ee362 Author: Bharath Bhushan manku.ti...@outlook.com Date: 2014-04-04T02:26:26Z move the class loader creation back to where it was in 0.9.0 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39526912 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user manku-timma commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39530349 After looking at the code a bit more, I see that the code to setContextClassLoader does not use SecurityManager AFAIS. createClassLoader is creating a File object. addReplClassLoaderIfNeeded is dynamically loading a class file. I dont see any use of SecurityManager in these two methods. I am not sure if it gets used implicitly. --- 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39533342 This patch also adds back a line that we earlier removed: ``` Thread.currentThread.setContextClassLoader(replClassLoader) ``` Does your fix require that line to work? If so, what is the error if you remove 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. ---
[GitHub] spark pull request: [SPARK-1403] Move the class loader creation ba...
Github user pwendell commented on the pull request: https://github.com/apache/spark/pull/322#issuecomment-39533417 @ueshin removed it in SPARK-1210: https://github.com/apache/spark/pull/15 I proposed a more conservative change in this comment: https://github.com/apache/spark/pull/15#issuecomment-38758426 But we ultimately went ahead and just removed it because we couldn't see a case where it was necessary... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
