Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/14652 )
Change subject: KUDU-2162 Expose stats about scan filters ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h File src/kudu/tserver/scanners.h: http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h@408 PS4, Line 408: // ScanMetrics contain metrics that will be sent to the client for each batch of rows Perhaps we should call it ScanRPCMetrics, since: 1) It won't confuse w.r.t. the existing ScannerMetrics 2) It represents quantities for one Scan RPC? http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h@409 PS4, Line 409: struct ScanMetrics { Each unit of indentation should be 2 characters wide, not 4. http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/scanners.h@454 PS4, Line 454: // The cumulative amounts of wall, user cpu, and system cpu time spent on : // this scanner, in seconds. This comment isn't as appropriate as it was before. Could you reword it? http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tablet_service.cc@698 PS4, Line 698: ScanMetrics scan_metrics; Probably doesn't need to be public. If you make SetResourceMetrics a ScanResultCollector member, this could be private. http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tserver_path_handlers.cc File src/kudu/tserver/tserver_path_handlers.cc: http://gerrit.cloudera.org:8080/#/c/14652/4/src/kudu/tserver/tserver_path_handlers.cc@531 PS4, Line 531: std::pair<MonoDelta, MonoDelta> Could replace this with 'auto'. -- To view, visit http://gerrit.cloudera.org:8080/14652 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I127b96a30467ea0abcaf6ed5b4d72df66b2dec55 Gerrit-Change-Number: 14652 Gerrit-PatchSet: 4 Gerrit-Owner: Anonymous Coward <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Anonymous Coward <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Sat, 09 Nov 2019 22:54:42 +0000 Gerrit-HasComments: Yes
