Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/17553 )
Change subject: WIP IMPALA-10557: Support Kudu's multi-row transaction ...................................................................... Patch Set 3: (6 comments) http://gerrit.cloudera.org:8080/#/c/17553/2/be/src/service/client-request-state.h File be/src/service/client-request-state.h: http://gerrit.cloudera.org:8080/#/c/17553/2/be/src/service/client-request-state.h@756 PS2, Line 756: pen transaction > nit: is this specific to Hive? If so, maybe mention that in these comments? Done http://gerrit.cloudera.org:8080/#/c/17553/2/be/src/service/query-options.h File be/src/service/query-options.h: http://gerrit.cloudera.org:8080/#/c/17553/2/be/src/service/query-options.h@252 PS2, Line 252: ADVANCED > Given this feature is considered experimental in Kudu, would it make sense Done http://gerrit.cloudera.org:8080/#/c/17553/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: http://gerrit.cloudera.org:8080/#/c/17553/2/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@206 PS2, Line 206: t > nit: maybe prefix this with "kudu" so it's clear the transactionality is sp Done http://gerrit.cloudera.org:8080/#/c/17553/2/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/2/fe/src/main/java/org/apache/impala/service/Frontend.java@1827 PS2, Line 1827: timeline.markEvent("Kudu transaction aborted"); > nit: does this timeline have a query ID associated with it already? If not, Done http://gerrit.cloudera.org:8080/#/c/17553/3/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/3/fe/src/main/java/org/apache/impala/service/Frontend.java@1717 PS3, Line 1717: else if ((targetTable instanceof FeKuduTable) : && queryOptions.isKudu_transaction()) { : FeKuduTable kuduTable = (FeKuduTable) targetTable; : KuduClient client = KuduUtil.getKuduClient(kuduTable.getKuduMasterHosts()); : KuduTransaction txn = null; : try { : // Open Kudu transaction. : txn = client.newTransaction(); : timeline.markEvent("Kudu transaction opened with query id: " : + queryCtx.getQuery_id().toString()); : insertStmt.setTransactionToken(txn.serialize()); : kuduTxnManager_.addTransaction(queryCtx.getQuery_id(), txn); : queryCtx.setIs_kudu_transactional(true); : } catch (IOException e) { : if (txn != null) txn.close(); : throw new TransactionException(e.getMessage()); : } : } > On Slack we discussed some considerations with regards to which daemon is i Added comments in KuduTransactionManager http://gerrit.cloudera.org:8080/#/c/17553/2/testdata/cluster/node_templates/common/etc/kudu/tserver.conf.tmpl File testdata/cluster/node_templates/common/etc/kudu/tserver.conf.tmpl: http://gerrit.cloudera.org:8080/#/c/17553/2/testdata/cluster/node_templates/common/etc/kudu/tserver.conf.tmpl@18 PS2, Line 18: -time_source=system_unsync > In your tests, we'll need to add --enable_txn_system_client_init here as we Done -- 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: 3 Gerrit-Owner: Wenzhe Zhou <wz...@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: Tue, 08 Jun 2021 05:40:36 +0000 Gerrit-HasComments: Yes
