11 Replies Latest reply on Jul 29, 2015 7:43 AM by marklittle

commit() throws exception: do we need to call rollback()?

rvansa Jul 24, 2015 12:33 PM

I haven't found information from above clearly stated in JTA specification, could you please cite any authoritative source? The issue is if the code below is correct:

tx.begin();
try {
   /* do something */
   tx.commit();
} catch (Exception e) {
   tx.rollback();
   /* handle failure during commit */
}

I believe that it's not; if the transaction has failed to commit, throwing an exception, it should be rolled back from the commit() call itself. User should not be responsible for calling rollback() - actually at this point the transaction should be disassociated. JTA 1.1 spec says in documentation for UserTransaction and TransactionManager (but not Transaction) commit()

Complete the transaction associated with the current thread. When this method completes, the thread becomes associated with no transaction

So after the commit completes (with exception), it's not associated and there's nothing to be rolled back.

The spec also says (for both UserTransaction, TransactionManager and Transaction):

RollbackException Thrown to indicate that the transaction has been rolled back rather than committed.

That suggests that in at least some cases (and it seems to me that it should do in all cases when commit fails before committing some of the resources) the commit() actually performs complete rollback(), and then throws the exception.

On the other hand, spec for Synchronization says that:

An unchecked exception thrown by a registered Synchronization object causes the transaction to be aborted. That is, upon encountering an unchecked exception thrown by a registered synchronization object, the transaction manager must mark the transaction for rollback.

It does not say that it immediatelly starts rollback(), just marks for the rollback. However, I think that this failure should not stop other synchronizations and the two-phase commit process to be invoked; it just says that the transaction cannot commit after this.

So, in my opinion the code should look like

tx.begin();
try {
   /* do something */
} finally {
   try {       
       if (tx.getStatus() == Status.ACTIVE) {
           tx.commit();      
       } else {
           tx.rollback();       
       }
   } catch (Exception e) {
       /* handle commit/rollback failure */
   }
}

Thanks for the clarification.

Note: In the one below I am checking the status because if tx.markRollbackOnly() was called in /* do something */, we would then call a commit() doomed to fail. But that should work, too... So, theoretically just commit() should be enough.

1. Re: commit() throws exception: do we need to call rollback()?

tomjenkinson Jul 27, 2015 4:51 AM (in response to rvansa)

Hi Radim,

Certainly for narayana if the commit fails we will attempt a rollback. The one situation where this won't happen is if an XAResource is none-compliant and throws an java.lang.Error but we haven't see this with current resource managers and should be relatively trivial for them to fix.

I think rather than tx.getStatus == Status.ACTIVE you should reason about your application business logic and whether commit is necessary and leave the getStatus to the transaction manager to work out if the TX is suitable for commit

tx.begin();
try {
   /* do something */
} finally {
   try {
       if (// BUSINESS LOGIC MEANS COMMIT) {
           tx.commit();
       } else {
           tx.rollback();
       }
   } catch (Exception e) {
       /* handle commit/rollback failure */
   }
}

Hope it helps,
Tom
Actions
2. Re: commit() throws exception: do we need to call rollback()?

rvansa Jul 27, 2015 6:24 AM (in response to tomjenkinson)

Hi, thanks for the response. The reason for the check is that I have used catch clause with tx.markRollbackOnly(), instead of introducing my own variable.

However, it's not really an answer for my question - I was asking about JTA spec, not for Narayana - though I'd expect you to know exactly what the JTA spec defines/requires/prohibits.

I can ask the question the other way, too: could it be an error to call rollback after failed commit? Besides few wasted CPU cycles, could that throw another exception? I hope that it can't trigger e.g. the synchronizations to be ran second time.
Actions
3. Re: commit() throws exception: do we need to call rollback()?

mmusgrov Jul 27, 2015 7:23 AM (in response to rvansa)

The main thing you want to achieve is that when the method ends no transaction is associated with the thread used to run the method. The condition (tx.getStatus() == Status.ACTIVE) will give you that information so if the transaction is still active you should call rollback (although the commit call will generally resulted in the disassociation). The other states where there is still an association will be managed by the transaction manager. The method setRollbackOnly does not affect the association. The authoritative source I used here was the javadoc for javax.transaction.UserTransaction and javax.transaction.Status
Actions
4. Re: commit() throws exception: do we need to call rollback()?

rvansa Jul 27, 2015 7:43 AM (in response to mmusgrov)

