Todd Lipcon has posted comments on this change.

Change subject: Add Google Breakpad support to Kudu
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/5620/1//COMMIT_MSG
Commit Message:

Line 28:   because breakpad does not properly namespace its includes (however it
> Yes, directory prefix. I actually wrote a script to do preprocessing of the
I'm pro-hack here because I think the possibility for include conflicts is kind 
of high. I also like the simplicity that -I$PREFIX/include/ is sufficient to 
get the thirdparty stuff properly.


http://gerrit.cloudera.org:8080/#/c/5620/1/src/kudu/util/minidump.cc
File src/kudu/util/minidump.cc:

PS1, Line 220: CreateDirsRecursively
> Personally I think it makes sense because they can specify an arbitrary pat
right, but for other paths (log dir, data dirs, etc) we don't recursively 
create them. I think typically our policy should be "we can create our 
configured directories and things below them, but nothing above them". i.e if 
you configure log_dir=/tmp/log that's OK and we'll create it, but if you 
configure log_dir=/data/3/kudu-logs/ and it turns out data/3 doesn't exist, we 
shouldn't go and create it (because it's probably a typo and we'd end up on an 
incorrect mount point, etc).

I seem to recall way back when (and maybe still today) that HDFS would always 
recursively create the configured data directories. It also didn't strip 
leading whitespace from the configuration value. So I accidentally configured a 
data dir to " /home/todd/foo/bar" and that turned into a relative path 
"/my/working/directory/\ /home/todd/foo/bar" and it took me an hour to figure 
out where my data was getting written. Maybe the example is a little contrived, 
but hopefully you can see the point.


-- 
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: 1
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

Reply via email to