[GitHub] [tez] jteagles commented on a change in pull request #116: TEZ-4299: Default java opts cause jdk11 to fail

[GitBox <gi...@apache.org>]

jteagles commented on a change in pull request #116:
URL: https://github.com/apache/tez/pull/116#discussion_r606267583



##########
File path: tez-api/src/main/java/org/apache/tez/client/TezClientUtils.java
##########
@@ -820,6 +820,23 @@ public static String addDefaultsToTaskLaunchCmdOpts(String vOpts, Configuration
     return vOpts;
   }
 
+  private static String getDefaultAmLaunchOpts() {
+    return getJavaVersion() >= 9 ? TezConfiguration.TEZ_AM_LAUNCH_CMD_OPTS_JDK9_DEFAULT
+      : TezConfiguration.TEZ_AM_LAUNCH_CMD_OPTS_DEFAULT;
+  }
+
+  private static String getDefaultTaskLaunchOpts() {
+    return getJavaVersion() >= 9 ? TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS_JDK9_DEFAULT
+      : TezConfiguration.TEZ_TASK_LAUNCH_CMD_OPTS_DEFAULT;
+  }
+
+  private static int getJavaVersion() {
+    String javaVersionString = System.getProperty("java.version");

Review comment:
       I originally detected the presence of java.lang.runtime.Version class to avoid parsing the version string. Knowing the actual version could be more useful in the future though so let's try to keep this for now.

##########
File path: tez-api/src/main/java/org/apache/tez/dag/api/TezConfiguration.java
##########
@@ -345,6 +345,8 @@ public TezConfiguration(boolean loadDefaults) {
   public static final String TEZ_AM_LAUNCH_CMD_OPTS = TEZ_AM_PREFIX +  "launch.cmd-opts";
   public static final String TEZ_AM_LAUNCH_CMD_OPTS_DEFAULT =
       "-XX:+PrintGCDetails -verbose:gc -XX:+PrintGCTimeStamps -XX:+UseNUMA -XX:+UseParallelGC";
+  public static final String TEZ_AM_LAUNCH_CMD_OPTS_JDK9_DEFAULT =

Review comment:
       Could we create TEZ_AM_LAUNCH_CMD_OPTS_JDK8_DEFAULT with the origin value of TEZ_AM_LAUNCH_CMD_OPTS_DEFAULT.
   Then we could statically assign TEZ_AM_LAUNCH_CMD_OPTS_DEFAULT to correct value of JDK8 or JDK9+. This would remove the runtime detection.




-- 
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.

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