[
https://issues.apache.org/jira/browse/MAPREDUCE-7456?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17769815#comment-17769815
]
ASF GitHub Bot commented on MAPREDUCE-7456:
-------------------------------------------
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?
> Extend add-opens flag to container launch commands on JDK17 nodes
> -----------------------------------------------------------------
>
> Key: MAPREDUCE-7456
> URL: https://issues.apache.org/jira/browse/MAPREDUCE-7456
> Project: Hadoop Map/Reduce
> Issue Type: New Feature
> Affects Versions: 3.4.0
> Reporter: Peter Szucs
> Assignee: Peter Szucs
> Priority: Major
> Labels: pull-request-available
>
> There was a previous ticket for adding add-opens flag to container launch to
> be able to run them on JDK17 nodes:
> https://issues.apache.org/jira/browse/MAPREDUCE-7449
> As testing discovered, this should be extended with
> "{_}-add-exports=java.base/sun.net.dns=ALL-UNNAMED{_}" and
> "{_}-add-exports=java.base/sun.net.util=ALL-UNNAMED{_}" options to be able to
> run containers on Isilon.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
