You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Mike Percy (Code Review)" <ge...@cloudera.org> on 2017/01/19 08:22:01 UTC
[kudu-CR] thirdparty: patch glog for better breakpad support
Hello Adar Dembo,
I'd like you to do a code review. Please visit
http://gerrit.cloudera.org:8080/5742
to review the following change.
Change subject: thirdparty: patch glog for better breakpad support
......................................................................
thirdparty: patch glog for better breakpad support
This patch is a backport of glog commit
cda16b3443e2d6ef88cdbbe10b9a11adea6f33fe which was committed after the
latest release (0.3.4).
Subject: Reset SIGABRT action only if FailureSignalHandler is installed.
Without this patch applied, CHECK() failures will not generate a
minidump. With the patch, CHECK() first displays a stack trace and then
generates a minidump.
Change-Id: I86b5649fcd11dd083e6a64bf9ba088437ff49b8f
---
M thirdparty/download-thirdparty.sh
A thirdparty/patches/glog-cda16b3443e2d6ef88cdbbe10b9a11adea6f33fe.patch
2 files changed, 96 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/42/5742/1
--
To view, visit http://gerrit.cloudera.org:8080/5742
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I86b5649fcd11dd083e6a64bf9ba088437ff49b8f
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
[kudu-CR] thirdparty: patch glog for better breakpad support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: thirdparty: patch glog for better breakpad support
......................................................................
Patch Set 2:
> > Still trying to wrap my head around who among breakpad and glog is
> > responsible for which signals...
>
> The way I implemented it in the following patch, the minidump
> signal handler is called first and the glog signal handler is
> called second.
Right, but for which signals? Just SIGABRT? AFAICT, glog handles SIGSEGV, SIGILL, SIGFPE, SIGABRT, SIGBUS, and SIGTERM.
--
To view, visit http://gerrit.cloudera.org:8080/5742
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I86b5649fcd11dd083e6a64bf9ba088437ff49b8f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] thirdparty: patch glog for better breakpad support
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: thirdparty: patch glog for better breakpad support
......................................................................
Patch Set 2:
> Still trying to wrap my head around who among breakpad and glog is
> responsible for which signals...
The way I implemented it in the following patch, the minidump signal handler is called first and the glog signal handler is called second.
--
To view, visit http://gerrit.cloudera.org:8080/5742
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I86b5649fcd11dd083e6a64bf9ba088437ff49b8f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] thirdparty: patch glog for better breakpad support
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has posted comments on this change.
Change subject: thirdparty: patch glog for better breakpad support
......................................................................
Patch Set 2:
> Right, but for which signals? Just SIGABRT? AFAICT, glog handles
> SIGSEGV, SIGILL, SIGFPE, SIGABRT, SIGBUS, and SIGTERM.
Good question. I wrote a test to verify this as part of the breakpad patch. Minidumps are produced for all of the above, except SIGTERM (by design, since SIGTERM is for graceful shutdown).
Stack traces are produced for all of the above signals.
--
To view, visit http://gerrit.cloudera.org:8080/5742
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I86b5649fcd11dd083e6a64bf9ba088437ff49b8f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-HasComments: No
[kudu-CR] thirdparty: patch glog for better breakpad support
Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.
Adar Dembo has posted comments on this change.
Change subject: thirdparty: patch glog for better breakpad support
......................................................................
Patch Set 2: Code-Review+2
Still trying to wrap my head around who among breakpad and glog is responsible for which signals...
--
To view, visit http://gerrit.cloudera.org:8080/5742
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I86b5649fcd11dd083e6a64bf9ba088437ff49b8f
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: No
[kudu-CR] thirdparty: patch glog for better breakpad support
Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.
Mike Percy has submitted this change and it was merged.
Change subject: thirdparty: patch glog for better breakpad support
......................................................................
thirdparty: patch glog for better breakpad support
This patch is a backport of glog commit
cda16b3443e2d6ef88cdbbe10b9a11adea6f33fe which was committed after the
latest release (0.3.4).
Subject: Reset SIGABRT action only if FailureSignalHandler is installed.
Without this patch applied, CHECK() failures will not generate a
minidump. With the patch, CHECK() first displays a stack trace and then
generates a minidump.
Change-Id: I86b5649fcd11dd083e6a64bf9ba088437ff49b8f
Reviewed-on: http://gerrit.cloudera.org:8080/5742
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
M thirdparty/download-thirdparty.sh
A thirdparty/patches/glog-cda16b3443e2d6ef88cdbbe10b9a11adea6f33fe.patch
2 files changed, 96 insertions(+), 1 deletion(-)
Approvals:
Adar Dembo: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/5742
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I86b5649fcd11dd083e6a64bf9ba088437ff49b8f
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>