You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Michael Smith (Code Review)" <ge...@cloudera.org> on 2023/05/24 18:10:53 UTC

[Impala-ASF-CR] IMPALA-11253: Support testing with Java 11 (take 2)

Michael Smith has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19928


Change subject: IMPALA-11253: Support testing with Java 11 (take 2)
......................................................................

IMPALA-11253: Support testing with Java 11 (take 2)

Adds new environment variable IMPALA_JDK_VERSION which can be 'system',
'8', or '11'.  The default is 'system', which uses the same logic as
before. If set to 8 or 11, it will ignore the system java and search for
java of that specific version (based on specific directories for Ubuntu
and Redhat). This is used by bin/bootstrap_system.sh to determine
whether to install java 8 or java 11 (other versions can come later). If
IMPALA_JDK_VERSION=11, then bin/start-impala-cluster.py adds the opens
needed to deal with the ehcache issue.

This no longer puts JAVA_HOME in bin/impala-config-local.sh as part of
bootstrap_system.sh. Instead, it provides a new environment variable
IMPALA_JAVA_HOME_OVERRIDE, which will be preferred over
IMPALA_JDK_VERSION.

This also updates the versions of Maven plugins related to the build.

Source and target releases are still set to Java 8 compatibility.

Adds a verifier to the end of run-all-tests that
InaccessibleObjectException is not present in impalad logs. Tested with

  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

Testing: ran test suite with Java 11

This reverts commit 1b6011c6a08123bb15921add253074dca7b4d390.

Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
---
M CMakeLists.txt
M LICENSE.txt
M README-build.md
M bin/bootstrap_system.sh
A bin/impala-config-java.sh
M bin/impala-config.sh
M bin/run-all-tests.sh
D cmake_modules/FindJNI.cmake
M fe/pom.xml
M fe/src/test/java/org/apache/impala/util/JMXJsonUtilTest.java
M java/TableFlattener/pom.xml
M java/datagenerator/pom.xml
M java/executor-deps/pom.xml
M java/ext-data-source/api/pom.xml
M java/ext-data-source/sample/pom.xml
M java/ext-data-source/test/pom.xml
M java/query-event-hook-api/pom.xml
M java/shaded-deps/hive-exec/pom.xml
M java/shaded-deps/s3a-aws-sdk/pom.xml
M java/test-corrupt-hive-udfs/pom.xml
M java/test-hive-udfs/pom.xml
M java/yarn-extras/pom.xml
M testdata/bin/run-ranger-server.sh
A tests/verifiers/test_banned_log_messages.py
24 files changed, 207 insertions(+), 406 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/19928/1
-- 
To view, visit http://gerrit.cloudera.org:8080/19928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Gerrit-Change-Number: 19928
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>

[Impala-ASF-CR] IMPALA-11253: Support testing with Java 11 (take 2)

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19928 )

Change subject: IMPALA-11253: Support testing with Java 11 (take 2)
......................................................................


Patch Set 2:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9348/ DRY_RUN=true


-- 
To view, visit http://gerrit.cloudera.org:8080/19928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Gerrit-Change-Number: 19928
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 24 May 2023 21:56:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11253: Support testing with Java 11 (take 2)

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19928 )

Change subject: IMPALA-11253: Support testing with Java 11 (take 2)
......................................................................

IMPALA-11253: Support testing with Java 11 (take 2)

Adds new environment variable IMPALA_JDK_VERSION which can be 'system',
'8', or '11'.  The default is 'system', which uses the same logic as
before. If set to 8 or 11, it will ignore the system java and search for
java of that specific version (based on specific directories for Ubuntu
and Redhat). This is used by bin/bootstrap_system.sh to determine
whether to install java 8 or java 11 (other versions can come later). If
IMPALA_JDK_VERSION=11, then bin/start-impala-cluster.py adds the opens
needed to deal with the ehcache issue.

