This is an automated email from the ASF dual-hosted git repository. michaelsmith pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 879afbab1fdcf8d81e69db559c5c5d08b52d11e4 Author: Michael Smith <[email protected]> AuthorDate: Thu May 4 16:13:12 2023 -0700 IMPALA-11260: Add add-opens to JAVA_TOOL_OPTIONS on startup During Impala startup, Before starting the JVM (by calling libhdfs), adds add-opens calls to JAVA_TOOL_OPTIONS to ensure Ehcache has access to non-public members so it can accurately calculate object size. This effectively circumvents new security precautions in Java 9+. Use '--jvm_automatic_add_opens=false' to disable it. Tested with Java 11 JDBC_TEST=false EE_TEST=false FE_TEST=false BE_TEST=false \ CLUSTER_TEST_FILES=custom_cluster/test_local_catalog.py \ run-all-tests.sh Change-Id: I47a6533b2aa94593d9348e8e3606633f06a111e8 Reviewed-on: http://gerrit.cloudera.org:8080/19845 Reviewed-by: Joe McDonnell <[email protected]> Reviewed-by: Quanlong Huang <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- be/src/common/init.cc | 58 +++++++++++++++++++++++++++++++++++++++++++++ bin/start-impala-cluster.py | 22 ----------------- docker/daemon_entrypoint.sh | 38 ----------------------------- 3 files changed, 58 insertions(+), 60 deletions(-) diff --git a/be/src/common/init.cc b/be/src/common/init.cc index f82326a5f..04051b5eb 100644 --- a/be/src/common/init.cc +++ b/be/src/common/init.cc @@ -94,6 +94,10 @@ DEFINE_string(local_library_dir, "/tmp", "Scratch space for local fs operations. Currently used for copying " "UDF binaries locally from HDFS and also for initializing the timezone db"); +DEFINE_bool(jvm_automatic_add_opens, true, + "Adds necessary --add-opens options for core Java modules necessary to correctly " + "calculate catalog metadata cache object sizes."); + // Defined by glog. This allows users to specify the log level using a glob. For // example -vmodule=*scanner*=3 would enable full logging for scanners. If redaction // is enabled, this option won't be allowed because some logging dumps table data @@ -297,6 +301,57 @@ void BlockImpalaShutdownSignal() { AbortIfError(pthread_sigmask(SIG_BLOCK, &signals, nullptr), error_msg); } +// Append add-opens args to JAVA_TOOL_OPTIONS for ehcache. +static Status JavaAddOpens() { + if (!FLAGS_jvm_automatic_add_opens) return Status::OK(); + + stringstream val_out; + char* current_val_c = getenv("JAVA_TOOL_OPTIONS"); + if (current_val_c != NULL) { + val_out << current_val_c; + } + + // These options are required for Java 9+, and ignored by Java 8. + for (const string& param : { + "--add-opens=java.base/java.io=ALL-UNNAMED", + "--add-opens=java.base/java.lang.module=ALL-UNNAMED", + "--add-opens=java.base/java.lang.ref=ALL-UNNAMED", + "--add-opens=java.base/java.lang.reflect=ALL-UNNAMED", + "--add-opens=java.base/java.lang=ALL-UNNAMED", + "--add-opens=java.base/java.net=ALL-UNNAMED", + "--add-opens=java.base/java.nio.charset=ALL-UNNAMED", + "--add-opens=java.base/java.nio.file.attribute=ALL-UNNAMED", + "--add-opens=java.base/java.nio=ALL-UNNAMED", + "--add-opens=java.base/java.security=ALL-UNNAMED", + "--add-opens=java.base/java.util.concurrent=ALL-UNNAMED", + "--add-opens=java.base/java.util.jar=ALL-UNNAMED", + "--add-opens=java.base/java.util.zip=ALL-UNNAMED", + "--add-opens=java.base/java.util=ALL-UNNAMED", + "--add-opens=java.base/jdk.internal.loader=ALL-UNNAMED", + "--add-opens=java.base/jdk.internal.math=ALL-UNNAMED", + "--add-opens=java.base/jdk.internal.module=ALL-UNNAMED", + "--add-opens=java.base/jdk.internal.perf=ALL-UNNAMED", + "--add-opens=java.base/jdk.internal.ref=ALL-UNNAMED", + "--add-opens=java.base/jdk.internal.reflect=ALL-UNNAMED", + "--add-opens=java.base/jdk.internal.util.jar=ALL-UNNAMED", + "--add-opens=java.base/sun.nio.fs=ALL-UNNAMED", + "--add-opens=jdk.dynalink/jdk.dynalink.beans=ALL-UNNAMED", + "--add-opens=jdk.dynalink/jdk.dynalink.linker.support=ALL-UNNAMED", + "--add-opens=jdk.dynalink/jdk.dynalink.linker=ALL-UNNAMED", + "--add-opens=jdk.dynalink/jdk.dynalink.support=ALL-UNNAMED", + "--add-opens=jdk.dynalink/jdk.dynalink=ALL-UNNAMED", + "--add-opens=jdk.management.jfr/jdk.management.jfr=ALL-UNNAMED", + "--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED" + }) { + val_out << " " << param; + } + + if (setenv("JAVA_TOOL_OPTIONS", val_out.str().c_str(), 1) < 0) { + return Status(Substitute("Could not update JAVA_TOOL_OPTIONS: $0", GetStrErrMsg())); + } + return Status::OK(); +} + void impala::InitCommonRuntime(int argc, char** argv, bool init_jvm, TestInfo::Mode test_mode, bool external_fe) { srand(time(NULL)); @@ -451,6 +506,9 @@ void impala::InitCommonRuntime(int argc, char** argv, bool init_jvm, if (!fs_cache_init_status.ok()) CLEAN_EXIT_WITH_ERROR(fs_cache_init_status.GetDetail()); if (init_jvm) { + // Add JAVA_TOOL_OPTIONS for ehcache + ABORT_IF_ERROR(JavaAddOpens()); + if (!external_fe) { JniUtil::InitLibhdfs(); } diff --git a/bin/start-impala-cluster.py b/bin/start-impala-cluster.py index 98f391503..88b886ab7 100755 --- a/bin/start-impala-cluster.py +++ b/bin/start-impala-cluster.py @@ -212,28 +212,6 @@ def build_java_tool_options(jvm_debug_port=None): "server=y,suspend=n ").format(debug_port=jvm_debug_port) + java_tool_options if options.jvm_args is not None: java_tool_options += " " + options.jvm_args - - if int(os.environ["IMPALA_JDK_VERSION"]) >= 9: - JAVA11_BASE_OPENS = ["java.io", "java.lang.module", "java.lang.ref", "java.lang", - "java.net", "java.nio.charset", "java.nio.file.attribute", - "java.nio", "java.security", "java.util.concurrent", - "java.util.jar", "java.util.zip", "java.util", - "jdk.internal.loader", "jdk.internal.math", - "jdk.internal.module", "jdk.internal.perf", "jdk.internal.ref", - "jdk.internal.reflect", "jdk.internal.util.jar", "sun.nio.fs"] - for base_open in JAVA11_BASE_OPENS: - java_tool_options += " --add-opens=java.base/{0}=ALL-UNNAMED".format(base_open) - - JAVA11_DYNALINK_OPENS = ["jdk.dynalink.beans", "jdk.dynalink.linker.support", - "jdk.dynalink.linker", "jdk.dynalink.support", - "jdk.dynalink"] - for dynalink_open in JAVA11_DYNALINK_OPENS: - java_tool_options += \ - " --add-opens=jdk.dynalink/{0}=ALL-UNNAMED".format(dynalink_open) - - java_tool_options += " --add-opens=jdk.management.jfr/jdk.management.jfr=ALL-UNNAMED" - java_tool_options += \ - " --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED" return java_tool_options def kill_matching_processes(binary_names, force=False): diff --git a/docker/daemon_entrypoint.sh b/docker/daemon_entrypoint.sh index 434ecc840..d6209d743 100755 --- a/docker/daemon_entrypoint.sh +++ b/docker/daemon_entrypoint.sh @@ -55,13 +55,11 @@ fi # This detection logic handles both Java 8 and Java 11. # If both are present (which shouldn't happen), Java 11 is preferred. JAVA_HOME=Unknown -USING_JAVA11=false if [[ $DISTRIBUTION == Ubuntu ]]; then # Since the Java location includes the CPU architecture, use a glob # to find Java home if compgen -G "/usr/lib/jvm/java-11-openjdk*" ; then JAVA_HOME=$(compgen -G "/usr/lib/jvm/java-11-openjdk*") - USING_JAVA11=true if compgen -G "/usr/lib/jvm/java-8-openjdk*" ; then echo "WARNING: Java 8 is also present" fi @@ -73,7 +71,6 @@ elif [[ $DISTRIBUTION == Redhat ]]; then if [[ -d /usr/lib/jvm/jre-11 ]]; then echo "Detected Java 11" JAVA_HOME=/usr/lib/jvm/jre-11 - USING_JAVA11=true if [[ -d /usr/lib/jvm/jre-1.8.0 ]]; then echo "WARNING: Java 8 is also present" fi @@ -117,41 +114,6 @@ done echo "CLASSPATH: $CLASSPATH" echo "LD_LIBRARY_PATH: $LD_LIBRARY_PATH" -# IMPALA-11260: ehcache underestimates sizes on Java 9+ due to restrictions -# on reflection. As a workaround, this turns off the restrictions to allow -# ehcache to function. The real fix needs to happen on the ehcache side. -if $USING_JAVA11 ; then - JAVA11_OPTIONS=" --add-opens=java.base/java.io=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.lang.module=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.lang.ref=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.lang.reflect=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.lang=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.net=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.nio.charset=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.nio.file.attribute=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.nio=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.security=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.util.concurrent=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.util.jar=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.util.zip=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/java.util=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/jdk.internal.math=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/jdk.internal.module=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/jdk.internal.reflect=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/jdk.internal.util.jar=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=java.base/sun.nio.fs=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=jdk.dynalink/jdk.dynalink.beans=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=jdk.dynalink/jdk.dynalink.linker.support=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=jdk.dynalink/jdk.dynalink.linker=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=jdk.dynalink/jdk.dynalink.support=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=jdk.dynalink/jdk.dynalink=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=jdk.management.jfr/jdk.management.jfr=ALL-UNNAMED" - JAVA11_OPTIONS+=" --add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED" - JAVA_TOOL_OPTIONS="$JAVA11_OPTIONS $JAVA_TOOL_OPTIONS" -fi - # Default to 2GB heap. Allow overriding by externally-set JAVA_TOOL_OPTIONS. export JAVA_TOOL_OPTIONS="-Xmx2g $JAVA_TOOL_OPTIONS"
