You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Riza Suminto (Code Review)" <ge...@cloudera.org> on 2022/05/04 00:43:08 UTC
[kudu-CR] [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
Riza Suminto has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18489
Change subject: [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
......................................................................
[client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
The work done in the scope of KUDU-3351 included the server-side changes
and corresponding changes in the Kudu C++ API to expose the metrics to
the client applications. This patch implement similar changes to expose
such metrics in Java client API.
Change-Id: I956eb0c0a2cadcf3491550630b861bb48462e8eb
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/BatchResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
9 files changed, 116 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/18489/1
--
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: newchange
Gerrit-Change-Id: I956eb0c0a2cadcf3491550630b861bb48462e8eb
Gerrit-Change-Number: 18489
Gerrit-PatchSet: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
[kudu-CR] [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
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 <ri...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 May 2022 18:24:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18489 )
Change subject: [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
......................................................................
[client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
The work done in the scope of KUDU-3351 included the server-side changes
and corresponding changes in the Kudu C++ API to expose the metrics to
the client applications. This patch implement similar changes to expose
such metrics in Java client API.
Change-Id: I956eb0c0a2cadcf3491550630b861bb48462e8eb
Reviewed-on: http://gerrit.cloudera.org:8080/18489
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/BatchResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
9 files changed, 120 insertions(+), 5 deletions(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
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: merged
Gerrit-Change-Id: I956eb0c0a2cadcf3491550630b861bb48462e8eb
Gerrit-Change-Number: 18489
Gerrit-PatchSet: 3
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[kudu-CR] [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin 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: Code-Review+2
--
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 <ri...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 May 2022 22:12:21 +0000
Gerrit-HasComments: No
[kudu-CR] [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18489
to look at the new patch set (#2).
Change subject: [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
......................................................................
[client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
The work done in the scope of KUDU-3351 included the server-side changes
and corresponding changes in the Kudu C++ API to expose the metrics to
the client applications. This patch implement similar changes to expose
such metrics in Java client API.
Change-Id: I956eb0c0a2cadcf3491550630b861bb48462e8eb
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Batch.java
M java/kudu-client/src/main/java/org/apache/kudu/client/BatchResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/OperationResponse.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java
M java/kudu-client/src/main/java/org/apache/kudu/client/SessionConfiguration.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
9 files changed, 120 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/89/18489/2
--
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: newpatchset
Gerrit-Change-Id: I956eb0c0a2cadcf3491550630b861bb48462e8eb
Gerrit-Change-Number: 18489
Gerrit-PatchSet: 2
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
[kudu-CR] [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin 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:
Thank you for the contribution!
--
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 <ri...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 May 2022 22:12:32 +0000
Gerrit-HasComments: No
[kudu-CR] [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
Posted by "Riza Suminto (Code Review)" <ge...@cloudera.org>.
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 1:
Hi Alex, this is the follow up patch from KUDU-3351. Please kindly review.
--
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: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 May 2022 20:52:50 +0000
Gerrit-HasComments: No
[kudu-CR] [client] KUDU-3365: Expose INSERT/UPDATE metrics in the Java API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin 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 1: Code-Review+1
(7 comments)
LGTM, just a few nits
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:
style nit: the indent should be 4 spaces, IIRC
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: ResourceMetrics getWriteOpMetrics() {
nit: does it make sense to add @Nullable annotation here?
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 write path as well with this patch.
http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@87
PS1, Line 87: String key = entry.getKey();
nit: this variable is used only once in the increment() call below, so maybe inline the 'entry.getKey()' call instead of introducing a variable?
http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@104
PS1, Line 104: its pb
nit: PB
http://gerrit.cloudera.org:8080/#/c/18489/1/java/kudu-client/src/main/java/org/apache/kudu/client/ResourceMetrics.java@104
PS1, Line 104: not
nit: not be
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: public
nit: maybe, change to private if it's not used anywhere else?
--
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: 1
Gerrit-Owner: Riza Suminto <ri...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Riza Suminto <ri...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 May 2022 01:43:24 +0000
Gerrit-HasComments: Yes