XAExpectionUnitTestCase Failures/JBossTS/JBoss 4.2

[weston.price]

In one of our unit tests

org.jboss.test.jca.test.XAExceptionUnitTestCase

we have two tests that simulate errors when a JCA resource is used in an EJB. One is a resource exception, the other a runtime exception. Both occur in the matchManagedConnections() method of of the TestManagedConnectionFactory. Being that an error/excpetion occurs, the underlying connection is destroyed prior to use; no problems here. Howerver, in one test the transaction is rolled back at the end of the in the finally block. Because the

<track-by-transaction>

flag is set to false when JBossTS attempts any sort of activity on the underlying resource it fails because the resource has already been destroyed. JBossTM seemed to either mask this condition, or just ignore it all together while JBossTS reports a failure which is causing an ERROR in the tests. From what I understand, it seems that the

<track-by-transaction>

flag should really be true by default for XA connections(we already do this for local resources). I can't envision why this shouldn't be set as the default moving forward with the new transaction manager stuff. In fact, most JDBC based resources from the big RDBMS vendors (Oracle, DB2) and some of the open source guys (Postgres most notably but for other reasons) already require this.

[weston.price]

I will be a bit more specific:

The tests require the failure to occur in the matchManagedConnections method. When the test originally runs, the pool is empty. As a result, a connection is successfully created and enlisted/delisted in the tranasction and returned to the pool.

The second attempt to acquire the connection fails in the match and the connection is destroyed. Howerver, JBossTS rightfully 'remembers' this guy and when JBossTS attempts to finish the txn protocol the connection has already been destroyed causing the failure.

So, effectively the <track-connection-by-txn> really should be here and in fact, I believe it's a bug that it's not.

[adrian.brock]

No.

<track-connection-by-tx/>

is there to disable transaction interleaving.
It is only relevant for XA, it must be true for local.

There has been some debate in the past on whether we should disable
interleaving by default since most XAResources don't support it.
This includes all xa datasources that I've come across which is why
their examples disable it.

e.g. possible change
1) Disable interleaving by default
2) Ignore the track-connection-by-tx
3) Add a new flag

<interleaving/>

JBossMQ supports transaction interleaving, meaning the pool
does not have to be as big as for other JMS impls! See below.

Explanation of transaction interleaving:

The purpose of interleaving is to improve the performance of the pool.

If the XAResource supports interleaving then multiple transactions can
go through it at once (although only one transaction can be associated at once).

e.g. Using a single real connection/XAResource

Tx1: Ask for a connection and enlist the xa resource
XAResource.start(xid1, TMNOFLAGS)

Tx1: Return the connection to the pool
XAResource.end(xid1, TMSUSPEND)

Tx2: Ask for a connection and enlist the xa resource (it is the same) - XAResource.start(xid2, TMNOFLAGS)

(Point A)

Tx2: Return the connection to the pool
XAResource.end(xid2, TMSUSPEND)

Tx1: Ask for a connection and enlist the xa resource - we resume the old one
XAResource.start(xid1, TMRESUME)

Additionally, the XAResource 2PC commtt protocol can be invoked
concurrently if the XAResource supports interleaving.

i.e. in the above at (Point A) the Tx1/xid1 could be committed even
though the XAResource is currently associated with xid2

As you can see. Support for interleaving greatly reduces the number
of connections required in the pool.
If you have transaction stickiness (track-connection-by-tx),
you need one connection per transaction, even if the transaction
has already tried to return the connection to the pool, but it hasn't committed it yet.

Example of where interleaving is useful:

1) get jms connection and receive a message
2) return jms connection for somebody else to use
3) do long running process (might take minutes)
4) commit

With track-connection-by-tx, the connection won't become available
until after the long running process is complete, with interleaving
somebody else can use the connection during the long running process.

[adrian.brock]

Now the explanation is over, what is the exact problem?

The whole point of the XAException test is to make sure the correct
exceptions are being reported, e.g. RuntimeExceptions are not getting leaked
where XAExceptions are expected.

[weston.price]

Now the explanation is over, what is the exact problem?

I explained this already.

The tests require the failure to occur in the matchManagedConnections method. When the test originally runs, the pool is empty. As a result, a connection is successfully created and enlisted/delisted in the tranasction and returned to the pool.

The second attempt to acquire the connection fails in the match and the connection is destroyed. Howerver, JBossTS rightfully 'remembers' this guy and when JBossTS attempts to finish the txn protocol the connection has already been destroyed causing the failure.

Basically, JBossTS is attempting to complete the XA protocol on a resource that has already been destroyed which is causing the failure presented in the JIRA issue. The tests itself, as you point out, is designed to make sure that the correct behavior occurs when a match in the managed connection fails. However, as is often the case, it has turned up this other issue.

When you say

There has been some debate in the past on whether we should disable
interleaving by default since most XAResources don't support it.
This includes all xa datasources that I've come across which is why
their examples disable it.

This is basically what I am proposing since the default setting to allow for interleaving seems to cause problems with JBossTS and XAResources.

