You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-issues@hadoop.apache.org by "szilard-nemeth (via GitHub)" <gi...@apache.org> on 2023/09/27 21:16:46 UTC
[GitHub] [hadoop] szilard-nemeth commented on a diff in pull request #6118: MAPREDUCE-7456. Extend add-opens flag to container launch commands on JDK17 nodes
szilard-nemeth commented on code in PR #6118:
URL: https://github.com/apache/hadoop/pull/6118#discussion_r1339225210
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml:
##########
@@ -1478,6 +1478,17 @@
<value>-Xmx256m</value>
</property>
+ <property>
+ <name>yarn.nodemanager.container-localizer.java.opts.add-exports-as-default</name>
+ <value>true</value>
+ <description>Since on JDK17 it's no longer possible to use the reflection API to
+ access public fields and methods add-exports flags should be added to container
Review Comment:
Nit: comma should be added after: "field and methods"
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml:
##########
@@ -1478,6 +1478,17 @@
<value>-Xmx256m</value>
</property>
+ <property>
+ <name>yarn.nodemanager.container-localizer.java.opts.add-exports-as-default</name>
+ <value>true</value>
+ <description>Since on JDK17 it's no longer possible to use the reflection API to
Review Comment:
Nit: It's --> It is (more formal)
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-api/src/main/java/org/apache/hadoop/yarn/conf/YarnConfiguration.java:
##########
@@ -2228,6 +2228,14 @@ public static boolean isAclEnabled(Configuration conf) {
public static final String NM_CONTAINER_LOCALIZER_JAVA_OPTS_DEFAULT =
"-Xmx256m";
+ /*
+ * Flag to indicate whether JDK17's required add-exports flags should be added to
+ * container localizers regardless of the user specified java opts.
Review Comment:
Nit: JAVA_OPTS?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ContainerLocalizer.java:
##########
@@ -102,6 +102,9 @@ public class ContainerLocalizer {
private static final FsPermission USERCACHE_FOLDER_PERMS =
new FsPermission((short) 0755);
public static final String CSI_VOLIUME_MOUNTS_ROOT = "csivolumes";
+ private static final String ADDITIONAL_JDK17_PLUS_OPTIONS =
Review Comment:
I can't see "--add-opens=java.base/java.lang=ALL-UNNAMED " here whereas I can see it in ContainerLaunch.java?
Can you explain with a code comment why is this difference?
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/main/resources/yarn-default.xml:
##########
@@ -1478,6 +1478,17 @@
<value>-Xmx256m</value>
</property>
+ <property>
+ <name>yarn.nodemanager.container-localizer.java.opts.add-exports-as-default</name>
+ <value>true</value>
+ <description>Since on JDK17 it's no longer possible to use the reflection API to
+ access public fields and methods add-exports flags should be added to container
+ localizers regardless of the user specified java opts. Setting this to true will
Review Comment:
Nit : JAVA_OPTS
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-server/hadoop-yarn-server-nodemanager/src/main/java/org/apache/hadoop/yarn/server/nodemanager/containermanager/localizer/ContainerLocalizer.java:
##########
@@ -400,6 +403,13 @@ private LocalizerStatus createStatus() throws InterruptedException {
public static List<String> getJavaOpts(Configuration conf) {
String opts = conf.get(YarnConfiguration.NM_CONTAINER_LOCALIZER_JAVA_OPTS_KEY,
YarnConfiguration.NM_CONTAINER_LOCALIZER_JAVA_OPTS_DEFAULT);
+ boolean isExtraJDK17OptionsConfigured =
+ conf.getBoolean(YarnConfiguration.NM_CONTAINER_LOCALIZER_JAVA_OPTS_ADD_EXPORTS_KEY,
+ YarnConfiguration.NM_CONTAINER_LOCALIZER_JAVA_OPTS_ADD_EXPORTS_DEFAULT);
+
Review Comment:
Can you add a testcase for ContainerLocalizer as well?
--
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-unsubscribe@hadoop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org