-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
YARN-11386. Fix issue with classpath resolution #5183
YARN-11386. Fix issue with classpath resolution #5183
Conversation
Here are the classpath resolutions before and after applying the fix (the current PR) - |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
@jbrennan333 could you please review this PR? This is related to one of your commits - YARN-10664. |
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.
Good catch! This change looks good to me.
* This PR ensures that all the special notations such as <CPS> are resolved before getting added to classpath.
Description of PR
While launching the AM, the client puts the classpath into environment HashMap of the ContainerLaunchContext. To support cross-platform compatibility, some special notations must be used by the client. For example,
<CPS>
must be used as the classpath separator instead of ";" (Windows) or ":" (Linux). The NM would resolve all the<CPS>
using the appropriate path separator according to the OS platform, prior to launching the container. In addition to this, the NM is also expected to expand and resolve all of the wildcard ( * ) occurrences in the classpath before launching the container, since the JVM doesn't understand the wildcard charater.The issue we see here is, neither
<CPS>
resolution nor wildcard expansion is happening.I tested this from Spark. To run a Spark application on YARN, one needs to set the spark.yarn.jars config to point to the location where Spark jars are present. NM would then add these jars to the classpath before launching the Spark AM container. The resolution didn't happen -
Note that
%3CCPS%3E
in the above code block should've been replaced by ";" or ":" as part of classpath resolution. Since this didn't happen, NM failed to launch the Spark AM container -How was this patch tested?
I tested locally by submitting a Spark job and ensured that it ran successfully.
For code changes:
LICENSE
,LICENSE-binary
,NOTICE-binary
files?