[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Todd Lipcon has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9319 ) Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing Previously we used glog's wrapper around libunwind for stack tracing. However that has a deficiency that it assumes that, process wide, only one thread can be inside libunwind at a time[1] It appears that this is left over from some very old versions of libunwind, or was already unnecessarily conservative. libunwind is meant to be thread safe, and we have tests that will trigger if it is not. This just extracts the function body of the glog function we were using and does the same work manually. Without this fix, the "collect from all the threads at the same time" code path resulted in most of the threads collecting an empty trace since they tried to call libunwind at the same time. [1] https://github.com/google/glog/issues/298 Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Reviewed-on: http://gerrit.cloudera.org:8080/9319 Reviewed-by: Mike Percy Tested-by: Kudu Jenkins --- M build-support/iwyu/iwyu.sh A build-support/iwyu/mappings/libunwind.imp M src/kudu/util/debug-util-test.cc M src/kudu/util/debug-util.cc M src/kudu/util/debug-util.h 5 files changed, 81 insertions(+), 17 deletions(-) Approvals: Mike Percy: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 9 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9319 to look at the new patch set (#8). Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing Previously we used glog's wrapper around libunwind for stack tracing. However that has a deficiency that it assumes that, process wide, only one thread can be inside libunwind at a time[1] It appears that this is left over from some very old versions of libunwind, or was already unnecessarily conservative. libunwind is meant to be thread safe, and we have tests that will trigger if it is not. This just extracts the function body of the glog function we were using and does the same work manually. Without this fix, the "collect from all the threads at the same time" code path resulted in most of the threads collecting an empty trace since they tried to call libunwind at the same time. [1] https://github.com/google/glog/issues/298 Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 --- M build-support/iwyu/iwyu.sh A build-support/iwyu/mappings/libunwind.imp M src/kudu/util/debug-util-test.cc M src/kudu/util/debug-util.cc M src/kudu/util/debug-util.h 5 files changed, 81 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/9319/8 -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9319 ) Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. Patch Set 8: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/9319/7/src/kudu/util/debug-util.cc File src/kudu/util/debug-util.cc: http://gerrit.cloudera.org:8080/#/c/9319/7/src/kudu/util/debug-util.cc@544 PS7, Line 544: skip_frames++; // Do not include the "Collect" frame > I believe it did, didn't it? Ah, I just looked and the other calls are no-arg, contrary to what I thought. Thanks for checking. -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 21 Feb 2018 19:24:45 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/9319 ) Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/9319/7/src/kudu/util/debug-util.cc File src/kudu/util/debug-util.cc: http://gerrit.cloudera.org:8080/#/c/9319/7/src/kudu/util/debug-util.cc@544 PS7, Line 544: skip_frames++; // Do not include the "Collect" frame > Since we are doing this here now, should this patch also modify all existin I believe it did, didn't it? -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 21 Feb 2018 09:19:50 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9319 ) Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. Patch Set 7: (1 comment) http://gerrit.cloudera.org:8080/#/c/9319/7/src/kudu/util/debug-util.cc File src/kudu/util/debug-util.cc: http://gerrit.cloudera.org:8080/#/c/9319/7/src/kudu/util/debug-util.cc@544 PS7, Line 544: skip_frames++; // Do not include the "Collect" frame Since we are doing this here now, should this patch also modify all existing callers of Collect() to reduce the skipped frames by one? -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 21 Feb 2018 08:11:19 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9319 to look at the new patch set (#7). Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing Previously we used glog's wrapper around libunwind for stack tracing. However that has a deficiency that it assumes that, process wide, only one thread can be inside libunwind at a time[1] It appears that this is left over from some very old versions of libunwind, or was already unnecessarily conservative. libunwind is meant to be thread safe, and we have tests that will trigger if it is not. This just extracts the function body of the glog function we were using and does the same work manually. Without this fix, the "collect from all the threads at the same time" code path resulted in most of the threads collecting an empty trace since they tried to call libunwind at the same time. [1] https://github.com/google/glog/issues/298 Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 --- M build-support/iwyu/iwyu.sh A build-support/iwyu/mappings/libunwind.imp M src/kudu/util/debug-util-test.cc M src/kudu/util/debug-util.cc M src/kudu/util/debug-util.h 5 files changed, 81 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/9319/7 -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 7 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9319 to look at the new patch set (#5). Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing Previously we used glog's wrapper around libunwind for stack tracing. However that has a deficiency that it assumes that, process wide, only one thread can be inside libunwind at a time[1] It appears that this is left over from some very old versions of libunwind, or was already unnecessarily conservative. libunwind is meant to be thread safe, and we have tests that will trigger if it is not. This just extracts the function body of the glog function we were using and does the same work manually. Without this fix, the "collect from all the threads at the same time" code path resulted in most of the threads collecting an empty trace since they tried to call libunwind at the same time. [1] https://github.com/google/glog/issues/298 Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 --- M build-support/iwyu/iwyu.sh A build-support/iwyu/mappings/libunwind.imp M src/kudu/util/debug-util-test.cc M src/kudu/util/debug-util.cc M src/kudu/util/debug-util.h 5 files changed, 81 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/9319/5 -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 5 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9319 to look at the new patch set (#3). Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing Previously we used glog's wrapper around libunwind for stack tracing. However that has a deficiency that it assumes that, process wide, only one thread can be inside libunwind at a time[1] It appears that this is left over from some very old versions of libunwind, or was already unnecessarily conservative. libunwind is meant to be thread safe, and we have tests that will trigger if it is not. This just extracts the function body of the glog function we were using and does the same work manually. Without this fix, the "collect from all the threads at the same time" code path resulted in most of the threads collecting an empty trace since they tried to call libunwind at the same time. [1] https://github.com/google/glog/issues/298 Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 --- M build-support/iwyu/iwyu.sh A build-support/iwyu/mappings/libunwind.imp M src/kudu/util/debug-util-test.cc M src/kudu/util/debug-util.cc M src/kudu/util/debug-util.h 5 files changed, 75 insertions(+), 17 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/9319/3 -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 3 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Hello Will Berkeley, Tidy Bot, Mike Percy, Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/9319 to look at the new patch set (#2). Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing Previously we used glog's wrapper around libunwind for stack tracing. However that has a deficiency that it assumes that, process wide, only one thread can be inside libunwind at a time. It appears that this is left over from some very old versions of libunwind, or was already unnecessarily conservative. libunwind is meant to be thread safe, and we have tests that will trigger if it is not. This just extracts the function body of the glog function we were using and does the same work manually. Without this fix, the "collect from all the threads at the same time" code path resulted in most of the threads collecting an empty trace since they tried to call libunwind at the same time. Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 --- M src/kudu/util/debug-util-test.cc M src/kudu/util/debug-util.cc M src/kudu/util/debug-util.h 3 files changed, 36 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/9319/2 -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 2 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Will Berkeley
[kudu-CR] KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing
Hello Will Berkeley, Mike Percy, I'd like you to do a code review. Please visit http://gerrit.cloudera.org:8080/9319 to review the following change. Change subject: KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing .. KUDU-2291 (part 6) stacks: use libunwind directly for stack tracing Previously we used glog's wrapper around libunwind for stack tracing. However that has a deficiency that it assumes that, process wide, only one thread can be inside libunwind at a time. It appears that this is left over from some very old versions of libunwind, or was already unnecessarily conservative. libunwind is meant to be thread safe, and we have tests that will trigger if it is not. This just extracts the function body of the glog function we were using and does the same work manually. Without this fix, the "collect from all the threads at the same time" code path resulted in most of the threads collecting an empty trace since they tried to call libunwind at the same time. Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 --- M src/kudu/util/debug-util-test.cc M src/kudu/util/debug-util.cc M src/kudu/util/debug-util.h 3 files changed, 33 insertions(+), 11 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/19/9319/1 -- To view, visit http://gerrit.cloudera.org:8080/9319 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I3a53e55d7c4e7ee50bcac5b1e81267df56383634 Gerrit-Change-Number: 9319 Gerrit-PatchSet: 1 Gerrit-Owner: Todd Lipcon Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