This no longer puts JAVA_HOME in bin/impala-config-local.sh as part of
bootstrap_system.sh. Instead, it provides a new environment variable
IMPALA_JAVA_HOME_OVERRIDE, which will be preferred over
IMPALA_JDK_VERSION.

This also updates the versions of Maven plugins related to the build.

Source and target releases are still set to Java 8 compatibility.

Adds a verifier to the end of run-all-tests that
InaccessibleObjectException is not present in impalad logs. Tested with

  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

Testing: ran test suite with Java 11

This reverts the revert commit 1b6011c, restoring these changes minus
code to update IMPALA_JDK_VERSION based on $JAVA -version as that could
break subsequent sourcing of impala-config.sh.

Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Reviewed-on: http://gerrit.cloudera.org:8080/19928
Reviewed-by: Joe McDonnell <jo...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M CMakeLists.txt
M LICENSE.txt
M README-build.md
M bin/bootstrap_system.sh
A bin/impala-config-java.sh
M bin/impala-config.sh
M bin/run-all-tests.sh
D cmake_modules/FindJNI.cmake
M fe/pom.xml
M fe/src/test/java/org/apache/impala/util/JMXJsonUtilTest.java
M java/TableFlattener/pom.xml
M java/datagenerator/pom.xml
M java/executor-deps/pom.xml
M java/ext-data-source/api/pom.xml
M java/ext-data-source/sample/pom.xml
M java/ext-data-source/test/pom.xml
M java/query-event-hook-api/pom.xml
M java/shaded-deps/hive-exec/pom.xml
M java/shaded-deps/s3a-aws-sdk/pom.xml
M java/test-corrupt-hive-udfs/pom.xml
M java/test-hive-udfs/pom.xml
M java/yarn-extras/pom.xml
M testdata/bin/run-ranger-server.sh
A tests/verifiers/test_banned_log_messages.py
24 files changed, 207 insertions(+), 406 deletions(-)

Approvals:
  Joe McDonnell: Looks good to me, approved
  Impala Public Jenkins: Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/19928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Gerrit-Change-Number: 19928
Gerrit-PatchSet: 3
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11253: Support testing with Java 11 (take 2)

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19928 )

Change subject: IMPALA-11253: Support testing with Java 11 (take 2)
......................................................................


Patch Set 2:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19928/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19928/1//COMMIT_MSG@36
PS1, Line 36: This reverts the revert commit 1b6011c, restoring these chang
> If I'm understanding this properly, the problem with the first patch was th
Done



-- 
To view, visit http://gerrit.cloudera.org:8080/19928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Gerrit-Change-Number: 19928
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 24 May 2023 21:50:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11253: Support testing with Java 11 (take 2)

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19928 )

Change subject: IMPALA-11253: Support testing with Java 11 (take 2)
......................................................................


Patch Set 2: Code-Review+2

Thanks for fixing this


-- 
To view, visit http://gerrit.cloudera.org:8080/19928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Gerrit-Change-Number: 19928
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 24 May 2023 21:50:56 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11253: Support testing with Java 11 (take 2)

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19928 )

Change subject: IMPALA-11253: Support testing with Java 11 (take 2)
......................................................................


Patch Set 2: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/19928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Gerrit-Change-Number: 19928
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 25 May 2023 03:15:32 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-11253: Support testing with Java 11 (take 2)

Posted by "Michael Smith (Code Review)" <ge...@cloudera.org>.
Hello Quanlong Huang, Joe McDonnell, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/19928

