Andrew Wong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16929 )
Change subject: KUDU-2612 Java client transaction implementation ...................................................................... Patch Set 4: (5 comments) http://gerrit.cloudera.org:8080/#/c/16929/4/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/16929/4/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@292 PS4, Line 292: final Transactions.TxnStatePB txnState = resp.txnState(); > No, the transaction state isn't exposed anywhere, and that's intentional. Right, at most I think we'll expose state for some tooling, but it seems more future-proof to keep the main public API free from internal details. http://gerrit.cloudera.org:8080/#/c/16929/4/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTransaction.java@468 PS4, Line 468: where the table is already fully altered. nit: where the transaction is fully committed? http://gerrit.cloudera.org:8080/#/c/16929/4/java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java File java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java: http://gerrit.cloudera.org:8080/#/c/16929/4/java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java@296 PS4, Line 296: // TODO(aserbin): implement this nit: remove? Or is there more to be done? http://gerrit.cloudera.org:8080/#/c/16929/4/java/kudu-client/src/main/java/org/apache/kudu/client/RpcProxy.java@440 PS4, Line 440: // TODO(aserbin): try sending request to other TxnManager instance, if possible Is the 'connection' here compatible with using client.handleNotLeader()? I wonder if we can get by with always trying to re-find the leader, especially if the master does go down. Or does that happen already? http://gerrit.cloudera.org:8080/#/c/16929/4/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/16929/4/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTransaction.java@401 PS4, Line 401: ou nit: typo -- To view, visit http://gerrit.cloudera.org:8080/16929 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie0236875e7264877c3f7a13da4a5a3da6423786b Gerrit-Change-Number: 16929 Gerrit-PatchSet: 4 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-Comment-Date: Thu, 21 Jan 2021 21:15:53 +0000 Gerrit-HasComments: Yes
