Xiang Yang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20921 )

Change subject: IMPALA-12362: (part-1/4) Refactor service management scripts.
......................................................................


Patch Set 10:

(8 comments)

Hi quanlong, thanks for your review, I've added a new command 'health' to check 
whether the impala service is ready or not.

http://gerrit.cloudera.org:8080/#/c/20921/7/package/bin/impala.sh
File package/bin/impala.sh:

http://gerrit.cloudera.org:8080/#/c/20921/7/package/bin/impala.sh@21
PS7, Line 21: custom
> nit: "customize"
Done


http://gerrit.cloudera.org:8080/#/c/20921/7/package/bin/impala.sh@22
PS7, Line 22: custom
> nit: "customize"
Done


http://gerrit.cloudera.org:8080/#/c/20921/7/package/bin/impala.sh@47
PS7, Line 47:     return 1
> This is not enough since the PID can be reused by other process after the s
Done.
I added a new command 'health' to check whether the impala service is ready or 
not.


http://gerrit.cloudera.org:8080/#/c/20921/7/package/bin/impala.sh@50
PS7, Line 50:   if ps -p ${pid} -o comm=|grep ${service} &> /dev/null ; then
            :     echo "${service} is running with PID ${pid}."
            :     return 0
            :   fi
> nit: move the content of the else-branch outside since the if-branch always
Done


http://gerrit.cloudera.org:8080/#/c/20921/7/package/bin/impala.sh@86
PS7, Line 86: jvm_dir=$(dirn
> Use "${HADOOP_HOME:=}" in case it's not set.
Done


http://gerrit.cloudera.org:8080/#/c/20921/7/package/bin/impala.sh@97
PS7, Line 97:
> Use "${LIBHDFS_OPTS:-}" in case it's not set.
Done


http://gerrit.cloudera.org:8080/#/c/20921/7/package/bin/start-impalad.sh
File package/bin/start-impalad.sh:

http://gerrit.cloudera.org:8080/#/c/20921/7/package/bin/start-impalad.sh@a26
PS7, Line 26:
            :
> Can we add a similar guide somewhere for running with another username?
Done. added to the header of impala.sh.


http://gerrit.cloudera.org:8080/#/c/20921/7/package/conf/statestored_flags
File package/conf/statestored_flags:

http://gerrit.cloudera.org:8080/#/c/20921/7/package/conf/statestored_flags@5
PS7, Line 5: v=1
> We can remove "webserver_doc_root" as you suggested at https://gerrit.cloud
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8f4dcad9cfa12d351d562e7ef8c0a8957d3ca147
Gerrit-Change-Number: 20921
Gerrit-PatchSet: 10
Gerrit-Owner: Xiang Yang <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Xiang Yang <[email protected]>
Gerrit-Comment-Date: Sat, 03 Feb 2024 10:46:28 +0000
Gerrit-HasComments: Yes

Reply via email to