Skip to content

[SPARK-36599][CORE] Fix the http class server to work again #33849

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.

Sign up for GitHub

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
yellowflash wants to merge 1 commit into from
Closed

[SPARK-36599][CORE] Fix the http class server to work again #33849

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions repl/src/main/scala/org/apache/spark/repl/ExecutorClassLoader.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,8 @@ class ExecutorClassLoader(
parent: ClassLoader,
userClassPathFirst: Boolean) extends ClassLoader(null) with Logging {
val uri = new URI(classUri)
val directory = uri.getPath

val parentLoader = new ParentClassLoader(parent)

// Allows HTTP connect and read timeouts to be controlled for testing / debugging purposes
private[repl] var httpUrlConnectionTimeoutMillis: Int = -1
Copy link
Member

@srowen srowen Aug 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree this is unused


private val fetchFn: (String) => InputStream = uri.getScheme() match {
case "spark" => getClassFileInputStreamFromSparkRPC
case _ =>
Expand Down Expand Up @@ -160,7 +155,7 @@ class ExecutorClassLoader(

private def getClassFileInputStreamFromFileSystem(fileSystem: FileSystem)(
pathInDirectory: String): InputStream = {
val path = new Path(directory, pathInDirectory)
val path = new Path(new Path(uri), pathInDirectory)
Copy link
Member

@srowen srowen Aug 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain a little more what inputs fail without this change?

Copy link
Author

@yellowflash yellowflash Aug 30, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hadoop Http filesystem require the paths to be fully qualified URLs.
It does path.toUri.toUrl which fails in our case because the Path is not fully qualified.
So the class loader doesn't work if the class uri is http://..../ I raised a PR on hadoop too,

apache/hadoop#3338
But this is a regression on spark, as it used to work with a very specific implementation for http based class servers and now since it uses the Filesystem api for everything and it doesn't work anymore.

try {
fileSystem.open(path)
} catch {
Expand Down