Adar Dembo has posted comments on this change. Change subject: subprocess: add ScopedSubprocess ......................................................................
Patch Set 1: (2 comments) http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/integration-tests/full_stack-insert-scan-test.cc File src/kudu/integration-tests/full_stack-insert-scan-test.cc: PS1, Line 232: string cmd = Substitute("perf record --pid=$0", getpid()); : if (FLAGS_perf_fp_flag) cmd += " --call-graph fp"; > I think the original code here intended that you should pass --perf_fp_flag What systems are those? On the two that I tested (Ubuntu 16 and el6.6) "perf record --call-graph" complained that --call-graph requires a value (i.e. " fp"). http://gerrit.cloudera.org:8080/#/c/6741/1/src/kudu/util/subprocess.h File src/kudu/util/subprocess.h: Line 175: // Subprocess that is automatically killed when it goes out of scope. > I'm not entirely following this, cuz Subprocess itself already does automat MakeScopedCleanup() doesn't work so well if you want to embed the cleanup itself in another object (the type is tricky to deduce which is why we use 'auto' everywhere). You can see an example of that in my next patch. But, you're right that this does reproduce much of ~Subprocess(). I'll change this to 1) a way to customize the signal sent in ~Subprocess, and 2) a fallback to SIGKILL if the custom signal takes too long to deliver. -- To view, visit http://gerrit.cloudera.org:8080/6741 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iaf31bd4ea6de2917521a9852714fb33cfbec1f61 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Adar Dembo <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
