Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10847 )

Change subject: IMPALA-6271: Impala daemon should log a message when it's being 
shut down
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/catalog/catalogd-main.cc
File be/src/catalog/catalogd-main.cc:

http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/catalog/catalogd-main.cc@63
PS6, Line 63:   LOG(INFO) << "Caught SIGTERM daemon going down" << endl;
nit: please format this more grammatically. For example "Caught SIGTERM. Daemon 
will exit."
nit: no need for endl in log messages

Do you need to do something to flush the log files here or will the normal 
atexit hook ensure that they are flushed?

Is it worth logging the pid/uid of the sending process using the siginfo_t 
struct?


http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/catalog/catalogd-main.cc@125
PS6, Line 125:   sigaction(SIGTERM, &action, nullptr);
why not set this earlier? seems like there could be a relatively long startup 
sequence before installing the handler here.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29
Gerrit-Change-Number: 10847
Gerrit-PatchSet: 6
Gerrit-Owner: Pranay Singh
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Zoram Thanga <[email protected]>
Gerrit-Comment-Date: Tue, 31 Jul 2018 22:04:37 +0000
Gerrit-HasComments: Yes

Reply via email to