[email protected] 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)

I pushed a patch with the latest changes to https://gerrit.cloudera.org/c/8375, 
we can continue with the review there.

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:
Done


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.
Done


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?
Done


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 ScanRe
Done


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'.
Done



--
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: Sun, 10 Nov 2019 07:25:21 +0000
Gerrit-HasComments: Yes

Reply via email to