Wenzhe Zhou 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: (11 comments) http://gerrit.cloudera.org:8080/#/c/17553/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17553/8//COMMIT_MSG@19 PS8, Line 19: "insert" statements > I'm curious: would it make sense to support similar semantics for CREATE TA We should support transaction for CTAS. Could we work it in following up Jira? http://gerrit.cloudera.org:8080/#/c/17553/8//COMMIT_MSG@22 PS8, Line 22: binary > nit: transaction token ? Done http://gerrit.cloudera.org:8080/#/c/17553/8//COMMIT_MSG@24 PS8, Line 24: binary > nit: transaction token ? Done 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-cons I changed it back as non const. Thanks for the reference. 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 changed to info 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 c Added precondition for txn. http://gerrit.cloudera.org:8080/#/c/17553/8/tests/custom_cluster/test_kudu.py File tests/custom_cluster/test_kudu.py: http://gerrit.cloudera.org:8080/#/c/17553/8/tests/custom_cluster/test_kudu.py@336 PS8, Line 336: class TestKuduTransaction(CustomClusterTestSuite): > Another test case that might be worth adding is running two separate Impala Good suggestion. Added the test case. http://gerrit.cloudera.org:8080/#/c/17553/8/tests/custom_cluster/test_kudu.py@336 PS8, Line 336: class TestKuduTransaction(CustomClusterTestSuite): > If it makes sense, one more test scenario to add might be to ensure the tra Is --txn_keepalive_interval_ms a Kudu starting configuration? It seems that Impala end-to-end test do not restart Kudu cluster. Could we set it with Kudu API in runtime? http://gerrit.cloudera.org:8080/#/c/17553/8/tests/custom_cluster/test_kudu.py@346 PS8, Line 346: def test_kudu_txn_commit(self, cursor, unique_database): > Since only INSERT/INSERT_IGNORE is supported for Kudu transactions, does it Sorry, I did not get it how to add such scenario. Could you give more detail? I tried to run query "INSERT IGNORE" from impala-shell as below: INSERT IGNORE INTO my-test VALUES (99, "John"); But got syntax error for IGNORE. http://gerrit.cloudera.org:8080/#/c/17553/8/tests/custom_cluster/test_kudu.py@366 PS8, Line 366: query_options = {'debug_action': 'FIS_FAIL_KUDU_TABLE_SINK_BATCH:FAIL@1.0'} > +1 Done http://gerrit.cloudera.org:8080/#/c/17553/8/tests/custom_cluster/test_kudu.py@366 PS8, Line 366: query_options = {'debug_action': 'FIS_FAIL_KUDU_TABLE_SINK_BATCH:FAIL@1.0'} > A more organic test might be to try inserting duplicate rows. That would ex Added a test cases with duplicated key values. -- 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: Fri, 11 Jun 2021 17:38:34 +0000 Gerrit-HasComments: Yes
