You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2021/10/29 16:12:49 UTC

[GitHub] [shardingsphere] zhfeng opened a new pull request #13362: Fix #12771 to support XA statements in proxy

zhfeng opened a new pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362


   * minor issues in XA statement parse
   
   * add XATransactionBackendHandler
   
   * add inXA in TransactionStatus
   
   Fixes #12771
   
   Replace #13187 
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] tristaZero commented on pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
tristaZero commented on pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#issuecomment-963792799


   @jingshanglu Hey, please give this PR a look.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#issuecomment-955840764


   @roodkcab Please can you check with this fix to see if it works in your case ? Thanks !


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] TeslaCN commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r739939552



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       Could we use the existing`transactionType` and `inTransaction`? 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r741596964



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       So I think it should change the following
   https://github.com/apache/shardingsphere/blob/8fc35c9afee87627dd43381c086dfe81df062b47/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java#L56-L64
   
   with ```return (InTransaction && TransactionType.BASE != transactionType) || inXA;```
   
   So we can hold the connections during the XA transaction. Does it make sense ?

##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       So I think it should change the following
   https://github.com/apache/shardingsphere/blob/8fc35c9afee87627dd43381c086dfe81df062b47/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java#L56-L64
   
   with ```return (InTransaction && TransactionType.BASE != transactionType) || inXA;```
   
   So we can hold the connections during the XA transaction. Does it make sense ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] TeslaCN commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r743823357



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       I see your point. The `TransactionType.XA` and `inXa` seem confusing. I think the `inXa` is something like `TransactionType.MANUAL`.
   From perspective of performance, broadcasting the SQL may cause cause high latency, especially if there is a large quantity of actual data nodes.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r740896852



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       Thanks - I will take look.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] TeslaCN commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r740885588



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       You may check the following classes
   - `org.apache.shardingsphere.proxy.frontend.command.CommandExecutorTask`
   - `org.apache.shardingsphere.proxy.frontend.state.impl.OKProxyState`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r739998274



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       Well, can you point out the codes related ? I can take a close look.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r740896852



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       Thanks - I will take look.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r741596964



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       So I think it should change the following
   https://github.com/apache/shardingsphere/blob/8fc35c9afee87627dd43381c086dfe81df062b47/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java#L56-L64
   
   with ```return (InTransaction && TransactionType.BASE != transactionType) || inXA;```
   
   So we can hold the connections during the XA transaction. Does it make sense ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r747409833



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       Well,  I think ```TransactionType.XA``` here is different semantics with ```inXA``.
   
   * ```TransactionType.XA``` is used by shardingsphere to manage the distributed transaction **INTERNAL**
   * ```inXA``` means that users will be responsible to manage the transaction by themself, espcially using XA. So I'd like to change to use ```TransactionType.MANUAL```.
   
   Also I'd like to understand the desgin strategy of transactions in shardingsphere in details. @terrymanu is there any document can be shared ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r740896852



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       Thanks - I will take look.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] TeslaCN commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r743823357



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       I see your point. The `TransactionType.XA` and `inXa` seem confusing. I think the `inXa` is something like `TransactionType.MANUAL`.
   From perspective of performance, broadcasting the SQL may cause cause high latency, especially if there is a large quantity of actual data nodes.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] TeslaCN commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r740885588



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       You may check the following classes
   - `org.apache.shardingsphere.proxy.frontend.command.CommandExecutorTask`
   - `org.apache.shardingsphere.proxy.frontend.state.impl.OKProxyState`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r741596964



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       So I think it should change the following
   https://github.com/apache/shardingsphere/blob/8fc35c9afee87627dd43381c086dfe81df062b47/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java#L56-L64
   
   with ```return (InTransaction && TransactionType.BASE != transactionType) || inXA;```
   
   So we can hold the connections during the XA transaction. Does it make sense ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] TeslaCN commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r739967836



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       If not `inTransaction`, connections will be returned to DataSource after each `CommandExecuteTask` finished. And which thread pool is selected to execute the task is related to `transactionType`(Refer to the class `OKProxyState`). I think this issue is not so simple that just passing the SQL to actual databases.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r739940601



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       No, I don't think so. They are two different transaction types and I think ```inXA``` is used in cases which handle the XA related statments to mark that the current transaction is in XA mode.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] jingshanglu commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r747367304



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       > So I think it should change the following
   > 
   > https://github.com/apache/shardingsphere/blob/8fc35c9afee87627dd43381c086dfe81df062b47/shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java#L56-L64
   > 
   > with `return (InTransaction && TransactionType.BASE != transactionType) || inXA;`
   > 
   > So we can hold the connections during the XA transaction. Does it make sense ?
   
   @zhfeng @TeslaCN Maybe we can use
   ```
   return inTransaction && (TransactionType.XA == transactionType || .....);
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] TeslaCN commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r743823357



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       I see your point. The `TransactionType.XA` and `inXa` seem confusing. I think the `inXa` is something like `TransactionType.MANUAL`.
   From perspective of performance, broadcasting the SQL may cause cause high latency, especially if there is a large quantity of actual data nodes.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] TeslaCN commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
TeslaCN commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r740885588



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       You may check the following classes
   - `org.apache.shardingsphere.proxy.frontend.command.CommandExecutorTask`
   - `org.apache.shardingsphere.proxy.frontend.state.impl.OKProxyState`




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] jingshanglu commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
jingshanglu commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r749008697



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       > Well, I think `TransactionType.XA` here is different semantics with ```inXA``.
   > 
   > * `TransactionType.XA` is used by shardingsphere to manage the distributed transaction **INTERNAL**
   > * `inXA` means that users will be responsible to manage the transaction by themself, espcially using XA. So I'd like to change to use `TransactionType.MANUAL`.
   > 
   > Also I'd like to understand the desgin strategy of transactions in shardingsphere in details. @terrymanu is there any document can be shared ?
   how about `TransactionType.XA` and `inTransaction` equals to `inXA`? @zhfeng 
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r749040713



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       No, I'd like to say them are totaly different here.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#issuecomment-992643577


   Sine we have not a decision to support XA, I withdraw this PR and just commit a fix for mysql xa parser. see #14075  


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng closed pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng closed pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [shardingsphere] zhfeng commented on a change in pull request #13362: Fix #12771 to support XA statements in proxy

Posted by GitBox <gi...@apache.org>.
zhfeng commented on a change in pull request #13362:
URL: https://github.com/apache/shardingsphere/pull/13362#discussion_r744038498



##########
File path: shardingsphere-proxy/shardingsphere-proxy-backend/src/main/java/org/apache/shardingsphere/proxy/backend/communication/jdbc/transaction/TransactionStatus.java
##########
@@ -34,6 +34,9 @@
     
     @Setter
     private volatile boolean inTransaction;
+
+    @Setter
+    private volatile boolean inXA;

Review comment:
       yeah, exactly - It should be user's resposiblity to control the transactions. And I can understand the performance issues,  if the bussiness logic has a strong data consistent, that must be a tradeofff.
   
   So, can we introduce the ```TransactionType.MANUAL```? and if the user select this type, the proxy will broadcast all of the transaction related statements to the backend.
   
   Does it make sense ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org