Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/20263 )
Change subject: IMPALA-12362: Improve Linux packaging support. ...................................................................... Patch Set 6: (8 comments) Thanks for improving the packaging support! There are several independent topics in this patch, e.g. CMake files refactoring, scripts refactoring, default configuration changes, adding more binaries, etc. To be easier for review, It'd be nice to split this into several smaller patches. http://gerrit.cloudera.org:8080/#/c/20263/6//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20263/6//COMMIT_MSG@18 PS6, Line 18: - add support for admissiond service. It'd be nice to also add impala-profile-tool so users can parse the thrift profiles. The location is be/build/release/util/impala-profile-tool http://gerrit.cloudera.org:8080/#/c/20263/6/package/CMakeLists.txt File package/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/20263/6/package/CMakeLists.txt@28 PS6, Line 28: install(FILES ${STATESTORED_SYMLINK} DESTINATION ${IMPALA_INSTALLDIR}/sbin) : install(FILES ${CATALOGD_SYMLINK} DESTINATION ${IMPALA_INSTALLDIR}/sbin) : install(FILES ${ADMISSIOND_SYMLINK} DESTINATION ${IMPALA_INSTALLDIR}/sbin) : install(TARGETS impalad DESTINATION ${IMPALA_INSTALLDIR}/sbin) We already have these in be/src/service/CMakeLists.txt. Do we need to duplicate them here? http://gerrit.cloudera.org:8080/#/c/20263/6/package/bin/impala.sh File package/bin/impala.sh: http://gerrit.cloudera.org:8080/#/c/20263/6/package/bin/impala.sh@21 PS6, Line 21: custom nit: customize http://gerrit.cloudera.org:8080/#/c/20263/6/package/bin/impala.sh@22 PS6, Line 22: custom nit: customize http://gerrit.cloudera.org:8080/#/c/20263/6/package/bin/impala.sh@49 PS6, Line 49: else nit: don't need "else" http://gerrit.cloudera.org:8080/#/c/20263/6/package/bin/impala.sh@109 PS6, Line 109: sleep 1 Any reason for removing the logic of wait_for_ready? I think it's helpful when launching Impala on a large cluster. Admins can know when the launch really finishes. http://gerrit.cloudera.org:8080/#/c/20263/6/package/conf/catalogd_flags File package/conf/catalogd_flags: http://gerrit.cloudera.org:8080/#/c/20263/6/package/conf/catalogd_flags@a9 PS6, Line 9: Why do we remove this? Without the correct doc root, the webUI might not be able to be rendered. http://gerrit.cloudera.org:8080/#/c/20263/6/package/conf/catalogd_flags@20 PS6, Line 20: # -v=1 We should set -v=1 explicitly. Otherwise, no INFO logs will be shown. The default of glog is -v=0. -- To view, visit http://gerrit.cloudera.org:8080/20263 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If3914dcda69f81a735cdf70d76c59fa09454777b Gerrit-Change-Number: 20263 Gerrit-PatchSet: 6 Gerrit-Owner: Xiang Yang <yx91...@126.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Laszlo Gaal <laszlo.g...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Comment-Date: Tue, 16 Jan 2024 02:16:31 +0000 Gerrit-HasComments: Yes
