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]