Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8375 )
Change subject: KUDU-2162 Expose stats about scan filters ...................................................................... Patch Set 3: (4 comments) The test failures still need to be addressed. http://gerrit.cloudera.org:8080/#/c/8375/3/src/kudu/tserver/scanners.h File src/kudu/tserver/scanners.h: http://gerrit.cloudera.org:8080/#/c/8375/3/src/kudu/tserver/scanners.h@424 PS3, Line 424: MonoDelta time_since_start = start_time.Initialized() ? (now - start_time) : MonoDelta::FromSeconds(0); Nit: line too long, please wrap. http://gerrit.cloudera.org:8080/#/c/8375/3/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: http://gerrit.cloudera.org:8080/#/c/8375/3/src/kudu/tserver/tablet_service.cc@702 PS3, Line 702: void SetResourceMetrics(rpc::RpcContext* context, Since context isn't being modified, pass it by cref. Likewise, can this function be made const? Or does the mutation of 'metrics' prevent that? http://gerrit.cloudera.org:8080/#/c/8375/3/src/kudu/tserver/tablet_service.cc@712 PS3, Line 712: MonoDelta duration = scan_rpc_metrics.GetScanDurationAndTimeSinceStart(ScanState::kActive).first; Line too long, please wrap. http://gerrit.cloudera.org:8080/#/c/8375/3/src/kudu/tserver/tablet_service.cc@719 PS3, Line 719: private: Nit: indent by one character. -- 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: 3 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 08:43:06 +0000 Gerrit-HasComments: Yes
