Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19863 )
Change subject: IMPALA-11941: Support Java 17 in Impala ...................................................................... Patch Set 23: (3 comments) http://gerrit.cloudera.org:8080/#/c/19863/21/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/19863/21/be/src/common/init.cc@308 PS21, Line 308: static std::string FindFileInPath( : const filesystem::path& path, const std::regex& pattern) { : if (path.empty()) return ""; : if (!filesystem::exists(path)) return ""; : if (filesystem::is_directory(path)) { : for (filesystem::directory_entry& child : filesystem::directory_iterator(path)) { : // if child matches jamm-*.jar, use it : if (std::regex_match(child.path().filename().string(), pattern)) { : return child.path().string(); : } : } : } else if (std::regex_match(path.filename().string(), pattern)) { : return path.string(); : } : return ""; : } > Should we put this in a utility file like be/src/util/filesystem-util.*? Th Sure, that makes sense. http://gerrit.cloudera.org:8080/#/c/19863/21/bin/run-all-tests.sh File bin/run-all-tests.sh: http://gerrit.cloudera.org:8080/#/c/19863/21/bin/run-all-tests.sh@247 PS21, Line 247: # Add Jamm as javaagent for CatalogdMetaProviderTest.testWeights : JAMM_JAR=$(compgen -G ${IMPALA_HOME}/fe/target/dependency/jamm-*.jar) : export JAVA_TOOL_OPTIONS="${JAVA_TOOL_OPTIONS-} -javaagent:${JAMM_JAR}" > Do the add-opens that we have in init.cc need to be specified here too? They don't come up. I haven't dug deep enough to know how to write a unit test that would require them. http://gerrit.cloudera.org:8080/#/c/19863/16/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java File fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java: http://gerrit.cloudera.org:8080/#/c/19863/16/fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java@2021 PS16, Line 2021: size += METER.measureDeep(key); > Ok, doesn't seem to completely avoid it in Java 17. custom_cluster/test_loc Almost done. Going to try running test_banned_log_messages on container test runs as some of the logs are exposed (for capture) and I found java.lang accesses that generate CannotAccessFieldException. -- To view, visit http://gerrit.cloudera.org:8080/19863 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic378896f572e030a3a019646a96a32a07866a737 Gerrit-Change-Number: 19863 Gerrit-PatchSet: 23 Gerrit-Owner: Michael Smith <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Laszlo Gaal <[email protected]> Gerrit-Reviewer: Michael Smith <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 20 Jun 2023 17:27:01 +0000 Gerrit-HasComments: Yes
