David Ribeiro Alves has posted comments on this change.

Change subject: external mini cluster: spawn perf record for each daemon during 
Start()
......................................................................


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6742/1/src/kudu/integration-tests/external_mini_cluster.cc
File src/kudu/integration-tests/external_mini_cluster.cc:

PS1, Line 270:   if (FLAGS_perf_record) {
             :     opts.perf_record_filename =
             :         Substitute("$0/perf-$1.data", opts.log_dir, daemon_id);
             :   }
nit: does it matter that the flag is set? i.e. can't you just set the filename 
always? 3LOC less...


PS1, Line 720: "perf",
             :       "record",
             :       "--call-graph",
             :       "fp",
             :       "-o",
it seems we're now calling this in multiple places. worth a util method that 
just takes the pid?


-- 
To view, visit http://gerrit.cloudera.org:8080/6742
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I92079f616648788b461d550057b8e23bc9174b71
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Adar Dembo <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <[email protected]>
Gerrit-HasComments: Yes

Reply via email to