Lalant commented on code in PR #7820:
URL: https://github.com/apache/ignite-3/pull/7820#discussion_r3014670145


##########
modules/transactions/src/main/java/org/apache/ignite/internal/tx/impl/TxRecoveryEngine.java:
##########
@@ -87,7 +87,6 @@ public CompletableFuture<TransactionMeta> triggerTxRecovery(
         // If the transaction state is pending, then the transaction should be 
rolled back,
         // meaning that the state is changed to aborted and a corresponding 
cleanup request
         // is sent in a common durable manner to a partition that has 
initiated recovery.
-        // TODO https://issues.apache.org/jira/browse/IGNITE-27386 the reason 
of rollback needs to be explained.
         return txManager.finish(
                         HybridTimestampTracker.emptyTracker(),
                         // Tx recovery is executed on the commit partition.

Review Comment:
   There is 
   `new TransactionInternalException(TX_ROLLBACK_ERR, format("Unexpected 
transaction "`
      near by. Is it also incorrect? Which code do you propose?



##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/api/ItSqlApiBaseTest.java:
##########
@@ -736,6 +743,88 @@ public void 
runtimeErrorInQueryCausesTransactionToFail(String query) {
                 "Transaction is already finished due to an error");
     }
 
+    @Test
+    public void 
runtimeErrorReturnsSameTransactionErrorBeforeAndAfterRollbackCompletion() 
throws Exception {
+        sql("CREATE TABLE tst(id INTEGER PRIMARY KEY, val INTEGER)");
+
+        IgniteSql sql = igniteSql();
+
+        Transaction tx = igniteTx().begin();
+        UUID txId = tx instanceof InternalTransaction ? ((InternalTransaction) 
tx).id() : null;
+
+        // Enlist enough operations to make rollback non-trivial.
+        for (int i = 0; i < 100; i++) {
+            execute(tx, sql, "INSERT INTO tst VALUES (?, ?)", i, i);
+        }
+
+        assertThrowsSqlException(
+                Sql.RUNTIME_ERR,
+                "Division by zero",
+                () -> execute(tx, sql, "SELECT val / 0 FROM tst WHERE id = ?", 
0)
+        );
+
+        IgniteException[] immediateExceptions = new IgniteException[5];
+        for (int i = 0; i < immediateExceptions.length; i++) {
+            immediateExceptions[i] = (IgniteException) assertThrowsWithCause(
+                    () -> executeForRead(sql, tx, "SELECT * FROM tst WHERE id 
= ?", 1),
+                    IgniteException.class
+            );
+        }
+
+        if (txId != null) {
+            assertTrue(waitForCondition(() -> {
+                TxStateMeta meta = txManager().stateMeta(txId);
+
+                return meta != null && TxState.isFinalState(meta.txState());
+            }, 5_000), "Expected transaction to reach final state");
+        } else {
+            // Client transaction API doesn't expose internal tx id, wait a 
bit to compare with a later request.
+            Thread.sleep(500);
+        }
+
+        IgniteException abortedStateException = (IgniteException) 
assertThrowsWithCause(
+                () -> executeForRead(sql, tx, "SELECT * FROM tst WHERE id = 
?", 1),
+                IgniteException.class
+        );
+
+        assertEquals(Transactions.TX_ALREADY_FINISHED_WITH_EXCEPTION_ERR, 
abortedStateException.code());
+        assertTrue(abortedStateException.getMessage().contains("Transaction is 
already finished due to an error"));
+
+        for (IgniteException immediateException : immediateExceptions) {
+            assertEquals(abortedStateException.code(), 
immediateException.code());
+            assertTrue(immediateException.getMessage().contains("Transaction 
is already finished due to an error"));
+        }
+    }
+
+    @Test
+    public void rollbackWithExceptionCauseIsPropagatedToSubsequentSqlRequest() 
{
+        sql("CREATE TABLE tst(id INTEGER PRIMARY KEY, val INTEGER)");
+        sql("INSERT INTO tst VALUES (?, ?)", 1, 1);
+
+        Transaction tx = igniteTx().begin();
+
+        assumeTrue(tx instanceof InternalTransaction, "InternalTransaction is 
required");
+
+        InternalTransaction internalTx = (InternalTransaction) tx;
+        String rollbackCauseMessage = "rollback-cause-primary-replica-changed";
+        TransactionException rollbackCause = new 
TransactionException(REPLICA_MISS_ERR, rollbackCauseMessage);
+
+        await(internalTx.rollbackWithExceptionAsync(rollbackCause));
+
+        IgniteException ex = assertInstanceOf(
+                IgniteException.class,
+                assertThrowsWithCause(
+                        () -> executeForRead(igniteSql(), tx, "SELECT * FROM 
tst WHERE id = ?", 1),
+                        IgniteException.class
+                )
+        );
+
+        assertEquals(Transactions.TX_ALREADY_FINISHED_WITH_EXCEPTION_ERR, 
ex.code());
+        assertTrue(ex.getMessage().contains("Transaction is already finished 
due to an error"));
+        assertTrue(hasCause(ex, TransactionException.class));
+        assertTrue(hasMessageInChain(ex, rollbackCauseMessage), "Expected 
rollback cause message in user-visible exception chain");

Review Comment:
   Done



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