Todd Lipcon has posted comments on this change. Change subject: KUDU-1444. Get resource metrics of a scan. ......................................................................
Patch Set 5: (7 comments) http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/client/client.h File src/kudu/client/client.h: Line 29: #include "kudu/client/resource_metrics.h" nit: please sort imports http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/client/resource_metrics-internal.h File src/kudu/client/resource_metrics-internal.h: Line 39: void Increment(std::string name, int64_t amount); this can be a const std::string&, or perhaps a StringPiece Line 42: int64_t GetMetric(const std::string name) const; same http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/client/resource_metrics.h File src/kudu/client/resource_metrics.h: Line 40: void Increment(std::string name, int64_t amount); const std::string& Line 43: int64_t GetMetric(const std::string name) const; same http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/client/scanner-internal.cc File src/kudu/client/scanner-internal.cc: Line 156: resource_metrics_.Increment(field->name(), reflection->GetInt64(resource_metrics, field)); this should probably check whether the field is set (reflection->HasField(resource_metrics, field)), before doing the Increment http://gerrit.cloudera.org:8080/#/c/3013/5/src/kudu/tserver/tablet_service.cc File src/kudu/tserver/tablet_service.cc: Line 88: extern const char* CFILE_CACHE_MISS_BYTES_METRIC_NAME; nit: dont indent inside namespaces -- To view, visit http://gerrit.cloudera.org:8080/3013 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Iedaf570a7601651c93275ae0a8565f1e33da842d Gerrit-PatchSet: 5 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: zhen.zhang <zhangz...@xiaomi.com> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-Reviewer: zhen.zhang <zhangz...@xiaomi.com> Gerrit-HasComments: Yes
