[GitHub] [incubator-celeborn] pan3793 commented on a diff in pull request #1161: [CELEBORN-171] Support JDK11.

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

pan3793 commented on code in PR #1161:
URL: https://github.com/apache/incubator-celeborn/pull/1161#discussion_r1068355067


##########
sbin/start-worker.sh:
##########
@@ -32,10 +35,15 @@ if [ "$CELEBORN_WORKER_OFFHEAP_MEMORY" = "" ]; then
   CELEBORN_WORKER_OFFHEAP_MEMORY="1g"
 fi
 
-export CELEBORN_JAVA_OPTS="-Xmx$CELEBORN_WORKER_MEMORY -XX:MaxDirectMemorySize=$CELEBORN_WORKER_OFFHEAP_MEMORY $CELEBORN_WORKER_JAVA_OPTS"
+JAVA_VER=$(java -version 2>&1 | head -1 | cut -d'"' -f2 | sed '/^1\./s///' | cut -d'.' -f1)
+if [ "$JAVA_VER" -gt "8" ]; then
+  export CELEBORN_JAVA_OPTS="-Xmx$CELEBORN_WORKER_MEMORY -XX:MaxDirectMemorySize=$CELEBORN_WORKER_OFFHEAP_MEMORY $CELEBORN_WORKER_JAVA_OPTS --add-opens java.base/jdk.internal.misc=ALL-UNNAMED --illegal-access=warn -Dio.netty.tryReflectionSetAccessible=true"
+else
+  export CELEBORN_JAVA_OPTS="-Xmx$CELEBORN_WORKER_MEMORY -XX:MaxDirectMemorySize=$CELEBORN_WORKER_OFFHEAP_MEMORY $CELEBORN_WORKER_JAVA_OPTS"
+fi

Review Comment:
   ```suggestion
   export CELEBORN_JAVA_OPTS="-Xmx$CELEBORN_WORKER_MEMORY -XX:MaxDirectMemorySize=$CELEBORN_WORKER_OFFHEAP_MEMORY $CELEBORN_WORKER_JAVA_OPTS"
   JAVA_VERSION=$(java -version 2>&1 | grep " version " | head -1 | awk '{print $3}' | tr -d '"')
   if [[ ! "$JAVA_VERSION" == 1.8.* ]]; then
     export CELEBORN_JAVA_OPTS="${CELEBORN_JAVA_OPTS} --add-opens java.base/jdk.internal.misc=ALL-UNNAMED --illegal-access=warn -Dio.netty.tryReflectionSetAccessible=true"
   fi
   ```
   The above suggestion is based on:
   
   When I set `JAVA_HOME=/path/jdk17` and run `java -version`, the first line of output is not the java version, so `grep " version "` is added, and I try my best to avoid using regex to make each shell command as short and simple enough.
   ```
   ➜  ~ java -version
   [0.000s][warning][os,container] Duplicate cpuset controllers detected. Picking /sys/fs/cgroup/cpuset, skipping /mnt/wsl/docker-desktop-bind-mounts/Ubuntu/85bae2ead9d613eb16150e89460f06daea72d31beaa1c37c68457a29a0fdba68/cpuset.
   java version "17.0.4" 2022-07-19 LTS
   Java(TM) SE Runtime Environment (build 17.0.4+11-LTS-179)
   Java HotSpot(TM) 64-Bit Server VM (build 17.0.4+11-LTS-179, mixed mode, sharing)
   ```
   
   And the if clause is changed to avoid duplicated code.



-- 
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: issues-unsubscribe@celeborn.apache.org

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