Skip to content

[SPARK-1089] fix the regression problem on ADD_JARS in 0.9#13

Closed
CodingCat wants to merge 4 commits into
apache:masterfrom
CodingCat:SPARK-1089
Closed

[SPARK-1089] fix the regression problem on ADD_JARS in 0.9#13
CodingCat wants to merge 4 commits into
apache:masterfrom
CodingCat:SPARK-1089

Conversation

@CodingCat

Copy link
Copy Markdown
Contributor

https://spark-project.atlassian.net/browse/SPARK-1089

copied from JIRA, reported by @ash211

"Using the ADD_JARS environment variable with spark-shell used to add the jar to both the shell and the various workers. Now it only adds to the workers and importing a custom class in the shell is broken.
The workaround is to add custom jars to both ADD_JARS and SPARK_CLASSPATH.
We should fix ADD_JARS so it works properly again.
See various threads on the user list:
https://mail-archives.apache.org/mod_mbox/incubator-spark-user/201402.mbox/%3CCAJbo4neMLiTrnm1XbyqomWmp0m+EUcg4yE-txuRGSVKOb5KLeA@mail.gmail.com%3E
(another one that doesn't appear in the archives yet titled "ADD_JARS not working on 0.9")"

The reason of this bug is two-folds

in the current implementation of SparkILoop.scala, the settings.classpath is not set properly when the process() method is invoked

the weird behaviour of Scala 2.10, (I personally thought it is a bug)

if we simply set value of a PathSettings object (like settings.classpath), the isDefault is not set to true (this is a flag showing if the variable is modified), so it makes the PathResolver loads the default CLASSPATH environment variable value to calculated the path (see https://github.com/scala/scala/blob/2.10.x/src/compiler/scala/tools/util/PathResolver.scala#L215)

what we have to do is to manually make this flag set, (https://github.com/CodingCat/incubator-spark/blob/e3991d97ddc33e77645e4559b13bf78b9e68239a/repl/src/main/scala/org/apache/spark/repl/SparkILoop.scala#L884)

@AmplabJenkins

Copy link
Copy Markdown

Can one of the admins verify this patch?

1 similar comment
@AmplabJenkins

Copy link
Copy Markdown

Can one of the admins verify this patch?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I realize you didn't write this but mind cleaning up this code?

if (addedClasspath != "") settings.classpath.append(addedClasspath)

@pwendell

Copy link
Copy Markdown
Contributor

Thanks I've merged this into master and 0.9. Actually didn't notice tests hadn't gone through. We can revert if there are any issues. Jenkins, test this please.

@AmplabJenkins

Copy link
Copy Markdown

Merged build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Merged build started.

@AmplabJenkins

Copy link
Copy Markdown

Merged build triggered.

@AmplabJenkins

Copy link
Copy Markdown

Merged build finished.

@AmplabJenkins

Copy link
Copy Markdown

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/12909/

@asfgit asfgit closed this in 345df5f Feb 27, 2014
@CodingCat CodingCat deleted the SPARK-1089 branch February 27, 2014 18:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants