how EJB3 CMT should handle different tx status flags

[smarlow]

Related to the how app thread should clear tx with status STATUS_ROLLEDBACK discussion, I would like feedback on a proposed EJB3 (CMT) container patch that handles ending the transaction differently for each tx status.  This is in the EJB3 CMTTxInterceptor.endTransaction() that handles ending the tx that a CMT bean is using.  Can you recommend how we should handle each different transaction status (if different then described):

Status.STATUS_ACTIVE - we still call tm.commit() as we did before the proposed patch.

Status.STATUS_MARKED_ROLLBACK - we still call tm.rollback() as we did before the proposed patch.

Status.STATUS_ROLLEDBACK - instead of calling tm.commit(), we now call tm.suspend to clear the tx from the thread.

Status.STATUS_NO_TRANSACTION - instead of calling tm.commit(), we now call tm.suspend to clear the tx from the thread.  Previously, we called tm.commit() which would of generated a failure.  I think that clearing the "no_transaction" tx from the thread is right but I'm not sure if seeing the status, could mean an invalid state.  If this could mean an invalid state, then throwing an exception might be better then what I did (changed to not throw an exception).

Status.STATUS_COMMITTED - instead of calling tm.commit(), we now call tm.suspend to clear the tx from the thread.  Previously, we called tm.commit() which would of generated a failure.  This  sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_PREPARED  - we now call tm.rollback() instead of tm.commit().  This sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_PREPARING - we now call tm.rollback() instead of tm.commit().  This  sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_ROLLING_BACK - we now call tm.rollback() instead of tm.commit().  This sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_UNKNOWN - we now call tm.rollback() instead of tm.commit(). 

Thanks,

Scott

[tomjenkinson]

Hi Scott,

Status.STATUS_ACTIVE - we still call tm.commit() as we did before the proposed patch.

Status.STATUS_MARKED_ROLLBACK - we still call tm.rollback() as we did before the proposed patch.

Status.STATUS_ROLLEDBACK - instead of calling tm.commit(), we now call tm.suspend to clear the tx from the thread.

Status.STATUS_NO_TRANSACTION - instead of calling tm.commit(), we now call tm.suspend to clear the tx from the thread.  Previously, we called tm.commit() which would of generated a failure.  I think that clearing the "no_transaction" tx from the thread is right but I'm not sure if seeing the status, could mean an invalid state.  If this could mean an invalid state, then throwing an exception might be better then what I did (changed to not throw an exception).

Status.STATUS_COMMITTED - instead of calling tm.commit(), we now call tm.suspend to clear the tx from the thread.  Previously, we called tm.commit() which would of generated a failure.  This  sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

I agree with these, and also agree it would be invalid to get a committed state at the end of CMT interceptor

Status.STATUS_PREPARED  - we now call tm.rollback() instead of tm.commit().  This sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_PREPARING - we now call tm.rollback() instead of tm.commit().  This  sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_ROLLING_BACK - we now call tm.rollback() instead of tm.commit().  This sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_UNKNOWN - we now call tm.rollback() instead of tm.commit().

From looking at the code I do not think it is possible for the transaction to be in one of these statuses by the CMT interceptor. There is synchronization in BasicAction so I do not expect these statuses to be visible by say a parrallel execution of the reaper.

[smarlow]

Status.STATUS_PREPARED  - we now call tm.rollback() instead of tm.commit().  This sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_PREPARING - we now call tm.rollback() instead of tm.commit().  This  sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_ROLLING_BACK - we now call tm.rollback() instead of tm.commit().  This sounds like an invalid state to find the tx in (might also want to throw an exception for this case).

Status.STATUS_UNKNOWN - we now call tm.rollback() instead of tm.commit().

From looking at the code I do not think it is possible for the transaction to be in one of these statuses by the CMT interceptor. There is synchronization in BasicAction so I do not expect these statuses to be visible by say a parrallel execution of the reaper.

I'll change the way we handle { prepared, preparing, rolling_back, unknown) to also call tm.suspend() and throw an EJBException.  I'll make it obvious in the EJBException message, which tx status is observed.

[tomjenkinson]

That sounds quite appropriate to me, at least for the PREPARED, PREPARING and ROLLING_BACK status. Essentially you are saying the transaction manager is dealing with this transaction, just suspend from the thread, even if in practice these states should not be obtainable from your code if it is using JBoss Transactions (as it would be in WildFly).