mmusgrov: Could you get to a state where tx.getStatus() returns Status.ACTIVE after tx.commit()? So I need to check the status when handling exception thrown from commit()? I guess that if commit() fails and the transaction was associated, I would call only rollback(), never commit() again. But I was in doubt whether it may be required to call rollback() even once.
Actions
5. Re: commit() throws exception: do we need to call rollback()?

mmusgrov Jul 27, 2015 8:18 AM (in response to rvansa)

Yes it is possible, though uncommon, for the status to still be active even after calling commit. I would recommend coding it like this:

try {
...
commit()
} finally {
if (tx.getStatus() == Status.ACTIVE)
rollback();
}

And if you called rollback in the try block (instead of the commit) then there is nothing you can do in the finally block if the transaction is still active except perhaps log an error or warning
Actions
6. Re: commit() throws exception: do we need to call rollback()?

rvansa Jul 27, 2015 8:38 AM (in response to mmusgrov)
mmusgrov : And how does that match to JTA spec? Citing JTA 1.1. spec from http://download.oracle.com/otn-pub/jcp/jta-1.1-spec-oth-JSpec/jta-1_1-spec.pdf, section 5, TransactionManager (regrettably the description of Transaction is shorter):

public void commit() throws RollbackException, HeuristicMixedException, HeuristicRollbackException, SecurityException, IllegalStateException, SystemException Complete the transaction associated with the current thread. When this method completes, the thread becomes associated with no transaction.

And when looking for Status.ACTIVE, it looks like active implies being associated:

public final static int STATUS_ACTIVE A transaction is associated with the target object and it is in the active state. An implementation returns this status after a transaction has been started and prior to a transaction coordinator issuing any prepares unless the transaction has been marked for rollback

So is the behaviour that you describe rather a quirk of the Narayana implementation? Or do I understand the word 'complete' (in a meaning of control flow exiting either through return or exception) in spec incorrectly?
Actions
7. Re: commit() throws exception: do we need to call rollback()?

mmusgrov Jul 27, 2015 9:22 AM (in response to rvansa)

When we process the commit we do not catch RuntimeExceptions so that is one possibility for why the thread might still be associated. I we did the disassociate for you then your error handling logic would be more complex - it gives you a chance to recovery more gracefully if you still have access to the transaction. Also prior to commit your own code may throw exceptions leaving the thread associated. So having a finally block where you do the status check is good safe programming practice.
Actions
8. Re: commit() throws exception: do we need to call rollback()?

sebersole Jul 28, 2015 9:45 AM (in response to mmusgrov)

For what it is worth everyone... Radim is not asking about something he sees specifically with Narayana.

The question stems from Hibernate ([HHH-9976] JdbcResourceLocalTransactionCoordinatorImpl does not rollback on failure during #beforeCompletionCallback - H…). Hibernate has to also deal with straight-up JDBC transactions. We are just trying to make the behavior of our Transaction interface that apps use consistent whether the backend is a JDBC transaction or a JTA transaction. At the moment, when the commit on the JDBC transaction fails we simply mark the Transaction for rollback only. The idea behind our Transaction interface is to abstract the application away from the underlying transaction mechanism. As such, we just want to make this consistent with the JTA spec.
Actions
9. Re: commit() throws exception: do we need to call rollback()?

tomjenkinson Jul 28, 2015 2:49 PM (in response to sebersole)

Thanks for clarifying Steve. Unless some java.lang.Error is thrown on the XAResource (indicating non-compliance of that resource) when commit completes the transaction will be committed or rolled back: TransactionManager (Java EE 6 )

I imagine the wording around Synchronizations could be some relic of OTS days and Synchronizations in other address spaces.

You don't want to call rollback on a rolled back transaction as that will raise an illegalstateexception as per: TransactionManager (Java EE 6 )
Actions
10. Re: commit() throws exception: do we need to call rollback()?

tomjenkinson Jul 28, 2015 2:51 PM (in response to mmusgrov)

Michael Musgrove wrote:

When we process the commit we do not catch RuntimeExceptions so that is one possibility for why the thread might still be associated

Interestingly we do process RuntimeExceptions https://github.com/jbosstm/narayana/blob/master/ArjunaJTA/jta/classes/com/arjuna/ats/internal/jta/resources/arjunacore/XAResourceRecord.java#L543 We don't process Errors.Potentially we could consider not processing RuntimeExceptions as it would also cater for non-compliant XAResources which is possibly an issue you don't want to mask.
Actions
11. Re: commit() throws exception: do we need to call rollback()?

marklittle Jul 29, 2015 7:43 AM (in response to rvansa)

If nested transactions are supported then upon commit of a subtransaction the thread may be associated with the parent, which could be in the ACTIVE state.
1 of 1 people found this helpful
Actions

Go to original post