You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Pranay Singh (Code Review)" <ge...@cloudera.org> on 2018/06/29 23:13:57 UTC
[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 <pid of impalad>
kill -s SIGQUIT <pid of impalad>
kill -s SIGSEGV <pid of impalad>
kill -s SIGABRT <pid of impalad>
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
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Sep 2018 18:11:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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, &action, 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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Aug 2018 23:29:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Sep 2018 20:18:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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, &action, 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Aug 2018 23:30:09 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Sep 2018 20:18:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Zoram Thanga (Code Review)" <ge...@cloudera.org>.
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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Jul 2018 17:15:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Thu, 05 Jul 2018 23:59:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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:
(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?
--
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Sep 2018 21:35:57 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Zoram Thanga (Code Review)" <ge...@cloudera.org>.
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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 23:03:39 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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(&action, 0, sizeof(struct sigaction));
: action.sa_sigaction = &HandleSigTerm;
: sigaction(SIGTERM, &action, 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Aug 2018 21:29:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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/770/ : 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 23:17:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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, &action, 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Aug 2018 15:53:17 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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 <lv...@cloudera.com>
Gerrit-Comment-Date: Mon, 02 Jul 2018 16:48:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Aug 2018 19:54:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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, &action, 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Aug 2018 17:43:01 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Sep 2018 20:18:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 03 Aug 2018 23:46:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 07 Aug 2018 22:06:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 23 Aug 2018 15:01:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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, &sig_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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 21 Aug 2018 23:31:43 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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(&action, 0, sizeof(struct sigaction));
: action.sa_sigaction = &HandleSigTerm;
: sigaction(SIGTERM, &action, 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 <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Aug 2018 17:38:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Aug 2018 19:40:20 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Sat, 29 Sep 2018 00:08:23 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 11 Jul 2018 21:33:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Aug 2018 19:08:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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, &action, 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Aug 2018 19:20:53 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Sep 2018 21:37:02 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Comment-Date: Mon, 02 Jul 2018 16:54:50 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 27 Sep 2018 21:53:39 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Thu, 12 Jul 2018 01:08:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Zoram Thanga (Code Review)" <ge...@cloudera.org>.
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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 06 Jul 2018 23:56:02 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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, &action, 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Fri, 24 Aug 2018 16:19:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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 <lv...@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
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Wed, 22 Aug 2018 15:58:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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, &action, 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, &sig_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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Mon, 20 Aug 2018 22:41:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Mon, 27 Aug 2018 15:48:56 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Todd Lipcon (Code Review)" <ge...@cloudera.org>.
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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Tue, 31 Jul 2018 22:04:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad>` 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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Sat, 04 Aug 2018 00:03:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Pranay Singh (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
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 <pid of impalad/catalogd/statestored>` 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 <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
[Impala-ASF-CR] IMPALA-6271: Impala daemon should log a message when it's being shut down
Posted by "Andrew Sherman (Code Review)" <ge...@cloudera.org>.
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 <as...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Pranay Singh
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Reviewer: Zoram Thanga <zo...@cloudera.com>
Gerrit-Comment-Date: Mon, 06 Aug 2018 17:59:14 +0000
Gerrit-HasComments: Yes