-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-6406] Launch Spark using assembly jar instead of a separate launcher jar #5085
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
Conversation
…extFiles The prefix "file:" is missing in the string inserted as key in HashMap
…onsistent with rest of Spark
…multiplier (redone to resolve merge conflicts)
…nravi Conflicts: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala
…nravi Conflicts: yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientArguments.scala yarn/common/src/main/scala/org/apache/spark/deploy/yarn/ClientBase.scala yarn/common/src/main/scala/org/apache/spark/deploy/yarn/YarnAllocator.scala
…n to check if maxResultSize > 0)
|
Addressed two of the three comments @vanzin. The third one is tricky, let's discuss tomorrow when you're around. |
…path as an argument
…nravi Conflicts: launcher/src/main/java/org/apache/spark/launcher/AbstractCommandBuilder.java
|
PR updated (instead of passing assembly jar on cmdline, we can find it dynamically using getLocation). |
bin/spark-class
Outdated
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.
You may need to add something like #4873 here.
|
Can an admin trigger jenkins for this PR please? @nishkamravi2 , I think you can also remove the line in make-distribution.sh that packages the launcher jar. Otherwise, some minor things, but looks good. Can you also change the PR title since it's not accurate? Something like "Launch Spark using the assembly instead of a separate launcher jar". |
|
retest this please |
|
Test build #29144 has finished for PR 5085 at commit
|
|
I can't comment on random code that you didn't change, but you'll need to change the code around this code in AbstractCommandBuilder: You can just skip the check for Hive and add all datanucleus jars if they're present. That's the same thing Sean did in the PR I mentioned before. (Bonus points for getting rid of the annoying message printed to stderr.) |
|
Addressed comments, fixed a couple of bugs. Can this be retested? Thanks. |
|
Jenkins, retest this please. |
|
Test build #29295 has finished for PR 5085 at commit
|
|
LGTM. |
|
Given @vanzin and @nishkamravi2 have thoroughly discussed and approved this, I'm inclined to merge it. The changes look sane and do simplify the launcher somewhat. |
|
@vanzin @nishkamravi2 When I try to run Is this a known issue? |
|
Looks like @davies filed a JIRA for this already: https://issues.apache.org/jira/browse/SPARK-6890 |
|
Hey @andrewor14, is this a Mac/Windows issue? I can't seem to be able to reproduce it on Linux. Also, do you happen to have the path that was returned by getLocation().getPath() ? |
|
Yes, the path was |
No description provided.