In terms of STATUS_UNKNOWN, it is in a different "category" for me. This is the text in the spec "A transaction is associated with the target object but its current status cannot be determined. This is a transient condition and a subsequent invocation will ultimately return a different status." Again, for JBoss Transactions you should not be able to get this status so now we are in the scenario where somehow the transaction manager is modified in such a manner that this state becomes visible. In that case the safest thing is to retry getStatus until you get a valid status (forever/configurable?), this may not be desirable in practice. Simply calling suspend wouldn't be correct if eventually the status would eventually result in a commit (Status.STATUS_ACTIVE) or rollback (Status.STATUS_MARKED_ROLLBACK).

[smarlow]

In terms of STATUS_UNKNOWN, it is in a different "category" for me. This is the text in the spec "A transaction is associated with the target object but its current status cannot be determined. This is a transient condition and a subsequent invocation will ultimately return a different status." Again, for JBoss Transactions you should not be able to get this status so now we are in the scenario where somehow the transaction manager is modified in such a manner that this state becomes visible. In that case the safest thing is to retry getStatus until you get a valid status (forever/configurable?), this may not be desirable in practice. Simply calling suspend wouldn't be correct if eventually the status would eventually result in a commit (Status.STATUS_ACTIVE) or rollback (Status.STATUS_MARKED_ROLLBACK).

For STATUS_UNKNOWN, is there any option other than retrying getStatus until we get a valid status?  Would calling tm.rollback() also be wrong?

[tomjenkinson]

I guess it would depend on the transaction manager implementation as that state isn't well defined in the spec as to what points it might be observed. Potentially yes, calling rollback may be the safest thing to do, potentially calling commit is OK as well. I think completing the transaction would be a good call, as to which way to complete it I don't really have any definitive guidance. I would err on the side of optimism and call commit I think. As I mentioned, from what I can tell this state will not be seen using JBoss Transactions/Narayana. The worst I can think would happen is an error in the log.

[tomjenkinson]

I think I misunderstood the initial question sorry so I will temporarily remove the correct answer flag.

I thought that the question was how to handle it in a client, I now realise although its the app thread it is actually container code that is processing a tx.commit or tm.commit:

In that case:

Status.STATUS_ACTIVE

call tm.commit()

Status.STATUS_MARKED_ROLLBACK

Either:

* Still call tm.commit so you can report the exception back to the client that they intended commit and the outcome was a rollback

* Call tm.rollback but report the exception to the client

Status.STATUS_ROLLEDBACK

Can happen if reaper has rolled back tx

Call tm.suspend to clear the tx from the thread and throw an exception to the client

Status.STATUS_NO_TRANSACTION

This is an invalid state so call tm.suspend to clear the tx from the thread and throw an exception to the client

Status.STATUS_COMMITTED

Handle as invalid state above

Status.STATUS_PREPARED

Handle as invalid state above

Status.STATUS_PREPARING

Handle as invalid state above

Status.STATUS_ROLLING_BACK

Can happen if reaper is rolling back tx

Either:

* Call tm.suspend to clear the tx from the thread and throw an exception to the client saying the tx is rolling back

* Call tm.rollback to clear the tx from the thread and throw an exception to the client saying the tx rolled back - may lock but at least you can return a complete condition to the client

Status.STATUS_UNKNOWN

Handle as invalid state above

Hope that clarifies,

Tom

[tomjenkinson]

Subsequent to [WFLY-4327] when background tx-timeout rolls back the transaction, application thread should call tx.rollback instead of…:

Status.STATUS_ACTIVE

* call tm.commit()

Status.STATUS_COMMITTING:

Status.STATUS_COMMITTED:

Status.STATUS_PREPARED:

Status.STATUS_PREPARING:

* Someone has called commit in parallel and the transaction is trying to commit so try to commit too: call tm.commit

Status.STATUS_MARKED_ROLLBACK

* Call tm.rollback (report a new exception to the client, if appropriate - unlikely to be)

Status.STATUS_ROLLEDBACK:

Status.STATUS_ROLLING_BACK:

* Call tm.rollback. Can happen if reaper has rolled back tx or another thread has done so. Will lock until tm.rollback completes. Will result in an IllegalStateException with error code 16063 and message "The transaction is not active"

Status.STATUS_UNKNOWN

This is an invalid state so you could call tm.rollback if appropriate to clear the tx from the thread and throw an exception to the client.

Status.STATUS_NO_TRANSACTION

Should likely need no action - you could call tm.suspend or nothing.

[smarlow]

Status.STATUS_COMMITTING:

Status.STATUS_COMMITTED:

Status.STATUS_PREPARED:

Status.STATUS_PREPARING:

* Someone has called commit in parallel and the transaction is trying to commit so try to commit too: call tm.commit

For "committing/preparing", do we need to be concerned with waiting for the "in parallel" operation to complete before we can get the error code that reflects the (background) operation?

[tomjenkinson]

For CMT it should not be possible to get to those states (unless some future extension allowed it somehow).