-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-2996] Implement userClassPathFirst for driver, yarn. #3233
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
89522ef
a853e74
91f7e54
a314f2d
46d8cf2
d0394b8
4a84d87
0b64d92
55c88fa
20373f5
7f8603c
7d14397
50afa5f
a963ea3
89d8072
fa1aafa
d1273b2
54e1a98
35949c8
5304d64
7b57cba
44010b6
f513871
3730151
a10f379
b6497f9
2e6c4b7
fbb8ab5
3cb6498
25d4fed
fe970a7
0e6ef19
0fe7777
70d4044
0e6d6be
2ce3c7a
3f768e3
a1b8d7e
cabf962
a8c69f1
fa7df88
a1499e2
9cf9cf1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,7 @@ import org.apache.ivy.plugins.resolver.{ChainResolver, IBiblioResolver} | |
|
|
||
| import org.apache.spark.deploy.rest._ | ||
| import org.apache.spark.executor._ | ||
| import org.apache.spark.util.Utils | ||
| import org.apache.spark.util.{ChildFirstURLClassLoader, MutableURLClassLoader, Utils} | ||
|
|
||
| /** | ||
| * Whether to submit, kill, or request the status of an application. | ||
|
|
@@ -467,11 +467,11 @@ object SparkSubmit { | |
| } | ||
|
|
||
| val loader = | ||
| if (sysProps.getOrElse("spark.files.userClassPathFirst", "false").toBoolean) { | ||
| new ChildExecutorURLClassLoader(new Array[URL](0), | ||
| if (sysProps.getOrElse("spark.driver.userClassPathFirst", "false").toBoolean) { | ||
| new ChildFirstURLClassLoader(new Array[URL](0), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think we just use ChildExecutorURLClassLoader and ExecutorURLClassLoader that it has existed. so we reduce code to change.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That kinda clashes with Sandy's comment, which I agree with, that calling them "Executor" is misleading since they're used in other contexts. And frankly, compared to the other changes here, changing those class names is a very small one. |
||
| Thread.currentThread.getContextClassLoader) | ||
| } else { | ||
| new ExecutorURLClassLoader(new Array[URL](0), | ||
| new MutableURLClassLoader(new Array[URL](0), | ||
| Thread.currentThread.getContextClassLoader) | ||
| } | ||
| Thread.currentThread.setContextClassLoader(loader) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,32 +17,44 @@ | |
|
|
||
| package org.apache.spark.deploy.worker | ||
|
|
||
| import java.io.File | ||
|
|
||
| import akka.actor._ | ||
|
|
||
| import org.apache.spark.{SecurityManager, SparkConf} | ||
| import org.apache.spark.util.{AkkaUtils, Utils} | ||
| import org.apache.spark.util.{AkkaUtils, ChildFirstURLClassLoader, MutableURLClassLoader, Utils} | ||
|
|
||
| /** | ||
| * Utility object for launching driver programs such that they share fate with the Worker process. | ||
| */ | ||
| object DriverWrapper { | ||
| def main(args: Array[String]) { | ||
| args.toList match { | ||
| case workerUrl :: mainClass :: extraArgs => | ||
| case workerUrl :: userJar :: mainClass :: extraArgs => | ||
| val conf = new SparkConf() | ||
| val (actorSystem, _) = AkkaUtils.createActorSystem("Driver", | ||
| Utils.localHostName(), 0, conf, new SecurityManager(conf)) | ||
| actorSystem.actorOf(Props(classOf[WorkerWatcher], workerUrl), name = "workerWatcher") | ||
|
|
||
| val currentLoader = Thread.currentThread.getContextClassLoader | ||
| val userJarUrl = new File(userJar).toURI().toURL() | ||
| val loader = | ||
| if (sys.props.getOrElse("spark.driver.userClassPathFirst", "false").toBoolean) { | ||
| new ChildFirstURLClassLoader(Array(userJarUrl), currentLoader) | ||
| } else { | ||
| new MutableURLClassLoader(Array(userJarUrl), currentLoader) | ||
| } | ||
| Thread.currentThread.setContextClassLoader(loader) | ||
|
|
||
| // Delegate to supplied main class | ||
| val clazz = Class.forName(args(1)) | ||
| val clazz = Class.forName(mainClass, true, loader) | ||
| val mainMethod = clazz.getMethod("main", classOf[Array[String]]) | ||
| mainMethod.invoke(null, extraArgs.toArray[String]) | ||
|
|
||
| actorSystem.shutdown() | ||
|
|
||
| case _ => | ||
| System.err.println("Usage: DriverWrapper <workerUrl> <driverMainClass> [options]") | ||
| System.err.println("Usage: DriverWrapper <workerUrl> <userJar> <driverMainClass> [options]") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we're technically changing the public API here, which is probably fine since there's not really a way around it in this case. @pwendell thoughts?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this object supposed to be public in the first place? Seems like it's only used through SparkSubmit (much like Yarn's |
||
| System.exit(-1) | ||
| } | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: use parentheses instead of curly braces here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like both patterns are used in Spark. I kinda prefer the braces, makes it clearer (to me) that it's a closure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah I actually prefer the style here