Lars Volker 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:

(7 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@62
PS6, Line 62: HandlerTerm
"HandleSigTerm"?


http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/catalog/catalogd-main.cc@64
PS6, Line 64:   exit(0);
Does this change the behavior? I.e., would we have exited with 0 before, too?


http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/catalog/catalogd-main.cc@120
PS6, Line 120:   // We want to log a message when catalogd is
nit: wrap comments at 90 chars, here and elsewhere.


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

http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/service/impalad-main.cc@60
PS6, Line 60: // doing an exit.
This code is now copy-pasted to several files. Can you dedup it?


http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/service/impalad-main.cc@104
PS6, Line 104:   // We want to log a message when impalad is
same here


http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/util/minidump.cc
File be/src/util/minidump.cc:

http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/util/minidump.cc@101
PS6, Line 101:   LOG(INFO) << "Signal received SIGUSR1" << endl;
The other messages are "Caught SIGTERM daemon going down". Can you make them 
more consistent? E.g. "Caught signal: SIGUSR1"?


http://gerrit.cloudera.org:8080/#/c/10847/6/tests/custom_cluster/test_breakpad.py
File tests/custom_cluster/test_breakpad.py:

http://gerrit.cloudera.org:8080/#/c/10847/6/tests/custom_cluster/test_breakpad.py@239
PS6, Line 239:     self.assert_impalad_log_contains('INFO', 'Caught SIGTERM 
daemon going down',
Is there a way to also check the exit code of the processes?



--
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 <l...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 18:01:32 +0000
Gerrit-HasComments: Yes

Reply via email to