Lars Volker has posted comments on this change. ( http://gerrit.cloudera.org:8080/9292 )
Change subject: IMPALA-6269: Expose KRPC metrics on debug webpage ...................................................................... Patch Set 13: (6 comments) PS13 contains the rebase. PS14 should address all left-over comments. http://gerrit.cloudera.org:8080/#/c/9292/11/be/src/rpc/impala-service-pool.cc File be/src/rpc/impala-service-pool.cc: http://gerrit.cloudera.org:8080/#/c/9292/11/be/src/rpc/impala-service-pool.cc@71 PS11, Line 71: for (const auto& method : method_infos) { : const string& method_name = method.first; : string payload_size_name = Substitute(" > These lines can be combined into: Done http://gerrit.cloudera.org:8080/#/c/9292/11/be/src/rpc/impala-service-pool.cc@76 PS11, Line 76: > Does this need to be defined in metrics.json too ? No, since it is not added to any metric groups and thus also not displayed on /metrics. I think we can add it there, but it would be inconsistent with the KRPC metrics, so I think it would be better to add them to /metrics in a separate change that also figures out how to add KRPC metrics there to begin with. http://gerrit.cloudera.org:8080/#/c/9292/11/tests/custom_cluster/test_krpc_metrics.py File tests/custom_cluster/test_krpc_metrics.py: http://gerrit.cloudera.org:8080/#/c/9292/11/tests/custom_cluster/test_krpc_metrics.py@52 PS11, Line 52: -datastream_service_queue_depth=1 > I will address all other comments, then rebase the change, and then address Replaced with the new flag. http://gerrit.cloudera.org:8080/#/c/9292/11/tests/custom_cluster/test_krpc_metrics.py@60 PS11, Line 60: [0] > The idea was that there needs to be at least one service, but you're right, Done. http://gerrit.cloudera.org:8080/#/c/9292/11/tests/custom_cluster/test_krpc_metrics.py@70 PS11, Line 70: -datastream_service_queue_depth=1 > See above. Replaced with the new flag. http://gerrit.cloudera.org:8080/#/c/9292/11/tests/webserver/test_web_pages.py File tests/webserver/test_web_pages.py: http://gerrit.cloudera.org:8080/#/c/9292/11/tests/webserver/test_web_pages.py@250 PS11, Line 250: rpcz['services'][0]['rpc_method_metrics'] > same comment as above Done -- To view, visit http://gerrit.cloudera.org:8080/9292 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7af7c1a84a5be82c979ca4ef1edf35167493be3f Gerrit-Change-Number: 9292 Gerrit-PatchSet: 13 Gerrit-Owner: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Lars Volker <l...@cloudera.com> Gerrit-Reviewer: Michael Ho <k...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Mon, 19 Feb 2018 23:37:50 +0000 Gerrit-HasComments: Yes
