Lars Volker has posted comments on this change. Change subject: Add Google Breakpad support to Kudu ......................................................................
Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/5620/3/src/kudu/integration-tests/minidump_generation-itest.cc File src/kudu/integration-tests/minidump_generation-itest.cc: Line 66: You might want to add a test for writing minidumps on SIGUSR1, especially since that would kill the process if the signal handler hasn't been set up correctly. http://gerrit.cloudera.org:8080/#/c/5620/3/src/kudu/util/minidump.cc File src/kudu/util/minidump.cc: PS3, Line 56: nly be written when a daemon exits " : "unexpectedly, This is actually not true anymore (but used to be), even for Impala, since sending SIGUSR1 will trigger a Minidump (I opened IMPALA-4736). Line 207: if (!FLAGS_enable_minidumps || FLAGS_minidump_path.empty()) return Status::OK(); If you skip registering the signal handler for SIGUSR1 here and someone sends that signal, then the process will die. I noticed this while reviewing your code and opened IMPALA-4737 to track it on our end. -- To view, visit http://gerrit.cloudera.org:8080/5620 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I495695cc38b75377f20b0497093a81ed5baa887f Gerrit-PatchSet: 3 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Mike Percy <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dinesh Bhat <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Mike Percy <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
