Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/17553 )
Change subject: IMPALA-10557: Support Kudu's multi-row transaction ...................................................................... Patch Set 8: (3 comments) http://gerrit.cloudera.org:8080/#/c/17553/8/be/src/exec/kudu-scan-node-base.h File be/src/exec/kudu-scan-node-base.h: http://gerrit.cloudera.org:8080/#/c/17553/8/be/src/exec/kudu-scan-node-base.h@102 PS8, Line 102: kudu::client::KuduClient* kudu_client() const Well, now it's actually not const-correct [1]. I'd rather keep it non-const, but it's up to you. If you want it to be const-correct, it should have the following signature: const kudu::client::KuduClient* kudu_client() const; [1] https://chromium.googlesource.com/chromium/src/+/master/styleguide/c++/const.md http://gerrit.cloudera.org:8080/#/c/17553/5/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/17553/5/fe/src/main/java/org/apache/impala/service/Frontend.java@2237 PS5, Line 2237: void nit: I'm curious why this is logged with 'error' priority when similar log in commitKuduTransaction() is of 'info' priority? Is that because rolling back a transaction happens only due to an error, and it's not possible to initiate rolling back a transaction explicitly? http://gerrit.cloudera.org:8080/#/c/17553/5/fe/src/main/java/org/apache/impala/service/Frontend.java@2239 PS5, Line 2239: Transaction What if txn is null? Can it happen only due to a programming error or it can happen in other cases? If the former, does it make sense to add a precondition? If the latter, does it deserve to be logged? -- To view, visit http://gerrit.cloudera.org:8080/17553 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I876ada48991afdff5d61b5d6a0417571aba7cb34 Gerrit-Change-Number: 17553 Gerrit-PatchSet: 8 Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Reviewer: Alexey Serbin <aser...@cloudera.com> Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com> Gerrit-Reviewer: Bikramjeet Vig <bikramjeet....@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com> Gerrit-Comment-Date: Thu, 10 Jun 2021 21:35:57 +0000 Gerrit-HasComments: Yes
