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 <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Grant Henke <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Tue, 22 Dec 2020 19:35:07 +0000 Gerrit-HasComments: Yes
