Todd Lipcon has posted comments on this change. Change subject: Add CPU usage and context switch metrics ......................................................................
Patch Set 1: (5 comments) Nice idea, just a couple small nits. Dan, can you take a look and see if these calls work on OSX or whether we need to wrap in some #ifdef? http://gerrit.cloudera.org:8080/#/c/1751/1/src/kudu/util/thread.cc File src/kudu/util/thread.cc: Line 70: "CPU utime", can you expect this out to "User CPU Time" Line 75: "CPU stime", ... and "System CPU Time" Line 85: "Involuntary context switches", nit: capitalize 'Context Switches' here and above on line 80 Line 87: "Total involuntary context switches"); I think the above four should also have 'EXPOSE_AS_COUNTER' since they're values that should only increase. Line 93: getrusage(RUSAGE_SELF, &ru); can you wrap the getrusage calls in 'CHECK_ERR()'? that's a glog macro that will make sure they return 0. I don't see any reason it would fail, but better to have the assertion. -- To view, visit http://gerrit.cloudera.org:8080/1751 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic28f4813999c6cf9af6781cdfba9097bc05727e0 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Binglin Chang <[email protected]> Gerrit-Reviewer: Internal Jenkins Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
