Riza Suminto has posted comments on this change. ( http://gerrit.cloudera.org:8080/18489 )
Change subject: [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API ...................................................................... Patch Set 2: (7 comments) http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java File java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java: http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java@188 PS1, Line 188: Reso > style nit: the indent should be 4 spaces, IIRC Done http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java File java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java: http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java@129 PS1, Line 129: */ > nit: does it make sense to add @Nullable annotation here? Done http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java File java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java: http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@35 PS1, Line 35: * A container for scanner resource metrics. : * <p> : * This class wraps a mapping from metric name to metric value for server-side : * metrics associated with a scanner > nit: could you please update this? The ResourceMetrics are now used in the Done http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@87 PS1, Line 87: increment(entry.getKey(), en > nit: this variable is used only once in the increment() call below, so mayb Done http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@104 PS1, Line 104: > nit: PB Done http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@104 PS1, Line 104: > nit: not be Done http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java: http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java@347 PS1, Line 347: private > nit: maybe, change to private if it's not used anywhere else? Done -- To view, visit http://gerrit.cloudera.org:8080/18489 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I956eb0c0a2cadcf3491550630b861bb48462e8eb Gerrit-Change-Number: 18489 Gerrit-PatchSet: 2 Gerrit-Owner: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <ale...@apache.org> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Comment-Date: Thu, 05 May 2022 18:24:38 +0000 Gerrit-HasComments: Yes
