Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/15315 )
Change subject: [java] subprocess: add a metrics message ...................................................................... Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess-echo/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java File java/kudu-subprocess-echo/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java: http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess-echo/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java@122 PS1, Line 122: Sends nit: Send. http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess-echo/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java@128 PS1, Line 128: Receives nit: Receive http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess-echo/src/test/java/org/apache/kudu/subprocess/echo/TestEchoSubprocess.java@135 PS1, Line 135: Sets nit: Set http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/InboundRequest.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/InboundRequest.java: http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/InboundRequest.java@21 PS1, Line 21: It is expected that the : * <code>SubprocessMetrics</code> have begun timing the time it takes to make : * it through the queue Is this required SubprocessMetrics has started timing before the initialization of InboundRequest? http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageParser.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageParser.java: http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/MessageParser.java@69 PS1, Line 69: metrics.startTimer(); > Note in a comment here what we're starting to record and who is expected to +1, as of now it is a bit confusing that we call startTimer twice here. http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessMetrics.java File java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessMetrics.java: http://gerrit.cloudera.org:8080/#/c/15315/1/java/kudu-subprocess/src/main/java/org/apache/kudu/subprocess/SubprocessMetrics.java@71 PS1, Line 71: reset Why use reset not stop here? -- To view, visit http://gerrit.cloudera.org:8080/15315 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I11a89fff8df23c5057c577f2aebfd40922d01e3c Gerrit-Change-Number: 15315 Gerrit-PatchSet: 1 Gerrit-Owner: Andrew Wong <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Attila Bukor <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Comment-Date: Mon, 02 Mar 2020 07:41:22 +0000 Gerrit-HasComments: Yes
