impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Lars Volker (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-2686: Add breakpad crash handler to all daemons
Date Fri, 15 Apr 2016 15:40:56 GMT
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 <lv@cloudera.com>
Gerrit-Reviewer: Casey Ching <casey@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Silvius Rus <srus@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message