You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Alexey Serbin (Code Review)" <ge...@cloudera.org> on 2020/12/22 06:58:33 UTC
[kudu-CR] WIP KUDU-2612 Java client transaction API
Alexey Serbin has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16894
Change subject: WIP KUDU-2612 Java client transaction API
......................................................................
WIP KUDU-2612 Java client transaction API
This is a raw WIP patch which at this point is more focused on the
API than the actual functionality under the hood. The actual
functionality to do the heavy-lifting (i.e. issuing RPC calls to
TxnManager, retrying in case of transient errors, etc.) is not yet here,
but I'm working on it.
The proposed API is mirroring the API for the C++ client with a few
twists because of the presence of the async-style objects in the
Kudu Java client API.
WIP:
* get initial feedback on the proposed API: the semantics, signatures
of the methods, supportability, extensibility, etc.
* add actual functionality for the under-the-hood work
* add more tests
* change the commit description
DONT_BUILD
Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java
6 files changed, 549 insertions(+), 5 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/1
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/16894/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16894/7//COMMIT_MSG@21
PS7, Line 21: asynchrnous
> nit: typo
Done
http://gerrit.cloudera.org:8080/#/c/16894/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@77
PS7, Line 77:
> nit: fix spacing?
Done
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Jan 2021 03:09:38 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16894
to look at the new patch set (#3).
Change subject: KUDU-2612 Java client transaction API
......................................................................
KUDU-2612 Java client transaction API
This patch focused on the API than the actual functionality under the
hood. The functionality to do the heavy-lifting (i.e. issuing RPC calls
to TxnManager, retrying in case of transient errors, tests, etc.) will
be posted as a separate patch as per our discussion with Andrew and Hao.
The proposed API is mirroring the API for the C++ client with a few
twists because of the presence of the async-style objects in the
Kudu Java client API.
Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
5 files changed, 468 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/3
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16894
to look at the new patch set (#4).
Change subject: KUDU-2612 Java client transaction API
......................................................................
KUDU-2612 Java client transaction API
This patch focused on the API than the actual functionality under the
hood. The functionality to do the heavy-lifting (i.e. issuing RPC calls
to TxnManager, retrying in case of transient errors, tests, etc.) will
be posted as a separate patch as per our discussion with Andrew and Hao.
The proposed API is mirroring the API for the C++ client with a few
twists because of the presence of the async-style objects in the
Kudu Java client API.
The asynchronous API bindings (i.e. bindings with Deferred<Xxx>) aren't
provided in this patch. I'm not sure it makes any sense in investing
in that at this point given that I'm not aware of any users of the
asynchrnous Kudu client API except for Java Kudu client itself.
If necessary, we can add AsyncKuduTransaction with appropriate semantics
later on.
Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
4 files changed, 425 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/4
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 4
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[kudu-CR] WIP KUDU-2612 Java client transaction API
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: WIP KUDU-2612 Java client transaction API
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java:
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java@292
PS1, Line 292: @{see AsyncKuduClient#newTransaction()}
nit: the comment around newTransaction() doesn't describe the behavior for non-transactional sessions. I'd prefer leaving this comment as is.
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@250
PS1, Line 250: rollback();
I'm hesitant to automatically rollback if we're still in-flight. For instance, if this were a Spark executor, I'd imagine we just want to flush the session and exit, relying on the driver to commit. Maybe we should add an option to the serdes to toggle automatic rollback.
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java:
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@65
PS1, Line 65: {@l
nit: missing right bracket?
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@70
PS1, Line 70: t
nit: star
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@96
PS1, Line 96: have
nit: has
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 22 Dec 2020 19:35:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: WIP KUDU-2612 Java client transaction API
......................................................................
Patch Set 1:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java:
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java@292
PS1, Line 292: @{see AsyncKuduClient#newTransaction()}
> nit: the comment around newTransaction() doesn't describe the behavior for
Whoops, that's a mistake. I meant to do this for KuduTransaction.newTransaction.
Fixed.
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@250
PS1, Line 250: rollback();
> I'm hesitant to automatically rollback if we're still in-flight. For instan
It's a very good point. It seems I went too far with adding this -- at this point it should be enough stopping keepalive txn heartbeating when close() is called: at least, we deliberately avoided adding something like that for KuduTransaction handle for Kudu C++ client.
I think we could add this auto-rollback feature later on, and do this for both C++ and Java client.
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java:
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@65
PS1, Line 65: {@l
> nit: missing right bracket?
Done
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@70
PS1, Line 70: t
> nit: star
Done
http://gerrit.cloudera.org:8080/#/c/16894/1/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@96
PS1, Line 96: have
> nit: has
Done
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 1
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 23 Dec 2020 03:08:51 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 2:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@43
PS2, Line 43: @c
> typo?
Done
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@47
PS2, Line 47: soon
> when is soon explicitly?
Done
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@50
PS2, Line 50: @InterfaceStability.Evolving
> Should we mark this as InterfaceStability.Unstable until the transaction fe
It's a good point, thanks.
Done.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@70
PS2, Line 70: this.client = client;
> when does txnId and keepaliveMillis get set when using this constructor?
This is done after receiving response for the BeginTransaction() RPC. I was planning to have the actual functionality in this patch, but after recent sync-up with Andrew and Hao there was a decision to keep this patch as a hollow API-only shell.
The actual functionality comes in a follow-up patch.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@248
PS2, Line 248: rolling back a transaction
> Is close only called when rolling back?
This is an override for AutoCloseable::close(). It's possible to call 'close()' explicitly, and it's totally fine to call 'KuduTransaction::rollback()' at any time explicitly when needed: e.g., when an exception is thrown by a write operation created from in transactional KuduSession.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@108
PS2, Line 108: public void setEnableKeepalive(boolean enableKeepalive) {
> Why isn't this toggled on the transaction itself?
A couple of reasons:
* in case of newly created transaction, the keepalive heartbeating should be automatically enabled always, so not much sense in such a control there
* in case of the 'star' topology, when deserializing a transaction from a token there isn't much sense of sending keepalive messages for every txn created out of token once the transaction commit/rollback is controlled by the top-level txn (e.g., the transaction started by Spark driver), otherwise it will lead to duplicated keepalive messages
Basically, the only case when it's necessary to enable keepalive for a transaction created by deserializing a txn token is the 'ring' topology.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@134
PS2, Line 134: public byte[] serialize() throws IOException {
> What's the reason for a separate class for this versus putting in KuduTrans
Because of the consistency reasons.
(a) It's necessary to have the control over the keepalive property for a transaction deserialized from a token, where the creator of the token (not the caller of the deserialize() method) controls the keepalive behavior
(b) adding serialize() into the KuduTransaction() along with keepalive control makes the deserialize() method inconsistent since it makes the caller think that deserialize() is also affected by the setEnableKeepalive() settings.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java@31
PS2, Line 31: public class TestKuduTransaction {
> Does it make sense to add a test for the expected behavior when the transac
Yeah, many more tests are being added.
I'll add the tests you mentioned in the follow-up patch with the actual functionality. Since PS3 this becomes a hollow API shell-only patch -- I'm moving all the tests out into the follow-up patch.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 20:33:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181
PS2, Line 1181: let = Preconditions.c
> After thinking about this a bit more, I found that for asynchronous program
Yes, I was wondering why we have a blocking call in the class that was meant for async ones. Your suggestion make sense to me. Thanks a lot!
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jan 2021 00:39:39 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 7:
> We discussed offline changing from a KuduTransactionSerializer to
> the serializer method which takes an option class. The benefit is
> all the serialize/deserialize methods are on the KuduTransaction
> itself and doesn't require a short lived object created for each
> serialize/deserialize call.
Yes, indeed.
I posted PS7 using the suggested approach.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 14 Jan 2021 23:53:36 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181
PS2, Line 1181: This call is blocking
> Would you mind explain a bit more about why it is blocking?
You mean we should follow the suite of majority of methods in this interface and return something like Deferred<KuduTransaction> and add 'KuduTransaction newTransaction()' into the KuduClient's interface?
I didn't see much value in returning Deferred<KuduTransaction> because I don't quite see the use cases for that. Let me know if you think otherwise and we should have this returning Deferred<KuduTransaction>, keeping the deferred purity of this interface.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 09:03:42 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16894
to look at the new patch set (#8).
Change subject: KUDU-2612 Java client transaction API
......................................................................
KUDU-2612 Java client transaction API
This patch is focused on the API rather than the actual functionality
under the hood. The functionality to do the heavy-lifting (i.e. issuing
RPC calls to TxnManager, retrying in case of transient errors, tests,
etc.) will be posted as a separate patch as per our discussion with
Andrew and Hao.
The proposed API is mirroring the API for the C++ client with a few
twists in the functions' signatures: the Java client uses exceptions
instead of return statuses, etc.
The asynchronous API bindings (i.e. bindings with Deferred<Xxx>) aren't
provided in this patch. I'm not sure it makes any sense in investing
in that at this point given that I'm not aware of any users of the
asynchronous Kudu client API except for Java Kudu client itself.
If necessary, we can add AsyncKuduTransaction with appropriate semantics
later on.
Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
3 files changed, 414 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/8
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 7: Code-Review+2
Looks good to me. Thanks for revising.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Jan 2021 01:31:41 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181
PS2, Line 1181: This call is blocking
> I like the idea of keeping the deferred purity of the interface.
OK, then I'll change it to returning Deferred<KuduTransaction> as well.
Thank you for the feedback!
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 20:34:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 6: Code-Review-2
We discussed offline changing from a KuduTransactionSerializer to the serializer method which takes an option class. The benefit is all the serialize/deserialize methods are on the KuduTransaction itself and doesn't require a short lived object created for each serialize/deserialize call.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Jan 2021 20:56:43 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181
PS2, Line 1181: This call is blocking
> OK, then I'll change it to returning Deferred<KuduTransaction> as well.
After thinking about this a bit more, I found that for asynchronous programming it's necessary to provide asynchronous interface for all transactional operations as well.
I'm not sure I want to invest time in that right now. So, I'm removing newTransaction() method from AsyncKuduClient and leave newTransaction() in KuduClient that returns regular synchronous-style interface for operations.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 23:30:41 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Grant Henke (Code Review)" <ge...@cloudera.org>.
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181
PS2, Line 1181: This call is blocking
> You mean we should follow the suite of majority of methods in this interfac
I like the idea of keeping the deferred purity of the interface.
I can imagine someone writing an async application that writes to kudu which asynchronously chains together opening, writing, and closing a transaction. It could also be useful to unify the handling of exceptions when making calls to this interface.
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@43
PS2, Line 43: @c
typo?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@47
PS2, Line 47: soon
when is soon explicitly?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@50
PS2, Line 50: @InterfaceStability.Evolving
Should we mark this as InterfaceStability.Unstable until the transaction feature is ready for use?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@70
PS2, Line 70: this.client = client;
when does txnId and keepaliveMillis get set when using this constructor?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@248
PS2, Line 248: rolling back a transaction
Is close only called when rolling back?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@108
PS2, Line 108: public void setEnableKeepalive(boolean enableKeepalive) {
Why isn't this toggled on the transaction itself?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java@134
PS2, Line 134: public byte[] serialize() throws IOException {
What's the reason for a separate class for this versus putting in KuduTransaction next to the deserialize method?
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java@31
PS2, Line 31: public class TestKuduTransaction {
Does it make sense to add a test for the expected behavior when the transaction feature is missing/disabled server side?
I think it would be good to test the auto close functionality too.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 06 Jan 2021 17:51:40 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16894
to look at the new patch set (#7).
Change subject: KUDU-2612 Java client transaction API
......................................................................
KUDU-2612 Java client transaction API
This patch focused on the API than the actual functionality under the
hood. The functionality to do the heavy-lifting (i.e. issuing RPC calls
to TxnManager, retrying in case of transient errors, tests, etc.) will
be posted as a separate patch as per our discussion with Andrew and Hao.
The proposed API is mirroring the API for the C++ client with a few
twists because of the presence of the async-style objects in the
Kudu Java client API.
The asynchronous API bindings (i.e. bindings with Deferred<Xxx>) aren't
provided in this patch. I'm not sure it makes any sense in investing
in that at this point given that I'm not aware of any users of the
asynchrnous Kudu client API except for Java Kudu client itself.
If necessary, we can add AsyncKuduTransaction with appropriate semantics
later on.
Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
3 files changed, 411 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/7
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16894
to look at the new patch set (#5).
Change subject: KUDU-2612 Java client transaction API
......................................................................
KUDU-2612 Java client transaction API
This patch focused on the API than the actual functionality under the
hood. The functionality to do the heavy-lifting (i.e. issuing RPC calls
to TxnManager, retrying in case of transient errors, tests, etc.) will
be posted as a separate patch as per our discussion with Andrew and Hao.
The proposed API is mirroring the API for the C++ client with a few
twists because of the presence of the async-style objects in the
Kudu Java client API.
The asynchronous API bindings (i.e. bindings with Deferred<Xxx>) aren't
provided in this patch. I'm not sure it makes any sense in investing
in that at this point given that I'm not aware of any users of the
asynchrnous Kudu client API except for Java Kudu client itself.
If necessary, we can add AsyncKuduTransaction with appropriate semantics
later on.
Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
4 files changed, 429 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/5
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 5
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Andrew Wong (Code Review)" <ge...@cloudera.org>.
Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 7: Code-Review+2
(2 comments)
LGTM for the most part, but would be good to get +2 from Grant.
http://gerrit.cloudera.org:8080/#/c/16894/7//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/16894/7//COMMIT_MSG@21
PS7, Line 21: asynchrnous
nit: typo
http://gerrit.cloudera.org:8080/#/c/16894/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java:
http://gerrit.cloudera.org:8080/#/c/16894/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@77
PS7, Line 77:
nit: fix spacing?
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 7
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Jan 2021 01:29:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 6
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 12 Jan 2021 20:15:49 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
Patch Set 8: Code-Review+2
Carrying over +2 from Andrew and Grant on PS7.
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 8
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Jan 2021 05:07:14 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-2612 Java client transaction 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/16894 )
Change subject: KUDU-2612 Java client transaction API
......................................................................
KUDU-2612 Java client transaction API
This patch is focused on the API rather than the actual functionality
under the hood. The functionality to do the heavy-lifting (i.e. issuing
RPC calls to TxnManager, retrying in case of transient errors, tests,
etc.) will be posted as a separate patch as per our discussion with
Andrew and Hao.
The proposed API is mirroring the API for the C++ client with a few
twists in the functions' signatures: the Java client uses exceptions
instead of return statuses, etc.
The asynchronous API bindings (i.e. bindings with Deferred<Xxx>) aren't
provided in this patch. I'm not sure it makes any sense in investing
in that at this point given that I'm not aware of any users of the
asynchronous Kudu client API except for Java Kudu client itself.
If necessary, we can add AsyncKuduTransaction with appropriate semantics
later on.
Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Reviewed-on: http://gerrit.cloudera.org:8080/16894
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
3 files changed, 414 insertions(+), 1 deletion(-)
Approvals:
Kudu Jenkins: Verified
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 9
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[kudu-CR] WIP KUDU-2612 Java client transaction API
Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16894 )
Change subject: WIP KUDU-2612 Java client transaction API
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/16894/2/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@1181
PS2, Line 1181: This call is blocking
Would you mind explain a bit more about why it is blocking?
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 05 Jan 2021 20:07:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-2612 Java client transaction API
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Andrew Wong, Grant Henke, Hao Hao, Tim Armstrong,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/16894
to look at the new patch set (#2).
Change subject: WIP KUDU-2612 Java client transaction API
......................................................................
WIP KUDU-2612 Java client transaction API
This is a raw WIP patch which at this point is more focused on the
API than the actual functionality under the hood. The actual
functionality to do the heavy-lifting (i.e. issuing RPC calls to
TxnManager, retrying in case of transient errors, etc.) is not yet here,
but I'm working on it.
The proposed API is mirroring the API for the C++ client with a few
twists because of the presence of the async-style objects in the
Kudu Java client API.
WIP:
* get initial feedback on the proposed API: the semantics, signatures
of the methods, supportability, extensibility, etc.
* add actual functionality for the under-the-hood work
* add more tests
* change the commit description
DONT_BUILD
Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduSession.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java
A java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransactionSerializer.java
A java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java
6 files changed, 544 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/94/16894/2
--
To view, visit http://gerrit.cloudera.org:8080/16894
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Idbb18e1ac0454a8ef9e3486430dfaa336e381e07
Gerrit-Change-Number: 16894
Gerrit-PatchSet: 2
Gerrit-Owner: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>