[email protected] has posted comments on this change. ( http://gerrit.cloudera.org:8080/8375 )
Change subject: KUDU-2162 Expose stats about scan filters ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/8375/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8375/1//COMMIT_MSG@11 PS1, Line 11: - duration scanner was open in nanoseconds > we should also expose a client-calculated metric which is the amount of tim Would it make sense to return two metrics based on the rpc's InboundCallTiming total duration (time_completed - time_received) and wait duration ( time_handled - time_received) ? http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java File java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java: http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@38 PS1, Line 38: */ > needs interface annotation Done http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@60 PS1, Line 60: * @return the value of the named metric; if the metric is not found, returns 0 > nit: can you use get() and then check the result vs null to avoid the doubl Done http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@63 PS1, Line 63: Long metricValue = metrics.get(name); > javadoc style Done http://gerrit.cloudera.org:8080/#/c/8375/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@77 PS1, Line 77: } > same Done http://gerrit.cloudera.org:8080/#/c/8375/1/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/8375/1/src/kudu/tserver/tablet_service.cc@1246 PS1, Line 1246: tablet_manager_(tablet_manager) { > I think we should also include CPU time, because that can help determine if Done http://gerrit.cloudera.org:8080/#/c/8375/1/src/kudu/tserver/tserver.proto File src/kudu/tserver/tserver.proto: http://gerrit.cloudera.org:8080/#/c/8375/1/src/kudu/tserver/tserver.proto@334 PS1, Line 334: // > doc this to be more specific on whether this includes data that was filtere Done -- To view, visit http://gerrit.cloudera.org:8080/8375 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id30a7e82357fe2fc28f6d316378a612af43d8c96 Gerrit-Change-Number: 8375 Gerrit-PatchSet: 2 Gerrit-Owner: Will Berkeley <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Sun, 10 Nov 2019 07:54:53 +0000 Gerrit-HasComments: Yes
