[
https://issues.apache.org/jira/browse/MAPREDUCE-2880?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13107462#comment-13107462
]
Vinod Kumar Vavilapalli commented on MAPREDUCE-2880:
----------------------------------------------------
Did a first pass of review. Didn't find it as scary as it threatened to be :)
Mostly very minor comments:
ContainerLaunch:
- {{sanitizeEnv()}}: I was confused about LOGNAME with the usual LOG context.
Perhaps put a comment saying it refs to LOGINNAME.
- Rename setEnvIfUnset -> setEnvIfNotset ?
- Shouldn't we set YARN_HOME, HADOOP_HDFS_HOME, HADOOP_COMMON_HOME and
JAVA_HOME as part of a NM whitelist? A configuration or better a shell script
which we can source into the child shell. The later will help us add items on
demand.
Rename ApplicationConstants.APPLICATION_CLASSPATH to
{DEFAULT|PREFIX}_APPLICATION_CLASSPATH ?
MRJobConfig: Rename MAPRED_MAP_ADMIN_JAVA_OPTS ->
MAPRED_FRAMEWORK_MAP_JAVA_OPTS or something similar? Definitely doesn't look
like an admin config to me. Similarly for other params?
MRApps:
- The linkName for all dist-cache files and MR_APP_JAR removed from CLASSPATH
works because of the glob pattern?
- {{setMRFrameworkClasspath(Map<String, String>)}} can be private.
- _setEnvFromInputString()_: Refs to "tt" should be fixed.
Holistically looks good, but I'll just look once more afresh tomorrow.
Stating what is already known, we should test this thoroughly :) Mainly the
distributed cache related tests. TestMRJobs, which also has a dist-cache test
is a good start.
> Fix classpath construction for MRv2
> -----------------------------------
>
> Key: MAPREDUCE-2880
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-2880
> Project: Hadoop Map/Reduce
> Issue Type: Improvement
> Components: mrv2
> Affects Versions: 0.23.0
> Reporter: Luke Lu
> Assignee: Arun C Murthy
> Fix For: 0.23.0
>
> Attachments: MAPREDUCE-2880.patch, MAPREDUCE-2880.patch,
> MAPREDUCE-2880.patch
>
>
> MRConstants.java refers a hard-coded version of MR AM jar. The build config
> works around with a symlink. The deployment currently needs symlink
> workaround as well. We need to fix this so that we can actually launch
> arbitrary versions of AMs.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