My plan was to enable <track-connection-by-tx> by default for XA resources, and force it to be explicitly enabled for resources where this makes sense (ie JMS resources).

[weston.price]

Sorry, I probably should have linked this sooner:


http://jira.jboss.org/jira/browse/JBAS-4017

[adrian.brock]

Now the explanation is over, what is the exact problem?

I explained this already.

The explanation was incomprehensible since in principal the tests
should work in both mode interleaving and transaction stickiness modes.

I want to hear what the underlying cause is:
1) The test is broken - it is basicaclly a mock test so it is entirely possible
that it is not following the spec properly
2) JBossTS is throwing the wrong exception
3) Some other root problem

[adrian.brock]

When you say

There has been some debate in the past on whether we should disable
interleaving by default since most XAResources don't support it.
This includes all xa datasources that I've come across which is why
their examples disable it.

This is basically what I am proposing since the default setting to allow for interleaving seems to cause problems with JBossTS and XAResources.

Support for interleaving is a spec requirement, see the JTA spec section 3.4.4,
so if JBossTS is broken then it is a bug.

NOTE: Interleaving is a "gambit".

Because of interleaving you can have multiple transactions associated
with the same connection. If the connection breaks you have more
transactions failing for that single underlying failure!

[adrian.brock]

I was thinking about this over the weekend.

The best way to fix this would be to implement the XAResource wrapper
that we have been talking about for JCA.

We could then check whether the connection is closed in end()
and also reinstate the old behaviour for a failure (i.e. log a warning).

This would also require some form of xid/transaction mapping
which I'm not sure how we do generically - probably some trick
in the tx.enlistResource() call which will callback on our XAResource wrapper
with the XID?

e.g. Something like:

 public void end(Xid xid, int flags) throws XAException
 {
 // Ignore if we are already destroyed
 if (cl.getState() == ConnectionListener.DESTROYED)
 return;

 try
 {
 delegate.end(xid, flags);
 }
 catch (Throwable t)
 {
 // We log the warning and force a rollback
 log.warn("Error ending resource " + cl, t);
 try
 {
 Transaction tx = getTransaction(xid); // ?????
 if (TxUtils.isActive(tx))
 tx.setRollbackOnly();
 }
 catch (Throwable t)
 {
 log.warn("Unable to setRollbackOnly " + xid, t);
 }
 }
 }

Similar already destroyed checks would also be possible
in the other calls (e.g. prepare/commit), except there we want to raise an XAException.

[adrian.brock]

Also, prehaps the code would be better always invoking the delegate XAResource
with the end() call, but then only log the warning if we are not already destroyed,
e.g.

 public void end(Xid xid, int flags) throws XAException
 {
 try
 {
 delegate.end(xid, flags);
 }
 catch (Throwable t)
 {
 // We log the warning (UNLESS WE ARE ALREADY DESTROYED) and force a rollback
 if (cl.getState() != ConnectionListener.DESTROYED)
 log.warn("Error ending resource " + cl, t);
...

[weston.price]

Good thoughts all around. The Wrapper already exists, I just need to accomodate this condition.

[weston.price]

Also, prehaps the code would be better always invoking the delegate XAResource
with the end() call, but then only log the warning if we are not already destroyed,

This is effectively what happens now without the exception behavior. In the org.jboss.resource.connectionmanager.xa.JcaXAResourceWrapper, I simply delegate all the XA calls to the underlying XAResource with the exception of the isSameRM which gets overriden depending upon the value in the *-ds.xml file.

[weston.price]

Something about this still just doesn't seem right. I am attempting to get with the JBossTS team to discuss what, if anything, we should be doing differently. I am not sure how catching, logging (basically masking) the underlying XA exception is going to help matters. While the JcaXAResourceWrapper could do some trickery to make this problem 'go away' I am not sure this is the right approach.

In the tests we are simply reporting what could very well be a real XA exception. If this is causing an IllegalStateException in JBossTS, I would really like to know what and what the *right* way to to fix this is.

[adrian.brock]

The issue came up because the old TM throws RollbackException
when resources cannot be ended, the new TM throws IllegalStateException.

This is not catered for in TxInterceptorCMT:

 try
 {
 // Marked rollback
 if (tx.getStatus() == Status.STATUS_MARKED_ROLLBACK)
 {
 tx.rollback();
 }
 else
 {
 // Commit tx
 // This will happen if
 // a) everything goes well
 // b) app. exception was thrown
 tx.commit();
 }
 }
 catch (RollbackException e)
 {
 throwJBossException(e, invocation.getType());
 }
 catch (HeuristicMixedException e)
 {
 throwJBossException(e, invocation.getType());
 }
 catch (HeuristicRollbackException e)
 {
 throwJBossException(e, invocation.getType());
 }
 catch (SystemException e)
 {
 throwJBossException(e, invocation.getType());
 }

So the EJB caller doesn't get the same exeption
"JBossTransactionRollbackException".

This needs including in the following work:
http://jira.jboss.com/jira/browse/JBAS-3633
as a general tidying up the exception handling logging
with the changed TM semantics.