You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by "Lars Volker (Code Review)" <ge...@cloudera.org> on 2016/06/02 12:31:13 UTC

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Lars Volker has uploaded a new change for review.

  http://gerrit.cloudera.org:8080/3285

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................

IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

When hitting a DCHECK/CHECK the daemons do not write minidumps. This
is caused by glog's own stack unwinding mechanism, which catches
SIGABRT and removes all other handlers before aborting.

This patch backports a change from glog, which only resets the
SIGABRT handler, if it is the one installed by glog itself.

https://github.com/google/glog/commit/cda16b3443e2d6ef88cdbbe10b9a11adea6f33fe

Testing:
Did a local build and made sure that both the DCHECK error are logged
and minidumps are written.

Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
---
M buildall.sh
A source/glog/glog-0.3.2-patches/0002-Preserve-custom-signal-handler-for-dcheck.patch
2 files changed, 87 insertions(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Toolchain refs/changes/85/3285/1
-- 
To view, visit http://gerrit.cloudera.org:8080/3285
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................


Patch Set 1:

I looked at glog 0.3.3 and 0.3.4, but neither contains the change we need, so we would need to pick it anyways.

This largely affects debug builds, so having it in 5.8 would allow us to make use of breakpad in tests running debug builds, such as stress tests. However it does not look critical in terms of the released bits.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-HasComments: No

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................


Patch Set 1:

Here is the jenkins test run to validate this change: http://unittest.jenkins.cloudera.com/job/verify-impala-toolchain-package-build/203/

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-HasComments: No

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has submitted this change and it was merged.

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................


IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

When hitting a DCHECK/CHECK the daemons do not write minidumps. This
is caused by glog's own stack unwinding mechanism, which catches
SIGABRT and removes all other handlers before aborting.

This patch backports a change from glog, which only resets the
SIGABRT handler, if it is the one installed by glog itself.

https://github.com/google/glog/commit/cda16b3443e2d6ef88cdbbe10b9a11adea6f33fe

Testing:
Did a local build and made sure that both the DCHECK error are logged
and minidumps are written.

Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
---
M buildall.sh
A source/glog/glog-0.3.2-patches/0002-Preserve-custom-signal-handler-for-dcheck.patch
2 files changed, 87 insertions(+), 0 deletions(-)

Approvals:
  Lars Volker: Verified
  Matthew Jacobs: Looks good to me, approved



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

Gerrit-MessageType: merged
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 2
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Posted by "Henry Robinson (Code Review)" <ge...@cloudera.org>.
Henry Robinson has posted comments on this change.

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................


Patch Set 1:

Out of interest, did you consider upgrading glog? Are you hoping that this gets into 5.8?

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-HasComments: No

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has uploaded a new patch set (#2).

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................

IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

When hitting a DCHECK/CHECK the daemons do not write minidumps. This
is caused by glog's own stack unwinding mechanism, which catches
SIGABRT and removes all other handlers before aborting.

This patch backports a change from glog, which only resets the
SIGABRT handler, if it is the one installed by glog itself.

https://github.com/google/glog/commit/cda16b3443e2d6ef88cdbbe10b9a11adea6f33fe

Testing:
Did a local build and made sure that both the DCHECK error are logged
and minidumps are written.

Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
---
M buildall.sh
A source/glog/glog-0.3.2-patches/0002-Preserve-custom-signal-handler-for-dcheck.patch
2 files changed, 87 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Toolchain refs/changes/85/3285/2
-- 
To view, visit http://gerrit.cloudera.org:8080/3285
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 2
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................


Patch Set 1:

(1 comment)

Thanks for the review, please see PS2. Another look at the glog change won't hurt, but given it has already been reviews upstream it should be fine.

http://gerrit.cloudera.org:8080/#/c/3285/1/buildall.sh
File buildall.sh:

PS1, Line 138: GFLAGS_VERSION=2.0 GLOG_VERSION=0.3.2-p2 $SOURCE_DIR/source/glog/build.sh
> Rather than editing this line, I'd leave the existing call to build with p1
Good point, I added it back.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................


Patch Set 1:

(1 comment)

I didn't look at the glog delta itself. Let me know if you'd like me to.

http://gerrit.cloudera.org:8080/#/c/3285/1/buildall.sh
File buildall.sh:

PS1, Line 138: GFLAGS_VERSION=2.0 GLOG_VERSION=0.3.2-p2 $SOURCE_DIR/source/glog/build.sh
Rather than editing this line, I'd leave the existing call to build with p1 in place and then have a line below which does this for p2. You can see we do this for other patch versions elsewhere. Right now we have buildall.sh literally build ALL (all released versions at least). It's a bit silly, but this is kind of the source of truth (rather than the s3 bucket which is just the result of these scripts) for all binaries anyone might need from native-toolchain. That's bad for other reasons, but all of this is a longer conversation.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 1
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: Yes

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Posted by "Matthew Jacobs (Code Review)" <ge...@cloudera.org>.
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................


Patch Set 2: Code-Review+2

LGTM, +2 assuming you've run the jenkins job to test the build (PUBLISH=0). Also locally that you've tested Impala against the new glog build.

After that, you can submit this and run the jenkins job w/ PUBLISH=1, then we can get your Impala-side change in.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 2
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No

[Toolchain-CR] IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps

Posted by "Lars Volker (Code Review)" <ge...@cloudera.org>.
Lars Volker has posted comments on this change.

Change subject: IMPALA-3656: Hitting DCHECK/CHECK does not write minidumps
......................................................................


Patch Set 2: Verified+1

Yes, I validated it with this jenkins run: http://unittest.jenkins.cloudera.com/job/verify-impala-toolchain-package-build/203/

I also verified locally that it works as intended.

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

Gerrit-MessageType: comment
Gerrit-Change-Id: I97f528f3190b2f09cd2a135551c618d0e501b7a0
Gerrit-PatchSet: 2
Gerrit-Project: Toolchain
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Henry Robinson <he...@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv...@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj...@cloudera.com>
Gerrit-HasComments: No