-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-6048] SparkConf should not translate deprecated configs on set #4799
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
94b4dfa
fef6c9c
c26a9e3
a369cb1
10e77b5
11c525b
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 |
|---|---|---|
|
|
@@ -92,6 +92,12 @@ private[spark] class Executor( | |
| private val executorActor = env.actorSystem.actorOf( | ||
| Props(new ExecutorActor(executorId)), "ExecutorActor") | ||
|
|
||
| // Whether to load classes in user jars before those in Spark jars | ||
| private val userClassPathFirst: Boolean = { | ||
| conf.getBoolean("spark.executor.userClassPathFirst", | ||
| conf.getBoolean("spark.files.userClassPathFirst", false)) | ||
| } | ||
|
|
||
| // Create our ClassLoader | ||
| // do this after SparkEnv creation so can access the SecurityManager | ||
| private val urlClassLoader = createClassLoader() | ||
|
|
@@ -309,7 +315,7 @@ private[spark] class Executor( | |
| val urls = userClassPath.toArray ++ currentJars.keySet.map { uri => | ||
| new File(uri.split("/").last).toURI.toURL | ||
| } | ||
| if (conf.getBoolean("spark.executor.userClassPathFirst", false)) { | ||
| if (userClassPathFirst) { | ||
| new ChildFirstURLClassLoader(urls, currentLoader) | ||
| } else { | ||
| new MutableURLClassLoader(urls, currentLoader) | ||
|
|
@@ -324,14 +330,13 @@ private[spark] class Executor( | |
| val classUri = conf.get("spark.repl.class.uri", null) | ||
| if (classUri != null) { | ||
| logInfo("Using REPL class URI: " + classUri) | ||
| val userClassPathFirst: java.lang.Boolean = | ||
| conf.getBoolean("spark.executor.userClassPathFirst", false) | ||
| try { | ||
| val _userClassPathFirst: java.lang.Boolean = userClassPathFirst | ||
|
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. Do you need this? Feels like just referencing
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. Maybe not, but I'm just preserving what was already there before. This is later used in some reflection code so I think it would be safest to leave it as is. I'll feel better about removing it after the release. |
||
| val klass = Class.forName("org.apache.spark.repl.ExecutorClassLoader") | ||
| .asInstanceOf[Class[_ <: ClassLoader]] | ||
| val constructor = klass.getConstructor(classOf[SparkConf], classOf[String], | ||
| classOf[ClassLoader], classOf[Boolean]) | ||
| constructor.newInstance(conf, classUri, parent, userClassPathFirst) | ||
| constructor.newInstance(conf, classUri, parent, _userClassPathFirst) | ||
| } catch { | ||
| case _: ClassNotFoundException => | ||
| logError("Could not find org.apache.spark.repl.ExecutorClassLoader on classpath!") | ||
|
|
||
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.
Since you're doing this only once during the app's lifetime (this method is only called from SparkContext AFAICT), you could simplify the code that tracks whether warns have been printed in
DeprecatedConfig. But ok to not do it if you want to limit the scope of the change.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 let's revamp this whole thing after the release