Lars Volker has posted comments on this change. Change subject: IMPALA-2686: Add breakpad crash handler to all daemons ......................................................................
Patch Set 6: (4 comments) Thanks for the comments. Please see PS8. I talked to Casey about writing tests and he recommended to write a custom_cluster test. http://gerrit.cloudera.org:8080/#/c/2028/6/be/src/util/minidump.cc File be/src/util/minidump.cc: Line 26: TODO > ? Done Line 51: printf("Wrote minidump to %s\n", descriptor.path()); > It would also be helpful to write to stderr so we get it in impalad.ERROR ( Done Line 69: string pattern = FLAGS_minidump_path + "/" + > Why do we need to match this exact pattern? Presumably anything with a .dmp Yes, changed it. Line 79: // To prevent us from messing with a file that is currently being written we only > Why would a file be being written? This is only checked during startup and The cases I had in mind at the time were multiple daemons writing to the same folder. After moving each into their own folder and disabling periodic cleanups it only leaves test setups where multiple impalads run on a single machine where one could startup while another one crashes. However that seems very unlikely, so I removed that part of the code. -- To view, visit http://gerrit.cloudera.org:8080/2028 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7a37a38488716ffe34296f3490ae291bbb7228d6 Gerrit-PatchSet: 6 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Casey Ching <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Silvius Rus <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-HasComments: Yes
