[GitHub] [hadoop] brumi1024 commented on a diff in pull request #5935: MAPREDUCE-7449: Add add-opens flag to container launch commands on JDK17 nodes
brumi1024 commented on code in PR #5935:
URL: https://github.com/apache/hadoop/pull/5935#discussion_r1290018339
##
hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-app/src/test/java/org/apache/hadoop/mapreduce/v2/app/job/impl/TestMapReduceChildJVM.java:
##
@@ -163,19 +164,19 @@ public void testCommandLineWithLog4JConifg() throws
Exception {
Assert.assertEquals(
"[" + MRApps.crossPlatformify("JAVA_HOME") + "/bin/java" +
- " -Djava.net.preferIPv4Stack=true" +
- " -Dhadoop.metrics.log.level=WARN " +
- " -Xmx820m -Djava.io.tmpdir=" + MRApps.crossPlatformify("PWD") + "/tmp"
+
- " -Dlog4j.configuration=" + testLogPropertieFile +
- " -Dyarn.app.container.log.dir=" +
- " -Dyarn.app.container.log.filesize=0" +
- " -Dhadoop.root.logger=INFO,CLA -Dhadoop.root.logfile=syslog" +
- " org.apache.hadoop.mapred.YarnChild 127.0.0.1" +
- " 54321" +
- " attempt_0__m_00_0" +
- " 0" +
- " 1>/stdout" +
- " 2>/stderr ]", app.launchCmdList.get(0));
+" -Djava.net.preferIPv4Stack=true" +
Review Comment:
Fixed, thanks.
--
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
[GitHub] [hadoop] brumi1024 commented on a diff in pull request #5935: MAPREDUCE-7449: Add add-opens flag to container launch commands on JDK17 nodes
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
