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: (6 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 TABLE AS SELECT for Kudu tables or it was rather a deliberate decision not to do so? Certainly, it's not possible to make the DDL phase of CREATE TABLE AS SELECT transactional as well, but may have some value to end user as well? http://gerrit.cloudera.org:8080/#/c/17553/8//COMMIT_MSG@22 PS8, Line 22: binary nit: transaction token ? http://gerrit.cloudera.org:8080/#/c/17553/8//COMMIT_MSG@24 PS8, Line 24: binary nit: transaction token ? 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 If it makes sense, one more test scenario to add might be to ensure the transaction handle kept by the front-end in KuduTransactionManager keeps the transaction open by heartbeating. That probably requires adding an extra FIS and setting --txn_keepalive_interval_ms to something short (like 1000 instead of default 30000) to keep the runtime of the test short. 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 make sense to add scenarios to verify that even if ENABLE_KUDU_TRANSACTION is set, Impala isn't going to initiate a Kudu transaction in such cases? 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 +1 -- 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 00:02:57 +0000 Gerrit-HasComments: Yes
