This is an automated email from the ASF dual-hosted git repository.
stigahuang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 3346d070a IMPALA-11260: (Addendum) Restrict add-opens to Java 9+
3346d070a is described below
commit 3346d070ad47b49ccbfa087e27b6ecb137b36a0a
Author: Michael Smith <[email protected]>
AuthorDate: Thu May 25 11:44:08 2023 -0700
IMPALA-11260: (Addendum) Restrict add-opens to Java 9+
Restricts jvm_automatic_add_opens to only apply to Java 9+ where the
option exists. Previously it would also include it in Java 8, which
caused the JVM to ignore all options in JAVA_TOOL_OPTIONS.
Tests for Java version by running $JAVA_HOME/bin/java -version (or
"java" if JAVA_HOME is unset) and parsing version from the first line.
All JVM implementations are expected to include the version in a quoted
string, such as "1.8.0_42" and "11.0.1".
Also added add-opens flags for frontend tests.
test_no_inaccessible_objects detected this in a test run.
Testing:
- manually confirmed -agentlib options are present with both Java
8 and Java 11.
- promoted test_jvm_mem_tracking to run in all strategies, as it's fast
and ensures JAVA_TOOL_OPTIONS is honored.
Change-Id: I85953e685f6bbbd213afd93f389066e82f193ddf
Reviewed-on: http://gerrit.cloudera.org:8080/19939
Reviewed-by: Zoltan Borok-Nagy <[email protected]>
Reviewed-by: Joe McDonnell <[email protected]>
Reviewed-by: Quanlong Huang <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
---
be/src/common/init.cc | 30 ++++++++++++++++++++++-
bin/run-all-tests.sh | 35 +++++++++++++++++++++++++++
docker/daemon_entrypoint.sh | 1 +
tests/custom_cluster/test_jvm_mem_tracking.py | 7 ------
4 files changed, 65 insertions(+), 8 deletions(-)
diff --git a/be/src/common/init.cc b/be/src/common/init.cc
index 04051b5eb..fa3f3a01f 100644
--- a/be/src/common/init.cc
+++ b/be/src/common/init.cc
@@ -18,6 +18,8 @@
#include "common/init.h"
#include <csignal>
+#include <regex>
+#include <boost/filesystem.hpp>
#include <gperftools/heap-profiler.h>
#include <third_party/lss/linux_syscall_support.h>
@@ -51,6 +53,7 @@
#include "util/network-util.h"
#include "util/openssl-util.h"
#include "util/os-info.h"
+#include "util/os-util.h"
#include "util/parse-util.h"
#include "util/periodic-counter-updater.h"
#include "util/pretty-printer.h"
@@ -305,13 +308,38 @@ void BlockImpalaShutdownSignal() {
static Status JavaAddOpens() {
if (!FLAGS_jvm_automatic_add_opens) return Status::OK();
+ // Unknown flags in JAVA_TOOL_OPTIONS causes Java to ignore everything, so
only include
+ // --add-opens for Java 9+. Uses JAVA_HOME if set, otherwise relies on PATH.
+ string cmd = "java";
+ const char* java_home = getenv("JAVA_HOME");
+ if (java_home != NULL) {
+ cmd = (boost::filesystem::path(java_home) / "bin" / "java").string();
+ }
+ cmd += " -version 2>&1";
+ string msg;
+ if (!RunShellProcess(cmd, &msg, false, {"JAVA_TOOL_OPTIONS"})) {
+ return Status(msg);
+ }
+
+ // Find a version string in the first line. Return if major version isn't 9+.
+ string first_line;
+ std::getline(istringstream(msg), first_line);
+ // Need to allow for a wide variety of formats for different JDK
implementations.
+ // Example: openjdk version "11.0.19" 2023-04-18
+ std::regex java_version_pattern("\"([0-9]{1,3})\\.[0-9]+\\.[0-9]+[^\"]*\"");
+ std::smatch matches;
+ if (!std::regex_search(first_line, matches, java_version_pattern)) {
+ return Status(Substitute("Could not determine Java version: $0", msg));
+ }
+ DCHECK_EQ(matches.size(), 2);
+ if (std::stoi(matches.str(1)) < 9) 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",
diff --git a/bin/run-all-tests.sh b/bin/run-all-tests.sh
index 22bb4e44e..051a89efd 100755
--- a/bin/run-all-tests.sh
+++ b/bin/run-all-tests.sh
@@ -243,6 +243,41 @@ do
# Requires a running impalad cluster because some tests (such as
DataErrorTest and
# JdbcTest) queries against an impala cluster.
pushd "${IMPALA_FE_DIR}"
+ if $JAVA -version 2>&1 | grep -q -E ' version "(9|[1-9][0-9])\.'; then
+ # If running with Java 9+, add-opens to JAVA_TOOL_OPTIONS for
+ # CatalogdMetaProviderTest.testWeights
+ JAVA_OPTIONS=" --add-opens=java.base/java.io=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.lang.module=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.lang.ref=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.lang.reflect=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.lang=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.net=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.nio.charset=ALL-UNNAMED"
+ JAVA_OPTIONS+="
--add-opens=java.base/java.nio.file.attribute=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.nio=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.security=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.util.concurrent=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.util.jar=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.util.zip=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/java.util=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/jdk.internal.loader=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/jdk.internal.math=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/jdk.internal.module=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/jdk.internal.perf=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/jdk.internal.ref=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/jdk.internal.reflect=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/jdk.internal.util.jar=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=java.base/sun.nio.fs=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=jdk.dynalink/jdk.dynalink.beans=ALL-UNNAMED"
+ JAVA_OPTIONS+="
--add-opens=jdk.dynalink/jdk.dynalink.linker.support=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=jdk.dynalink/jdk.dynalink.linker=ALL-UNNAMED"
+ JAVA_OPTIONS+="
--add-opens=jdk.dynalink/jdk.dynalink.support=ALL-UNNAMED"
+ JAVA_OPTIONS+=" --add-opens=jdk.dynalink/jdk.dynalink=ALL-UNNAMED"
+ JAVA_OPTIONS+="
--add-opens=jdk.management.jfr/jdk.management.jfr=ALL-UNNAMED"
+ JAVA_OPTIONS+="
--add-opens=jdk.management/com.sun.management.internal=ALL-UNNAMED"
+ export JAVA_TOOL_OPTIONS="$JAVA_OPTIONS ${JAVA_TOOL_OPTIONS-}"
+ fi
+
MVN_ARGS=""
if [[ "${TARGET_FILESYSTEM}" == "s3" ]]; then
# When running against S3, only run the S3 frontend tests.
diff --git a/docker/daemon_entrypoint.sh b/docker/daemon_entrypoint.sh
index d6209d743..fda7a65b9 100755
--- a/docker/daemon_entrypoint.sh
+++ b/docker/daemon_entrypoint.sh
@@ -87,6 +87,7 @@ if [[ $JAVA_HOME == Unknown ]]; then
fi
echo "JAVA_HOME: ${JAVA_HOME}"
+export JAVA_HOME
# Given JAVA_HOME, find libjsig.so and libjvm.so and add them to
LD_LIBRARY_PATH.
# JAVA_HOME could be a symlink, so follow symlinks when looking for the
libraries
LIB_JSIG_DIR=$(find -L "${JAVA_HOME}" -name libjsig.so | head -1 | xargs
dirname)
diff --git a/tests/custom_cluster/test_jvm_mem_tracking.py
b/tests/custom_cluster/test_jvm_mem_tracking.py
index 3999a0cb2..a5b615dd6 100644
--- a/tests/custom_cluster/test_jvm_mem_tracking.py
+++ b/tests/custom_cluster/test_jvm_mem_tracking.py
@@ -18,7 +18,6 @@
from __future__ import absolute_import, division, print_function
import logging
import json
-import pytest
from tests.common.custom_cluster_test_suite import CustomClusterTestSuite
from tests.common.impala_cluster import ImpalaCluster
from tests.verifiers.mem_usage_verifier import MemUsageVerifier,
parse_mem_value
@@ -32,12 +31,6 @@ class TestJvmMemTracker(CustomClusterTestSuite):
def get_workload(self):
return 'functional-query'
- @classmethod
- def setup_class(cls):
- if cls.exploration_strategy() != 'exhaustive':
- pytest.skip('runs only in exhaustive')
- super(TestJvmMemTracker, cls).setup_class()
-
@CustomClusterTestSuite.with_args(impalad_args="--mem_limit_includes_jvm=true \
--codegen_cache_capacity=0",
start_args="--jvm_args=-Xmx1g",
cluster_size=1)
