-
Notifications
You must be signed in to change notification settings - Fork 29k
Spark 939 allow user jars to take precedence over spark jars #217
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
Closed
holdenk
wants to merge
33
commits into
apache:master
from
holdenk:spark-939-allow-user-jars-to-take-precedence-over-spark-jars
Closed
Changes from all commits
Commits
Show all changes
33 commits
Select commit
Hold shift + click to select a range
e1d9f71
One loader workers.
holdenk 648b559
Both class loaders compile. Now for testing
holdenk 8d2241e
Adda FakeClass for testing ClassLoader precedence options
holdenk 16aecd1
Doesn't quite work
holdenk 792d961
Almost works
holdenk 7ef4628
Clean up
holdenk 22d83cb
Add a test suite for the executor url class loader suite
holdenk 47046ff
Remove bad import'
holdenk dc4fe44
Does not depend on being in my home directory
holdenk 691ee00
It works ish
holdenk 9e2d236
It works comrade
holdenk 8a67302
Test are good
holdenk 4919bf9
Fix parent calling class loader issue
holdenk d90d217
Fix fall back with custom class loader and add a test for it
holdenk a343350
Update rat-excludes and remove a useless file
holdenk 241b03d
fix my rat excludes
holdenk bb8d179
Fix issues with the repl class loader
holdenk 125ea7f
Easier to just remove those files, we don't need them
holdenk a0ef85a
Fix style issues
holdenk 7752594
re-add https comment
holdenk aa95083
CR feedback
holdenk d4ae848
rewrite component into scala
holdenk 7a7bf5f
CR feedback
holdenk 261aaee
simplify executorurlclassloader a bit
holdenk 858aba2
Remove a bunch of test junk
holdenk 9f68f10
Start rewriting the ExecutorURLClassLoaderSuite to not use the hard c…
holdenk 204b199
Fix the generated classes
holdenk f0b7114
Fix the core/src/test/scala/org/apache/spark/executor/ExecutorURLClas…
holdenk 644719f
User the class generator for the repl class loader tests too
holdenk 7546549
CR feedback, merge some of the testutils methods down, rename the cla…
holdenk 8f89965
Fix tests for new class name
holdenk 1955232
Fix long line in TestUtils
holdenk cf0cac9
Fix the executorclassloader
holdenk File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -39,4 +39,4 @@ work | |
| .*\.q | ||
| golden | ||
| test.out/* | ||
| .*iml | ||
| .*iml | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -291,15 +291,19 @@ private[spark] class Executor( | |
| * Create a ClassLoader for use in tasks, adding any JARs specified by the user or any classes | ||
| * created by the interpreter to the search path | ||
| */ | ||
| private def createClassLoader(): ExecutorURLClassLoader = { | ||
| val loader = Thread.currentThread().getContextClassLoader | ||
| private def createClassLoader(): MutableURLClassLoader = { | ||
| val loader = this.getClass.getClassLoader | ||
|
|
||
| // For each of the jars in the jarSet, add them to the class loader. | ||
| // We assume each of the files has already been fetched. | ||
| val urls = currentJars.keySet.map { uri => | ||
| new File(uri.split("/").last).toURI.toURL | ||
| }.toArray | ||
| new ExecutorURLClassLoader(urls, loader) | ||
| val userClassPathFirst = conf.getBoolean("spark.classpath.userClassPathFirst", false) | ||
| userClassPathFirst match { | ||
| case true => new ChildExecutorURLClassLoader(urls, loader) | ||
| case false => new ExecutorURLClassLoader(urls, loader) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -310,11 +314,14 @@ 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.classpath.userClassPathFirst", false) | ||
|
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. Needs docs (note to self) |
||
| try { | ||
| val klass = Class.forName("org.apache.spark.repl.ExecutorClassLoader") | ||
| .asInstanceOf[Class[_ <: ClassLoader]] | ||
| val constructor = klass.getConstructor(classOf[String], classOf[ClassLoader]) | ||
| constructor.newInstance(classUri, parent) | ||
| val constructor = klass.getConstructor(classOf[String], classOf[ClassLoader], | ||
| classOf[Boolean]) | ||
| constructor.newInstance(classUri, parent, userClassPathFirst) | ||
| } catch { | ||
| case _: ClassNotFoundException => | ||
| logError("Could not find org.apache.spark.repl.ExecutorClassLoader on classpath!") | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
32 changes: 32 additions & 0 deletions
32
core/src/main/scala/org/apache/spark/util/ParentClassLoader.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.util | ||
|
|
||
| /** | ||
| * A class loader which makes findClass accesible to the child | ||
| */ | ||
| private[spark] class ParentClassLoader(parent: ClassLoader) extends ClassLoader(parent) { | ||
|
|
||
| override def findClass(name: String) = { | ||
| super.findClass(name) | ||
| } | ||
|
|
||
| override def loadClass(name: String): Class[_] = { | ||
| super.loadClass(name) | ||
| } | ||
| } |
67 changes: 67 additions & 0 deletions
67
core/src/test/scala/org/apache/spark/executor/ExecutorURLClassLoaderSuite.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.executor | ||
|
|
||
| import java.io.File | ||
| import java.net.URLClassLoader | ||
|
|
||
| import org.scalatest.FunSuite | ||
|
|
||
| import org.apache.spark.TestUtils | ||
|
|
||
| class ExecutorURLClassLoaderSuite extends FunSuite { | ||
|
|
||
| val childClassNames = List("FakeClass1", "FakeClass2") | ||
| val parentClassNames = List("FakeClass1", "FakeClass2", "FakeClass3") | ||
| val urls = List(TestUtils.createJarWithClasses(childClassNames, "1")).toArray | ||
| val urls2 = List(TestUtils.createJarWithClasses(parentClassNames, "2")).toArray | ||
|
|
||
| test("child first") { | ||
| val parentLoader = new URLClassLoader(urls2, null) | ||
| val classLoader = new ChildExecutorURLClassLoader(urls, parentLoader) | ||
| val fakeClass = classLoader.loadClass("FakeClass2").newInstance() | ||
| val fakeClassVersion = fakeClass.toString | ||
| assert(fakeClassVersion === "1") | ||
| } | ||
|
|
||
| test("parent first") { | ||
| val parentLoader = new URLClassLoader(urls2, null) | ||
| val classLoader = new ExecutorURLClassLoader(urls, parentLoader) | ||
| val fakeClass = classLoader.loadClass("FakeClass1").newInstance() | ||
| val fakeClassVersion = fakeClass.toString | ||
| assert(fakeClassVersion === "2") | ||
| } | ||
|
|
||
| test("child first can fall back") { | ||
| val parentLoader = new URLClassLoader(urls2, null) | ||
| val classLoader = new ChildExecutorURLClassLoader(urls, parentLoader) | ||
| val fakeClass = classLoader.loadClass("FakeClass3").newInstance() | ||
| val fakeClassVersion = fakeClass.toString | ||
| assert(fakeClassVersion === "2") | ||
| } | ||
|
|
||
| test("child first can fail") { | ||
| val parentLoader = new URLClassLoader(urls2, null) | ||
| val classLoader = new ChildExecutorURLClassLoader(urls, parentLoader) | ||
| intercept[java.lang.ClassNotFoundException] { | ||
| classLoader.loadClass("FakeClassDoesNotExist").newInstance() | ||
| } | ||
| } | ||
|
|
||
|
|
||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
76 changes: 76 additions & 0 deletions
76
repl/src/test/scala/org/apache/spark/repl/ExecutorClassLoaderSuite.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| /* | ||
| * Licensed to the Apache Software Foundation (ASF) under one or more | ||
| * contributor license agreements. See the NOTICE file distributed with | ||
| * this work for additional information regarding copyright ownership. | ||
| * The ASF licenses this file to You under the Apache License, Version 2.0 | ||
| * (the "License"); you may not use this file except in compliance with | ||
| * the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package org.apache.spark.repl | ||
|
|
||
| import java.io.File | ||
| import java.net.URLClassLoader | ||
|
|
||
| import org.scalatest.BeforeAndAfterAll | ||
| import org.scalatest.FunSuite | ||
|
|
||
| import com.google.common.io.Files | ||
|
|
||
| import org.apache.spark.TestUtils | ||
|
|
||
| class ExecutorClassLoaderSuite extends FunSuite with BeforeAndAfterAll { | ||
|
|
||
| val childClassNames = List("ReplFakeClass1", "ReplFakeClass2") | ||
| val parentClassNames = List("ReplFakeClass1", "ReplFakeClass2", "ReplFakeClass3") | ||
| val tempDir1 = Files.createTempDir() | ||
| val tempDir2 = Files.createTempDir() | ||
| val url1 = "file://" + tempDir1 | ||
| val urls2 = List(tempDir2.toURI.toURL).toArray | ||
|
|
||
| override def beforeAll() { | ||
| childClassNames.foreach(TestUtils.createCompiledClass(_, tempDir1, "1")) | ||
| parentClassNames.foreach(TestUtils.createCompiledClass(_, tempDir2, "2")) | ||
| } | ||
|
|
||
| test("child first") { | ||
| val parentLoader = new URLClassLoader(urls2, null) | ||
| val classLoader = new ExecutorClassLoader(url1, parentLoader, true) | ||
| val fakeClass = classLoader.loadClass("ReplFakeClass2").newInstance() | ||
| val fakeClassVersion = fakeClass.toString | ||
| assert(fakeClassVersion === "1") | ||
| } | ||
|
|
||
| test("parent first") { | ||
| val parentLoader = new URLClassLoader(urls2, null) | ||
| val classLoader = new ExecutorClassLoader(url1, parentLoader, false) | ||
| val fakeClass = classLoader.loadClass("ReplFakeClass1").newInstance() | ||
| val fakeClassVersion = fakeClass.toString | ||
| assert(fakeClassVersion === "2") | ||
| } | ||
|
|
||
| test("child first can fall back") { | ||
| val parentLoader = new URLClassLoader(urls2, null) | ||
| val classLoader = new ExecutorClassLoader(url1, parentLoader, true) | ||
| val fakeClass = classLoader.loadClass("ReplFakeClass3").newInstance() | ||
| val fakeClassVersion = fakeClass.toString | ||
| assert(fakeClassVersion === "2") | ||
| } | ||
|
|
||
| test("child first can fail") { | ||
| val parentLoader = new URLClassLoader(urls2, null) | ||
| val classLoader = new ExecutorClassLoader(url1, parentLoader, true) | ||
| intercept[java.lang.ClassNotFoundException] { | ||
| classLoader.loadClass("ReplFakeClassDoesNotExist").newInstance() | ||
| } | ||
| } | ||
|
|
||
| } |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This class will need to be made package private if it goes into
maininstead oftest. I can do this when merging, just putting a reminder here.