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 <aser...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Grant Henke <granthe...@apache.org> Gerrit-Reviewer: Hao Hao <hao....@cloudera.com> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Wed, 23 Dec 2020 03:08:51 +0000 Gerrit-HasComments: Yes
