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 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17553/1/be/src/service/client-request-state.cc
File be/src/service/client-request-state.cc:

http://gerrit.cloudera.org:8080/#/c/17553/1/be/src/service/client-request-state.cc@825
PS1, Line 825:   if (InTransaction()) {
             :     AbortTransaction();
             :   } else if (InKuduTransaction()) {
             :     AbortKuduTransaction();
             :   }
> I wonder the rational to decouple kudu tractions from impala ones. For exam
Currently Impala support transaction for insert-only ACID tables for insert 
statements. We cannot insert rows to insert-only ACID table and Kudu table in 
same query. We did not write a common function since their implementations are 
different. Kudu does not provide transaction ID.


http://gerrit.cloudera.org:8080/#/c/17553/1/fe/src/main/java/org/apache/impala/common/KuduTransactionManager.java
File fe/src/main/java/org/apache/impala/common/KuduTransactionManager.java:

http://gerrit.cloudera.org:8080/#/c/17553/1/fe/src/main/java/org/apache/impala/common/KuduTransactionManager.java@54
PS1, Line 54: put(queryId, txn);
> Normally, a transaction can cover multiple queries as follows.
No, it does not support.


http://gerrit.cloudera.org:8080/#/c/17553/1/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/1/fe/src/main/java/org/apache/impala/service/Frontend.java@1717
PS1, 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());
              :           }
              :         }
> This covers insert/createTableAsSelect stmts which is very nice.
Currently Kudu only support transaction for insert. They are working on update, 
but not finish yet.


http://gerrit.cloudera.org:8080/#/c/17553/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java
File fe/src/main/java/org/apache/impala/service/JniFrontend.java:

http://gerrit.cloudera.org:8080/#/c/17553/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java@772
PS1, Line 772: TUniqueId queryId
> Would it be possible to pass in a kuduTransaction object direct here?  This
This function is called from C++ code so we cannot pass KuduTransaction object 
directly.


http://gerrit.cloudera.org:8080/#/c/17553/1/fe/src/main/java/org/apache/impala/service/JniFrontend.java@782
PS1, Line 782: TUniqueId queryId
> Same comment as above.
This function is called from C++ code so we cannot pass KuduTransaction object 
directly.



--
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: 1
Gerrit-Owner: Wenzhe Zhou <[email protected]>
Gerrit-Reviewer: Andrew Wong <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Qifan Chen <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Mon, 07 Jun 2021 23:47:11 +0000
Gerrit-HasComments: Yes

Reply via email to