[jira] [Commented] (YARN-316) YARN container launch may exceed maximum Windows command line length due to long classpath
[
https://issues.apache.org/jira/browse/YARN-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13561873#comment-13561873
]
Chris Nauroth commented on YARN-316:
Alejandro and Bikas, thanks for the feedback. I'm working on a new patch, but
here are a few quick replies first:
{quote}
On the 1st, agree, and i don't think the perf hit will be noticeable, the most
a a few of millisecs.
{quote}
The next version of the patch will bundle the classpath into a temp jar on all
platforms, not just Windows.
{quote}
If its not too much work, could you try using
http://commons.apache.org/lang/api-release/org/apache/commons/lang3/text/StrSubstitutor.html
instead of writing a custom string substitutor. Dont bother if its going to be
a lot of effort.
{quote}
I had looked at StrSubstitutor earlier, but there were some things that make it
awkward to use for this logic.
If the variable is undefined, then the StrSubstitutor will leave the variable
name in place instead of the more traditional shell behavior of replacing it
with empty string. For example, consider $FOO_$BAR_$BAZ and an environment
consisting of FOO=one and BAZ=two (BAR is undefined). StrSubstitutor returns
one_$BAR_two instead of one__two, which is what we expect from shell. To
work around this, we'd need to wrap the environment map in a custom map that
returns default values (i.e. Guava MapMaker) or subclass StrLookup:
http://commons.apache.org/lang/api-2.5/org/apache/commons/lang/text/StrLookup.html.
The other problem is that StrSubstitutor works best for matching variable names
that have a static prefix and suffix. This works great for Windows
(%VAR%/foo), but now that we're going to do the same thing for non-Windows,
we also need to handle shell variable names ($VAR/foo). We need to parse $,
followed by multiple legal variable name characters, terminated by any
non-legal variable name character. That can't be expressed with a static
suffix, but it's easily expressed with a regex. Another alternative is to
subclass StrMatcher:
http://commons.apache.org/lang/api-2.5/org/apache/commons/lang/text/StrMatcher.html.
It's definitely possible to make StrSubstitutor behave the way we need, but all
things considered, it would probably take at least double the code compared to
{{StringUtils#replaceTokens}}. I'm not planning on switching to StrSubstitutor
in the next patch, but if you disagree, please let me know.
{quote}
What prompted this change in MiniYarnCluster?
{quote}
I forgot to mention this part. At this point in the code, it's trying to
create a directory at a deeply nested path, and the parent path doesn't exist
yet. mkdir() was returning false. This wasn't causing test failures on Linux,
because the directory was still getting created later during container
initialization. However, it is a problem on Windows with the temp test
directory symlink, because winutils symlink currently requires that the target
already exists. (See HADOOP-9043.) I switched this to mkdirs() so that it
would recursively create the full path.
{quote}
Do we still need to use SimpleNames after using symlink?
{quote}
Yes, unfortunately, the symlink alone isn't sufficient. Here is an example of
the kind of test working directory it was using before my patch (390
characters):
C:/hdc/hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-jobclient/target/org.apache.hadoop.mapred.ClusterMapReduceTestCaseConfigurableMiniMRCluster/org.apache.hadoop.mapred.ClusterMapReduceTestCaseConfigurableMiniMRCluster-localDir-nm-1_1/usercache/cnauroth/appcache/application_1358229151479_0001/container_1358229151479_0001_01_01/default_container_executor.cmd
Using the temp symlink, that turns into this path (270 characters, still over
the limit of 260):
C:\Users\cnauroth\AppData\Local\Temp\1358803955776\org.apache.hadoop.mapred.ClusterMapReduceTestCaseConfigurableMiniMRCluster-localDir-nm-1_1\usercache\cnauroth\appcache\application_1358229151479_0001\container_1358229151479_0001_01_01\default_container_executor.cmd
Then, with the switch to simple class name, it fits (244 characters, bringing
us under the limit of 260):
C:\Users\cnauroth\AppData\Local\Temp\1358803955776\ClusterMapReduceTestCaseConfigurableMiniMRCluster-localDir-nm-1_1\usercache\cnauroth\appcache\application_1358229151479_0001\container_1358229151479_0001_01_01\default_container_executor.cmd
{quote}
Is there a JIRA to make the env substitution work for branch-1-win when
creating the classpath manifest? What about * expansion?
{quote}
Thank you for the reminder. I just filed MAPREDUCE-4959 to backport this logic
to branch-1-win. In my next version of this patch, I'm also going to try to
refactor more of the logic currently in {{ContainerLaunch}} to
{{FileUtil#createJarWithClassPath}}. I expect that will make the code easier
to backport to branch-1-win,
[jira] [Commented] (YARN-316) YARN container launch may exceed maximum Windows command line length due to long classpath
[ https://issues.apache.org/jira/browse/YARN-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13561891#comment-13561891 ] Bikas Saha commented on YARN-316: - I am in favor of keeping a windows specific path for classpath jar now and definitely open a jira to merge the code paths later on. I would be wary of destabilizing the linux path while edge cases and other bugs gets ironed out on windows. The perf needs to be measured. The branch-1-win jira should have some discussion on perf tests conducted to measure the difference. We might need to redo them for trunk because the code is different and perf results might be different. Please ignore strsubstitutor. Doesnt seem worth the effort. So even the simple class name and symlink are just about working for the current set of test case names. We might need another fix for this down the road. YARN container launch may exceed maximum Windows command line length due to long classpath -- Key: YARN-316 URL: https://issues.apache.org/jira/browse/YARN-316 Project: Hadoop YARN Issue Type: Bug Components: nodemanager Affects Versions: 3.0.0, trunk-win Reporter: Chris Nauroth Assignee: Chris Nauroth Attachments: YARN-316-branch-trunk-win.1.patch, YARN-316-branch-trunk-win.2.patch On Windows, a command line longer than 8192 characters will fail. This can cause YARN container launch to fail on Windows if the classpath argument exceeds this limit. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (YARN-316) YARN container launch may exceed maximum Windows command line length due to long classpath
[
https://issues.apache.org/jira/browse/YARN-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13561911#comment-13561911
]
Chris Nauroth commented on YARN-316:
{quote}
I am in favor of keeping a windows specific path for classpath jar now and
definitely open a jira to merge the code paths later on.
{quote}
Yes, that does sound like the safer approach. I have filed YARN-358 for this.
{quote}
So even the simple class name and symlink are just about working for the
current set of test case names. We might need another fix for this down the
road.
{quote}
This is true. Do you think it would be valuable to add a check in
{{DefaultContainerExecutor#launchContainer}} to see if the path to the
container launch script is 260 characters, and if so, fail fast with a
descriptive error message? This could be helpful even for the main product
code, in case someone deploys on Windows with a path configured in
yarn.nodemanager.local-dirs that is too long. I'll see if I can incorporate
this in my next version of this patch.
YARN container launch may exceed maximum Windows command line length due to
long classpath
--
Key: YARN-316
URL: https://issues.apache.org/jira/browse/YARN-316
Project: Hadoop YARN
Issue Type: Bug
Components: nodemanager
Affects Versions: 3.0.0, trunk-win
Reporter: Chris Nauroth
Assignee: Chris Nauroth
Attachments: YARN-316-branch-trunk-win.1.patch,
YARN-316-branch-trunk-win.2.patch
On Windows, a command line longer than 8192 characters will fail. This can
cause YARN container launch to fail on Windows if the classpath argument
exceeds this limit.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (YARN-316) YARN container launch may exceed maximum Windows command line length due to long classpath
[ https://issues.apache.org/jira/browse/YARN-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13560854#comment-13560854 ] Alejandro Abdelnur commented on YARN-316: - Chris, do you see any reason why the Class-Path manifest approach could not used for all OSes? Regarding the extra leading '0' removal in setPermissions, just make sure this leading zero is not used to force an octal interpretation of the mask. YARN container launch may exceed maximum Windows command line length due to long classpath -- Key: YARN-316 URL: https://issues.apache.org/jira/browse/YARN-316 Project: Hadoop YARN Issue Type: Bug Components: nodemanager Affects Versions: 3.0.0, trunk-win Reporter: Chris Nauroth Assignee: Chris Nauroth Attachments: YARN-316-branch-trunk-win.1.patch, YARN-316-branch-trunk-win.2.patch On Windows, a command line longer than 8192 characters will fail. This can cause YARN container launch to fail on Windows if the classpath argument exceeds this limit. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (YARN-316) YARN container launch may exceed maximum Windows command line length due to long classpath
[
https://issues.apache.org/jira/browse/YARN-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13560905#comment-13560905
]
Chris Nauroth commented on YARN-316:
{quote}
Chris, do you see any reason why the Class-Path manifest approach could not
used for all OSes?
{quote}
I initially tested the Class-Path manifest change on Mac without the {{if
(Shell.WINDOWS)}} guard, so this approach definitely can work across platforms.
Comments in HADOOP-8899, which was a similar change for branch-1-win
MapReduce, give the rationale for limiting the change to Windows: platforms
that don't have the command line length limitation don't need to suffer the
small performance hit of creating the extra jar.
It's a very small performance hit though, and it's a one-time initialization
cost, so long-running jobs will notice it less than short jobs. If we'd prefer
to keep the approach consistent, then we can do that. What do you think?
{quote}
Regarding the extra leading '0' removal in setPermissions, just make sure this
leading zero is not used to force an octal interpretation of the mask.
{quote}
I think we're OK on this. The 'o' in the format string (not the length of the
zero-padding) gives us octal formatting of the string prior to calling chmod or
winutils. Neither chmod nor winutils need a leading zero on the input. They
always interpret the input as an octal mode. I ran the full test suite after
this change, and it didn't cause any test failures.
YARN container launch may exceed maximum Windows command line length due to
long classpath
--
Key: YARN-316
URL: https://issues.apache.org/jira/browse/YARN-316
Project: Hadoop YARN
Issue Type: Bug
Components: nodemanager
Affects Versions: 3.0.0, trunk-win
Reporter: Chris Nauroth
Assignee: Chris Nauroth
Attachments: YARN-316-branch-trunk-win.1.patch,
YARN-316-branch-trunk-win.2.patch
On Windows, a command line longer than 8192 characters will fail. This can
cause YARN container launch to fail on Windows if the classpath argument
exceeds this limit.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (YARN-316) YARN container launch may exceed maximum Windows command line length due to long classpath
[ https://issues.apache.org/jira/browse/YARN-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13560911#comment-13560911 ] Alejandro Abdelnur commented on YARN-316: - On the 1st, agree, and i don't think the perf hit will be noticeable, the most a a few of millisecs. On the 2nd, thx for checking. YARN container launch may exceed maximum Windows command line length due to long classpath -- Key: YARN-316 URL: https://issues.apache.org/jira/browse/YARN-316 Project: Hadoop YARN Issue Type: Bug Components: nodemanager Affects Versions: 3.0.0, trunk-win Reporter: Chris Nauroth Assignee: Chris Nauroth Attachments: YARN-316-branch-trunk-win.1.patch, YARN-316-branch-trunk-win.2.patch On Windows, a command line longer than 8192 characters will fail. This can cause YARN container launch to fail on Windows if the classpath argument exceeds this limit. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (YARN-316) YARN container launch may exceed maximum Windows command line length due to long classpath
[
https://issues.apache.org/jira/browse/YARN-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13561269#comment-13561269
]
Bikas Saha commented on YARN-316:
-
If its not too much work, could you try using
http://commons.apache.org/lang/api-release/org/apache/commons/lang3/text/StrSubstitutor.html
instead of writing a custom string substitutor. Dont bother if its going to be
a lot of effort.
What prompted this change in MiniYarnCluster?
{code}
+ - + dirType + Dir-nm- + index + _ + i);
-dirs[i].mkdir();
+dirs[i].mkdirs();
{code}
Do we still need to use SimpleNames after using symlink?
Is there a JIRA to make the env substitution work for branch-1-win when
creating the classpath manifest? What about * expansion?
YARN container launch may exceed maximum Windows command line length due to
long classpath
--
Key: YARN-316
URL: https://issues.apache.org/jira/browse/YARN-316
Project: Hadoop YARN
Issue Type: Bug
Components: nodemanager
Affects Versions: 3.0.0, trunk-win
Reporter: Chris Nauroth
Assignee: Chris Nauroth
Attachments: YARN-316-branch-trunk-win.1.patch,
YARN-316-branch-trunk-win.2.patch
On Windows, a command line longer than 8192 characters will fail. This can
cause YARN container launch to fail on Windows if the classpath argument
exceeds this limit.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (YARN-316) YARN container launch may exceed maximum Windows command line length due to long classpath
[ https://issues.apache.org/jira/browse/YARN-316?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=13544123#comment-13544123 ] Chris Nauroth commented on YARN-316: To resolve this, we need to port the branch-1-win MapReduce change shown in HADOOP-8899 to YARN. YARN container launch may exceed maximum Windows command line length due to long classpath -- Key: YARN-316 URL: https://issues.apache.org/jira/browse/YARN-316 Project: Hadoop YARN Issue Type: Bug Components: nodemanager Affects Versions: 3.0.0, trunk-win Reporter: Chris Nauroth Assignee: Chris Nauroth On Windows, a command line longer than 8192 characters will fail. This can cause YARN container launch to fail on Windows if the classpath argument exceeds this limit. -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
