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

(4 comments)

Please see my inline comments and PS17. It also removes the now obsolete queue 
size and replaces it with memory usage values.

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@76
PS11, Line 76:
> Is there a JIRA for it ? Will it be done as part of IMPALA-6540 ?
I don't think this would be part of IMPALA-6540, and I filed IMPALA-6541 for 
it. This particular metric here should be moved to KRPC's internal code 
(KUDU-2313).


http://gerrit.cloudera.org:8080/#/c/9292/15/be/src/rpc/impala-service-pool.cc
File be/src/rpc/impala-service-pool.cc:

http://gerrit.cloudera.org:8080/#/c/9292/15/be/src/rpc/impala-service-pool.cc@254
PS15, Line 254: const string KrpcHistogramToString(const kudu::Histogram* 
histogram) {
> It may be more extensible to not return a formatted string. Instead, we can
Currently we're exposing only what is required to fulfill ToJson()'s contract. 
Once we want to wrap Kudu's Histograms and register them inside our metric 
groups, we will have to alter this abstraction (but I don't know how to do that 
yet). I created IMPALA-6541 to track this effort.


http://gerrit.cloudera.org:8080/#/c/9292/15/be/src/rpc/rpc-mgr.cc
File be/src/rpc/rpc-mgr.cc:

http://gerrit.cloudera.org:8080/#/c/9292/15/be/src/rpc/rpc-mgr.cc@164
PS15, Line 164: void RpcMgr::Shutdown() {
> We can probably give up ownership of the acceptor_pool_ before calling mess
Good point, done.


http://gerrit.cloudera.org:8080/#/c/9292/15/be/src/util/histogram-metric.h
File be/src/util/histogram-metric.h:

http://gerrit.cloudera.org:8080/#/c/9292/15/be/src/util/histogram-metric.h@96
PS15, Line 96:
> nit: long line
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: 17
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: Tue, 20 Feb 2018 07:12:32 +0000
Gerrit-HasComments: Yes

Reply via email to