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


##########
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:
   Had an offline discussion with @tomicooler, the issue with not using a 
placeholder is the way commands is contructed: in contrast to it's name and 
type it's a String list containing one element only, the command that is to be 
executed. In case of Java apps we must add this parameter before the main 
class, hence I see two options here: add the placeholder right after -Xmx (so 
that we know it'll be in the correct place) or I can deconstruct the first 
element of the commands array and add the add-opens flag to the correct place. 
I think the former is a cleaner, less error-prone solution.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


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

Reply via email to