tmgodinho commented on code in PR #7844:
URL: https://github.com/apache/ignite-3/pull/7844#discussion_r3116669365


##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java:
##########
@@ -94,12 +105,35 @@ public void rollback() throws TransactionException {
     public CompletableFuture<Void> rollbackAsync() {
         var tx0 = tx;
 
+        // TODO: IGNITE-28405 This is really fishy. It will probably let you 
reuse a transaction after calling a rollback :(
         if (tx0 == null) {
             // No operations were performed, nothing to rollback.
             return nullCompletedFuture();
         }
 
-        return tx0.thenCompose(ClientTransaction::rollbackAsync);
+        // If the transaction is not started. Issue the rollback and wait for 
the server response.
+        if (!tx0.isDone() && cancelled.compareAndSet(false, true)) {
+            return requestInfoFuture

Review Comment:
   We might have a small race condition here, because the 
`ClientTransaction::rollbackAsync` does not tolerate errors as well as the 
`TX_ROLLBACK_USING_FIRST_REQUEST`.
   I'll have to harden this a bit.



##########
modules/client/src/main/java/org/apache/ignite/internal/client/tx/ClientLazyTransaction.java:
##########
@@ -94,12 +105,35 @@ public void rollback() throws TransactionException {
     public CompletableFuture<Void> rollbackAsync() {
         var tx0 = tx;
 
+        // TODO: IGNITE-28405 This is really fishy. It will probably let you 
reuse a transaction after calling a rollback :(
         if (tx0 == null) {
             // No operations were performed, nothing to rollback.
             return nullCompletedFuture();
         }
 
-        return tx0.thenCompose(ClientTransaction::rollbackAsync);
+        // If the transaction is not started. Issue the rollback and wait for 
the server response.
+        if (!tx0.isDone() && cancelled.compareAndSet(false, true)) {

Review Comment:
   Yes, it should be ok, we don't need very strong guarantees. This method 
should be idempotent. The other `cancelled.compareAndSet(false, true)` is more 
an client-side optimization to reduce requests than anything else.
    
   Scenario 1:
   1. isDone is false. Rollback call passes to `TX_ROLLBACK_USING_FIRST_REQUEST`
   2. Then isDone flips to true.
   3. `TX_ROLLBACK_USING_FIRST_REQUEST` sent to server.
   4. Then server responds ok.
   5. Tx is successfully rolled back.
   
   Scenario 2:
   1. isDone is false. Rollback call passes to `TX_ROLLBACK_USING_FIRST_REQUEST`
   2. Then isDone flips to true.
   3. `TX_ROLLBACK_USING_FIRST_REQUEST` sent to server.
   4. Then server responds with failure.
   5. Rollback is resent with the previous method using reqId.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to