Harshil has posted comments on this change. ( http://gerrit.cloudera.org:8080/13345 )
Change subject: IMPALA-8560: Prometheus metrics support in Impala ...................................................................... Patch Set 6: (6 comments) http://gerrit.cloudera.org:8080/#/c/13345/6/be/src/util/collection-metrics.h File be/src/util/collection-metrics.h: http://gerrit.cloudera.org:8080/#/c/13345/6/be/src/util/collection-metrics.h@184 PS6, Line 184: static_cast<uint64_t>(boost::accumulators::min(acc_)), unit_) > These casts all seem unnecessary - ConvertToPrometheusSecs should work for Done http://gerrit.cloudera.org:8080/#/c/13345/6/be/src/util/metrics.h File be/src/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/13345/6/be/src/util/metrics.h@116 PS6, Line 116: inline > Inline not needed. Done http://gerrit.cloudera.org:8080/#/c/13345/6/be/src/util/metrics.h@117 PS6, Line 117: if (type == TUnit::type::TIME_MS || type == TUnit::type::TIME_US > if isn't needed - just Done http://gerrit.cloudera.org:8080/#/c/13345/5/be/src/util/metrics.h File be/src/util/metrics.h: http://gerrit.cloudera.org:8080/#/c/13345/5/be/src/util/metrics.h@101 PS5, Line 101: /// https://github.com/prometheus/docs/blob/master/content/docs/instrumenting/ > line too long (101 > 90) Done http://gerrit.cloudera.org:8080/#/c/13345/5/be/src/util/metrics.h@112 PS5, Line 112: > ok thanks Tim. Done http://gerrit.cloudera.org:8080/#/c/13345/5/be/src/util/metrics.h@191 PS5, Line 191: , TUnit > TIME_MS Done -- To view, visit http://gerrit.cloudera.org:8080/13345 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5349085a2007b568cb97f9b8130804ea64d7bb08 Gerrit-Change-Number: 13345 Gerrit-PatchSet: 6 Gerrit-Owner: Harshil <[email protected]> Gerrit-Reviewer: Harshil <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Lars Volker <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 28 May 2019 17:14:11 +0000 Gerrit-HasComments: Yes
