Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/22705 )
Change subject: POC: fix Yarn+Hive+Tez startup with JDK17 ...................................................................... Patch Set 11: (7 comments) http://gerrit.cloudera.org:8080/#/c/22705/11/bin/set-impala-java-tool-options.sh File bin/set-impala-java-tool-options.sh: http://gerrit.cloudera.org:8080/#/c/22705/11/bin/set-impala-java-tool-options.sh@20 PS11, Line 20: # Set JAVA_TOOL_OPTIONS needed by some Hadoop dependencies when running with JDK>=9 In this comment, please tell us what to do once all components has been built to be compatible with JDK 17. Can this file simply be deleted (revert this patch)? http://gerrit.cloudera.org:8080/#/c/22705/11/bin/set-impala-java-tool-options.sh@63 PS11, Line 63: export IMPALA_JAVA_TOOL_OPTIONS=$ADD_OPENS_OPTS Might be useful to have initial value for IMPALA_JAVA_TOOL_OPTIONS in jenkins EVAL_BLOCK, in case we want to prototype adding something. Thus, remove L60 and change this to: export IMPALA_JAVA_TOOL_OPTIONS="${IMPALA_JAVA_TOOL_OPTIONS} ${ADD_OPENS_OPTS}" http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/java/org/apache/impala/testutil/TestUtils.java File fe/src/test/java/org/apache/impala/testutil/TestUtils.java: http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@492 PS11, Line 492: version = version.substring(2, 3); Assert that version.equals("8") here? http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/java/org/apache/impala/testutil/TestUtils.java@495 PS11, Line 495: if( nit: missing whitespace. http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/resources/hive-site.xml.py File fe/src/test/resources/hive-site.xml.py: http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/resources/hive-site.xml.py@23 PS11, Line 23: hive_major_version = int(os.environ['IMPALA_HIVE_VERSION'][0]) : kerberize = os.environ.get('IMPALA_KERBERIZE') == 'true' : variant = os.environ.get('HIVE_VARIANT') : jdk_version = os.environ.get('IMPALA_JDK_VERSION_NUM') Change these constants to upper case as well? http://gerrit.cloudera.org:8080/#/c/22705/11/fe/src/test/resources/hive-site.xml.py@28 PS11, Line 28: JAVA_OPTIONS How about simply name this IMPALA_JAVA_TOOL_OPTIONS? http://gerrit.cloudera.org:8080/#/c/22705/11/tests/custom_cluster/test_catalog_hms_failures.py File tests/custom_cluster/test_catalog_hms_failures.py: http://gerrit.cloudera.org:8080/#/c/22705/11/tests/custom_cluster/test_catalog_hms_failures.py@30 PS11, Line 30: : NUM_SUBSCRIBERS = DEFAULT_CLUSTER_SIZE + 1 : : @SkipIf.is_test_jdk : class TestHiveMetaStoreFailure(CustomClusterTestSuite): flake8 should ask for 2 new lines here. Wonder why it silent this time. -- To view, visit http://gerrit.cloudera.org:8080/22705 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If58b64a21d14a4a55b12dfe9ea0b9c3d5fe9c9cf Gerrit-Change-Number: 22705 Gerrit-PatchSet: 11 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com> Gerrit-Reviewer: Michael Smith <michael.sm...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Mon, 31 Mar 2025 20:48:47 +0000 Gerrit-HasComments: Yes