to look at the new patch set (#2).

Change subject: IMPALA-11253: Support testing with Java 11 (take 2)
......................................................................

IMPALA-11253: Support testing with Java 11 (take 2)

Adds new environment variable IMPALA_JDK_VERSION which can be 'system',
'8', or '11'.  The default is 'system', which uses the same logic as
before. If set to 8 or 11, it will ignore the system java and search for
java of that specific version (based on specific directories for Ubuntu
and Redhat). This is used by bin/bootstrap_system.sh to determine
whether to install java 8 or java 11 (other versions can come later). If
IMPALA_JDK_VERSION=11, then bin/start-impala-cluster.py adds the opens
needed to deal with the ehcache issue.

This no longer puts JAVA_HOME in bin/impala-config-local.sh as part of
bootstrap_system.sh. Instead, it provides a new environment variable
IMPALA_JAVA_HOME_OVERRIDE, which will be preferred over
IMPALA_JDK_VERSION.

This also updates the versions of Maven plugins related to the build.

Source and target releases are still set to Java 8 compatibility.

Adds a verifier to the end of run-all-tests that
InaccessibleObjectException is not present in impalad logs. Tested with

  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

Testing: ran test suite with Java 11

This reverts the revert commit 1b6011c, restoring these changes minus
code to update IMPALA_JDK_VERSION based on $JAVA -version as that could
break subsequent sourcing of impala-config.sh.

Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
---
M CMakeLists.txt
M LICENSE.txt
M README-build.md
M bin/bootstrap_system.sh
A bin/impala-config-java.sh
M bin/impala-config.sh
M bin/run-all-tests.sh
D cmake_modules/FindJNI.cmake
M fe/pom.xml
M fe/src/test/java/org/apache/impala/util/JMXJsonUtilTest.java
M java/TableFlattener/pom.xml
M java/datagenerator/pom.xml
M java/executor-deps/pom.xml
M java/ext-data-source/api/pom.xml
M java/ext-data-source/sample/pom.xml
M java/ext-data-source/test/pom.xml
M java/query-event-hook-api/pom.xml
M java/shaded-deps/hive-exec/pom.xml
M java/shaded-deps/s3a-aws-sdk/pom.xml
M java/test-corrupt-hive-udfs/pom.xml
M java/test-hive-udfs/pom.xml
M java/yarn-extras/pom.xml
M testdata/bin/run-ranger-server.sh
A tests/verifiers/test_banned_log_messages.py
24 files changed, 207 insertions(+), 406 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/28/19928/2
-- 
To view, visit http://gerrit.cloudera.org:8080/19928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Gerrit-Change-Number: 19928
Gerrit-PatchSet: 2
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-11253: Support testing with Java 11 (take 2)

Posted by "Joe McDonnell (Code Review)" <ge...@cloudera.org>.
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19928 )

Change subject: IMPALA-11253: Support testing with Java 11 (take 2)
......................................................................


Patch Set 1:

(1 comment)

This is looking good

http://gerrit.cloudera.org:8080/#/c/19928/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19928/1//COMMIT_MSG@36
PS1, Line 36: This reverts commit 1b6011c6a08123bb15921add253074dca7b4d390.
If I'm understanding this properly, the problem with the first patch was that IMPALA_JDK_VERSION='system' was getting converted to IMPALA_JDK_VERSION=8 by the code here:

https://github.com/apache/impala/commit/ee6395db760e6629442c7e3fdda34519c61641c5#diff-fc0aa0b99cb982f212166f43bf69341f2a2a9af15401b3b8d4b09b4a4a3c6225R459-R468

Can we add a sentence saying that we fixed this issue with the original patch?



-- 
To view, visit http://gerrit.cloudera.org:8080/19928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Gerrit-Change-Number: 19928
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 24 May 2023 21:39:40 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-11253: Support testing with Java 11 (take 2)

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19928 )

Change subject: IMPALA-11253: Support testing with Java 11 (take 2)
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/13112/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.


-- 
To view, visit http://gerrit.cloudera.org:8080/19928
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie16504ad5738b1f228f97044afd3d9017ccc6c53
Gerrit-Change-Number: 19928
Gerrit-PatchSet: 1
Gerrit-Owner: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Joe McDonnell <jo...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 24 May 2023 18:33:57 +0000
Gerrit-HasComments: No