[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/10847 ) Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Reviewed-on: http://gerrit.cloudera.org:8080/10847 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 64 insertions(+), 5 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- 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: merged Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 19 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 18: Verified+1 -- 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: 18 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Sat, 29 Sep 2018 00:08:23 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 18: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3246/ DRY_RUN=false -- 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: 18 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 28 Sep 2018 20:18:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 18: Code-Review+2 -- 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: 18 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 28 Sep 2018 20:18:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
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 17: Code-Review+2 -- 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: 17 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 28 Sep 2018 20:18:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 17: (1 comment) http://gerrit.cloudera.org:8080/#/c/10847/17/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/17/be/src/common/init.cc@44 PS17, Line 44: #include "util/jni-util.h" > Did this come from a rebase? Yes , IMPALA-7421 seems to have introduced this -- 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: 17 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 27 Sep 2018 21:53:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 17: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/783/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- 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: 17 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 25 Sep 2018 18:11:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Andrew Sherman, Lars Volker, Zoram Thanga, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#17). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 64 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/17 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 17 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 16: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/766/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 16 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 24 Sep 2018 21:37:02 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 15: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/493/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 15 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 27 Aug 2018 19:40:20 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 15: (1 comment) http://gerrit.cloudera.org:8080/#/c/10847/14/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/14/be/src/common/init.cc@298 PS14, Line 298: > Should we consider aborting the process if we fail to install the signal ha Done -- 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: 15 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 27 Aug 2018 19:08:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Andrew Sherman, Lars Volker, Zoram Thanga, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#15). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 64 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/15 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 15 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
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 14: Code-Review+1 (1 comment) LGTM, one more nit. http://gerrit.cloudera.org:8080/#/c/10847/14/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/14/be/src/common/init.cc@298 PS14, Line 298: Should we consider aborting the process if we fail to install the signal handler? nit: add a colon after your message, before the space. -- 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: 14 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 27 Aug 2018 15:48:56 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 13: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/474/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 13 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 24 Aug 2018 19:54:29 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Andrew Sherman, Lars Volker, Zoram Thanga, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#13). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 60 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/13 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 13 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 12: (2 comments) http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@297 PS10, Line 297: if (sigaction(SIGTERM, , nullptr) == -1) { > > There is a value 1 displayed for old_action.sa_handler ... I tried printing the old action sa_handler in impalad.INFO and a value of 1 was displayed for sa_handler (old action). The value 1 corresponds to default action for SIGTERM, I don't think there is a need for validating it because default behavior of SIGTERM would result in killing of process without core dump and that behavior is not altered. http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc@298 PS12, Line 298: " > Ping? Added GetStrErrMsg() -- 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: 12 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 24 Aug 2018 19:20:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
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 12: (2 comments) http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@297 PS10, Line 297: if (sigaction(SIGTERM, , nullptr) == -1) { > There is a value 1 displayed for old_action.sa_handler ... Where is it displayed? Should we validate that it's this value then? http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc@298 PS12, Line 298: " > Should we also print the error itself then, e.g. using GetStrErrMsg()? Ping? -- 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: 12 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 24 Aug 2018 16:19:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@297 PS10, Line 297: if (sigaction(SIGTERM, , nullptr) == -1) { > Where does it display? Does that mean that we are overwriting a previous si I think that value corresponds to default signal handler constant value of 1, 2, 3 are default signal values. e.g https://www.ibm.com/support/knowledgecenter/en/ssw_i5_54/apis/sigactn.htm#TBLSIGTBL1 -- 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: 12 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 23 Aug 2018 17:43:01 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
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 12: (2 comments) http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@297 PS10, Line 297: if (sigaction(SIGTERM, , nullptr) == -1) { > There is a value 1 displayed for old_action.sa_handler so can't have a null Where does it display? Does that mean that we are overwriting a previous signal handler? What was that one doing? http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/12/be/src/common/init.cc@298 PS12, Line 298: " Should we also print the error itself then, e.g. using GetStrErrMsg()? -- 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: 12 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 23 Aug 2018 15:53:17 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 12: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/461/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 12 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 23 Aug 2018 15:01:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Andrew Sherman, Lars Volker, Zoram Thanga, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#12). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 60 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/12 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 12 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 11: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/450/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 11 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Wed, 22 Aug 2018 15:58:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Andrew Sherman, Lars Volker, Zoram Thanga, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#11). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 60 insertions(+), 5 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/11 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 11 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/util/minidump.cc File be/src/util/minidump.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/util/minidump.cc@119 PS10, Line 119: sigaction(SIGUSR1, _action, NULL); > While you're here, maybe check the return value to make sure this was succe oldact->sa_handler has a value 1 so can't have a check for nullptr -- 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: 10 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 21 Aug 2018 23:31:43 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 10: (2 comments) > (4 comments) http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@179 PS10, Line 179: : > nit: space on wrong side of : Done http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@297 PS10, Line 297: sigaction(SIGTERM, , nullptr); > We should at least check the return value here. We also should pass an empt There is a value 1 displayed for old_action.sa_handler so can't have a nullptr check that will hit the assert -- 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: 10 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 21 Aug 2018 23:30:09 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
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 10: (4 comments) http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@179 PS10, Line 179: : nit: space on wrong side of : You could also simplify to "Sender UID: root, PID: 1234" http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/common/init.cc@297 PS10, Line 297: sigaction(SIGTERM, , nullptr); We should at least check the return value here. We also should pass an empty struct sigaction *oldact and verify that there was no other signal handler present. http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/util/minidump.cc File be/src/util/minidump.cc: http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/util/minidump.cc@102 PS10, Line 102: : nit: whitespace, see comment on other location http://gerrit.cloudera.org:8080/#/c/10847/10/be/src/util/minidump.cc@119 PS10, Line 119: sigaction(SIGUSR1, _action, NULL); While you're here, maybe check the return value to make sure this was successful, and pass struct sigaction *oldact to make sure we didn't overwrite any other handlers? -- 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: 10 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 20 Aug 2018 22:41:31 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 9: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/227/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 9 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 07 Aug 2018 22:06:18 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Andrew Sherman, Lars Volker, Zoram Thanga, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#9). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 56 insertions(+), 4 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/9 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 9 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 8: (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@58 PS6, Line 58: using namespace apache::thrift; > If you make more changes to this file then you could remove this duplicated Done http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/catalog/catalogd-main.cc@64 PS6, Line 64: InitFeSupport(); > Without your change, what was the exit code when Impala caught a SIGTERM? I'm not able to find what was the exit code when Impala caught a SIGTERM out but as per convention exit code 0 is success/desirable and other exit codes are not desirable/failure. http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/common/init.cc@177 PS8, Line 177: // doing an exit. > nit: wrap at 90c, please also check the rest of the change Done http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/common/init.cc@291 PS8, Line 291: catalogd > This does not seem specific to the catalog. Updated the comment to include impalad/statestored/catalogd http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/common/init.cc@291 PS8, Line 291: // Signal handler for handling the SIGTERM. We want to log a message when catalogd is : // being shutdown using a SIGTERM. : struct sigaction action; : memset(, 0, sizeof(struct sigaction)); : action.sa_sigaction = : sigaction(SIGTERM, , nullptr); > Can this whole block be moved to earlier in the initialization? It takes a I have moved it further up as per Todd's comment, log messages won't be printed if I move the code block further up. http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/util/minidump.cc File be/src/util/minidump.cc: http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/util/minidump.cc@101 PS8, Line 101: LOG(INFO) << "Caught signal: SIGUSR1" << endl; > This should get the signal from the parameter. You could use the same log m Done http://gerrit.cloudera.org:8080/#/c/10847/8/tests/custom_cluster/test_breakpad.py File tests/custom_cluster/test_breakpad.py: http://gerrit.cloudera.org:8080/#/c/10847/8/tests/custom_cluster/test_breakpad.py@239 PS8, Line 239: self.assert_impalad_log_contains('INFO', 'Caught signal: SIGTERM. Daemon will exit', > Can you test that the log also contains the sending PID? I tried testing for PID, the problem is that signal is sent from a subprocess so can't get it's PID -- 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: 8 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 07 Aug 2018 21:29:15 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Andrew Sherman 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: (1 comment) 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@58 PS6, Line 58: using namespace apache::thrift; If you make more changes to this file then you could remove this duplicated line. -- 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: Andrew Sherman Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 06 Aug 2018 17:59:14 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
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 8: (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@64 PS6, Line 64: InitFeSupport(); > This exit will ensure a normal process termination, if I don't use it the p Without your change, what was the exit code when Impala caught a SIGTERM? http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/common/init.cc File be/src/common/init.cc: http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/common/init.cc@177 PS8, Line 177: // doing an exit. nit: wrap at 90c, please also check the rest of the change http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/common/init.cc@291 PS8, Line 291: catalogd This does not seem specific to the catalog. http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/common/init.cc@291 PS8, Line 291: // Signal handler for handling the SIGTERM. We want to log a message when catalogd is : // being shutdown using a SIGTERM. : struct sigaction action; : memset(, 0, sizeof(struct sigaction)); : action.sa_sigaction = : sigaction(SIGTERM, , nullptr); Can this whole block be moved to earlier in the initialization? It takes a while for everything to get initialized and if we catch a SIGTERM during that time we won't see any output (see Todd's earlier comment). http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/util/minidump.cc File be/src/util/minidump.cc: http://gerrit.cloudera.org:8080/#/c/10847/8/be/src/util/minidump.cc@101 PS8, Line 101: LOG(INFO) << "Caught signal: SIGUSR1" << endl; This should get the signal from the parameter. You could use the same log message format that you use for SIGTERM here, too. 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 signal: SIGTERM. Daemon will exit', > Checked the code for exit of the process, could not find the function where Can you think of ways that we could add this to the code? Maybe we can pass an expected exit code to kill_cluster()? http://gerrit.cloudera.org:8080/#/c/10847/8/tests/custom_cluster/test_breakpad.py File tests/custom_cluster/test_breakpad.py: http://gerrit.cloudera.org:8080/#/c/10847/8/tests/custom_cluster/test_breakpad.py@239 PS8, Line 239: self.assert_impalad_log_contains('INFO', 'Caught signal: SIGTERM. Daemon will exit', Can you test that the log also contains the sending PID? -- 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: 8 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Mon, 06 Aug 2018 17:38:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 8: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/185/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 8 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Sat, 04 Aug 2018 00:03:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Impala Public Jenkins 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 7: Build Failed https://jenkins.impala.io/job/gerrit-code-review-checks/184/ : Initial code review checks failed. See linked job for details on the failure. -- 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: 7 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 03 Aug 2018 23:46:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Lars Volker, Zoram Thanga, Todd Lipcon, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#8). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/common/init.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 3 files changed, 50 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/8 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 8 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Lars Volker, Zoram Thanga, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#7). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/common/init.cc M be/src/statestore/statestored-main.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 4 files changed, 50 insertions(+), 3 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/7 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 7 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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: (9 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"? Done 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. Da Changed the log message and removed the endl , looks like the messages are being flushed perhaps by atexit hook as you say. Since the testcase check for the message "Caught SIGTERM" which seems to be working fine. 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, to This exit will ensure a normal process termination, if I don't use it the process will not be terminated. 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. Done http://gerrit.cloudera.org:8080/#/c/10847/6/be/src/catalog/catalogd-main.cc@125 PS6, Line 125: sigaction(SIGTERM, , nullptr); > why not set this earlier? seems like there could be a relatively long start Done 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? Removed the code block and moved it to the common handler 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 Done 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 the Done 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? Checked the code for exit of the process, could not find the function where the exit of the process is checked. -- 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 Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 03 Aug 2018 23:29:51 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
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 Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 31 Jul 2018 18:01:32 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Zoram Thanga 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 5: Code-Review+1 LGTM -- 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: 5 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Tue, 17 Jul 2018 17:15:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 5: > Can you also add test case to look for the SIGTERM induced > 'shutdown log' in the log file? Since that's the whole point of > adding the handler, I think it would be good to ensure we don't > lose the log message. Added the code to check for log message before and after. -- 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: 5 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Thu, 12 Jul 2018 01:08:45 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Lars Volker, Zoram Thanga, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#5). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/service/impalad-main.cc M be/src/statestore/statestored-main.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 5 files changed, 77 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/5 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 5 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Zoram Thanga 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 4: Can you also add test case to look for the SIGTERM induced 'shutdown log' in the log file? Since that's the whole point of adding the handler, I think it would be good to ensure we don't lose the log message. -- 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: 4 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Wed, 11 Jul 2018 23:03:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Lars Volker, Zoram Thanga, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#4). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/service/impalad-main.cc M be/src/statestore/statestored-main.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 5 files changed, 67 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/4 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 4 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Lars Volker, Zoram Thanga, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#3). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shut down. This change logs a message when SIGTERM is received by impalad/catalogd/statestored. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shut down (some other signal was generated that led to impalad/catalogd/statestored crash). Testing: --- a) Used kill to send signals to impalad/catalogd/statestored `kill -s SIGTERM ` and see the log message is being logged in impalad/catalogd/statestored.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. d) Added new test in test_breakpad.py to handle SIGTERM for impalad/statestored/catalogd. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/catalog/catalogd-main.cc M be/src/service/impalad-main.cc M be/src/statestore/statestored-main.cc M be/src/util/minidump.cc M tests/custom_cluster/test_breakpad.py 5 files changed, 67 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/3 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 3 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 2: (9 comments) http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@9 PS2, Line 9: Currently Impalad does not log any message when SIGTERM is sent to impalad > nit: flush left Done http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@11 PS2, Line 11: recieved > nit: typo Done http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@12 PS2, Line 12: shutdown > nit: two words Done http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@14 PS2, Line 14: Testing: > This change should have an automatic test, possibly in custom-cluster-tests Done http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc File be/src/service/impalad-main.cc: http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@62 PS2, Line 62: static void HandlerTerm(int signum) > nit: formatting ({ on same line). You can run clang-format with the config Done http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@64 PS2, Line 64:LOG(INFO) << "SIGTERM encountered invoking default handler system going down" << endl; > I would change the wording to "Caught SIGTERM. Daemon going down." Done http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@108 PS2, Line 108: old_action > Do we currently set a handler for SIGTERM elsewhere? If not, we should remo No we don't handle the SIGTERM elsewhere I have removed the old_action now I exit the process after handling SIGTERM http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@108 PS2, Line 108: old_action > Please add a comment explaining why SIGTERM is specifically handled (i.e., Done http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/util/minidump.cc File be/src/util/minidump.cc: http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/util/minidump.cc@101 PS2, Line 101: recieved > nit: typo Done -- 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: 2 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Wed, 11 Jul 2018 21:33:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Zoram Thanga 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 2: (2 comments) http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc File be/src/service/impalad-main.cc: http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@64 PS2, Line 64:LOG(INFO) << "SIGTERM encountered invoking default handler system going down" << endl; I would change the wording to "Caught SIGTERM. Daemon going down." http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@108 PS2, Line 108: old_action > Do we currently set a handler for SIGTERM elsewhere? If not, we should remo Please add a comment explaining why SIGTERM is specifically handled (i.e., we want to log a message when Impalad/statestored/catalogd is being shut down via the signal for whatever reason). -- 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: 2 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Reviewer: Zoram Thanga Gerrit-Comment-Date: Fri, 06 Jul 2018 23:56:02 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
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 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@9 PS2, Line 9: Currently Impalad does not log any message when SIGTERM is sent to impalad nit: flush left http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@11 PS2, Line 11: recieved nit: typo http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@12 PS2, Line 12: shutdown nit: two words http://gerrit.cloudera.org:8080/#/c/10847/2//COMMIT_MSG@14 PS2, Line 14: Testing: This change should have an automatic test, possibly in custom-cluster-tests. It should cover all of impalad, statestored, and catalogd. http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc File be/src/service/impalad-main.cc: http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@62 PS2, Line 62: static void HandlerTerm(int signum) nit: formatting ({ on same line). You can run clang-format with the config in .clang-format. http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/service/impalad-main.cc@108 PS2, Line 108: old_action Do we currently set a handler for SIGTERM elsewhere? If not, we should remove this. http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/util/minidump.cc File be/src/util/minidump.cc: http://gerrit.cloudera.org:8080/#/c/10847/2/be/src/util/minidump.cc@101 PS2, Line 101: recieved nit: typo -- 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: 2 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Comment-Date: Thu, 05 Jul 2018 23:59:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Hello Lars Volker, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/10847 to look at the new patch set (#2). Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not log any message when SIGTERM is sent to impalad to terminate or to do a graceful shutdown. This change logs a message when SIGTERM is recieved by impalad. This logging will assist in debugging the issues seen in the field where impalad was not gracefully shutdown. Testing: --- a) Used kill to send signals to impalad `kill -s SIGTERM ` and see the message is being logged in impalad.INFO. b) Ran test_breakpad.py to check that existing breakpad functionalities are not affected. c) Ran exhaustive tests without failure. Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/service/impalad-main.cc M be/src/util/minidump.cc 2 files changed, 14 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/2 -- 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: newpatchset Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 2 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh 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 1: > How does your change interact with the Breakpad integration? > SIGSEGV and SIGABRT are also handled by the Breakpad signal handler > (https://chromium.googlesource.com/breakpad/breakpad/+/master/src/client/linux/handler/exception_handler.cc#115). > > It should have a test, possibly in the custom-cluster-tests folder, > to validate that Impala does the expected for each signal it could > receive. > > An alternative approach could be to print the received signal into > the logs when writing a minidump. I'm resorting to default handlers after the message has been dumped but anyways I'll test it with breakpad and see if it affects the core/minidump creation -- 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: 1 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Reviewer: Pranay Singh Gerrit-Comment-Date: Mon, 02 Jul 2018 16:54:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
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 1: How does your change interact with the Breakpad integration? SIGSEGV and SIGABRT are also handled by the Breakpad signal handler (https://chromium.googlesource.com/breakpad/breakpad/+/master/src/client/linux/handler/exception_handler.cc#115). It should have a test, possibly in the custom-cluster-tests folder, to validate that Impala does the expected for each signal it could receive. An alternative approach could be to print the received signal into the logs when writing a minidump. -- 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: 1 Gerrit-Owner: Pranay Singh Gerrit-Reviewer: Lars Volker Gerrit-Comment-Date: Mon, 02 Jul 2018 16:48:08 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Pranay Singh has uploaded this change for review. ( http://gerrit.cloudera.org:8080/10847 Change subject: IMPALA-6271: Impala daemon should log a message when it's being shut down .. IMPALA-6271: Impala daemon should log a message when it's being shut down Currently Impalad does not have signal handlers, so it's hard to triage issues related to impala crash as to what caused them. This change adds signal handlers that logs the name of the signals encountered. Testing: Used kill to send signals to impalad kill -s SIGTERM kill -s SIGQUIT kill -s SIGSEGV kill -s SIGABRT Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 --- M be/src/service/impalad-main.cc 1 file changed, 34 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/47/10847/1 -- 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: newchange Gerrit-Change-Id: Id20da9e30440b7348557beccb8a0da14775fcc29 Gerrit-Change-Number: 10847 Gerrit-PatchSet: 1 Gerrit-Owner: Pranay Singh