Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/22173 )
Change subject: IMPALA-13149: Show JVM info in the WebUI ...................................................................... Patch Set 5: (4 comments) http://gerrit.cloudera.org:8080/#/c/22173/5/be/src/util/default-path-handlers.cc File be/src/util/default-path-handlers.cc: http://gerrit.cloudera.org:8080/#/c/22173/5/be/src/util/default-path-handlers.cc@275 PS5, Line 275: catalog nit: "catalogd" http://gerrit.cloudera.org:8080/#/c/22173/5/be/src/util/default-path-handlers.cc@290 PS5, Line 290: if (!RunShellProcess(cmd, &msg, false, {"JAVA_TOOL_OPTIONS"})) { I'm a bit concerned on these: * It seems a waste to launch a new process for each http request just to get the constant version string. * There are also risks that we show a wrong java version due to JAVA_HOME is not set or incorrectly set. Note that with the dirs of libjvm.so and libjsig.so appear in LD_LIBRARY_PATH, Impala is able to launch. * We are also lack of test coverage on running this new process in a secured env. http://gerrit.cloudera.org:8080/#/c/22173/5/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/22173/5/tests/webserver/test_web_pages.py@128 PS5, Line 128: catalog nit: "catalogd" http://gerrit.cloudera.org:8080/#/c/22173/5/tests/webserver/test_web_pages.py@132 PS5, Line 132: assert "java_version" in root_page_json Can we simply merge this to the above test? -- To view, visit http://gerrit.cloudera.org:8080/22173 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iee03a05af9f0c5c2a7ab108ade503c6131bfe414 Gerrit-Change-Number: 22173 Gerrit-PatchSet: 5 Gerrit-Owner: Saurabh Katiyal <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Comment-Date: Tue, 14 Jan 2025 09:25:34 +0000 Gerrit-HasComments: Yes
