[ 
https://issues.apache.org/jira/browse/MAPREDUCE-7449?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17752652#comment-17752652
 ] 

ASF GitHub Bot commented on MAPREDUCE-7449:
-------------------------------------------

tomicooler commented on code in PR #5935:
URL: https://github.com/apache/hadoop/pull/5935#discussion_r1289663461


##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/launcher/ContainerLaunch.java:
##########
@@ -170,6 +170,13 @@ public static String expandEnvironment(String var,
     var = var.replace(ApplicationConstants.CLASS_PATH_SEPARATOR,
       File.pathSeparator);
 
+    if (Shell.isJavaVersionAtLeast(17)) {

Review Comment:
   Just wondering about this. Wouldn't a node-manager config be easer to reason 
about?
   
   Implementing like this is a bit strange, currently MR jobs can be configured 
(default=true) and the distributed shell app sets this regardless of the 
configuration. During the job submission/config time it's not clear if 
container will be launched on a Java>=17 node so that's the reason for the 
placeholder, later ContainerLaunch replaces it to the arg or an empty string. 
Maybe non MR apps would also require this option to run properly - which they 
could specify at the job config - but non-homogenous nodes (where nodes have 
different Java installs) can't be handled easily (maybe with node labels or 
some other trick).
   
   I think this should be a node-manager config instead. The ContainerLaunch 
could just simply add the arg when the java version is at least 17 and the 
config option is set.





> Add add-opens flag to container launch commands on JDK17 nodes
> --------------------------------------------------------------
>
>                 Key: MAPREDUCE-7449
>                 URL: https://issues.apache.org/jira/browse/MAPREDUCE-7449
>             Project: Hadoop Map/Reduce
>          Issue Type: New Feature
>          Components: mr-am, mrv2
>    Affects Versions: 3.4.0
>            Reporter: Benjamin Teke
>            Assignee: Benjamin Teke
>            Priority: Major
>              Labels: pull-request-available
>
> To allow containers to launch on JDK17 nodes the add-opens flag should be 
> added to the container launch commands if the node has JDK17+, and it 
> shouldn't on previous JDKs. This behaviour should be configurable.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: mapreduce-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: mapreduce-issues-h...@hadoop.apache.org

Reply via email to