Andrew Wong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15344 )

Change subject: subprocess: plumb Java metrics into C++
......................................................................


Patch Set 2:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/15344/1//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15344/1//COMMIT_MSG@11
PS1, Line 11: SubprossProxy
> SubprocessProxy?
Done


http://gerrit.cloudera.org:8080/#/c/15344/1/src/kudu/subprocess/echo_subprocess.h
File src/kudu/subprocess/echo_subprocess.h:

PS1:
> Agreed with Tidy Bot: if this file is all metric definitions, why is it a h
Done


http://gerrit.cloudera.org:8080/#/c/15344/1/src/kudu/subprocess/echo_subprocess.h@58
PS1, Line 58:
            :
> Don't want to use histograms for these?
Ah, yeah at first my rationale was that I thought gauges were more useful for 
understanding the state of the queue over time. But you're right that 
histograms track it at a more fine-grained level, especially when requests come 
in at a high rate.


http://gerrit.cloudera.org:8080/#/c/15344/1/src/kudu/subprocess/subprocess_proxy.h
File src/kudu/subprocess/subprocess_proxy.h:

http://gerrit.cloudera.org:8080/#/c/15344/1/src/kudu/subprocess/subprocess_proxy.h@60
PS1, Line 60:     sreq.mutable_request()->PackFrom(req);
            :     SubprocessResponsePB sresp;
            :     RETURN_NOT_OK(server_.Execute(&sreq, &sresp));
> Could you do this instead?
Done



--
To view, visit http://gerrit.cloudera.org:8080/15344
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7260ea13717dfd4af0138f77dfb6e5d239b3bee2
Gerrit-Change-Number: 15344
Gerrit-PatchSet: 2
Gerrit-Owner: Andrew Wong <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Comment-Date: Tue, 03 Mar 2020 20:32:42 +0000
Gerrit-HasComments: Yes

Reply via email to