Reset the rollbackOnly marker when a managed connection is recycled#2253
Open
BassemH wants to merge 1 commit into
Open
Reset the rollbackOnly marker when a managed connection is recycled#2253BassemH wants to merge 1 commit into
BassemH wants to merge 1 commit into
Conversation
LocalAndXATransaction.end(xid, TMFAIL) marks the TransactionContext rollbackOnly so that no further work is done in the transaction that has just failed (AMQ-7485). The marker is only ever cleared by TransactionContext.begin() or start(xid, flags), that is, at the next transaction boundary on that context. In the resource adapter the TransactionContext is owned by ActiveMQManagedConnection, so it lives as long as the pooled physical connection rather than as long as the transaction. When the application server returns such a connection to the pool after a rolled back transaction and later hands it out again for work that is not part of a transaction, there is no new transaction boundary, the marker is still set, and ActiveMQSession.send() rejects every send with "transaction marked rollback only". The connection stays unusable for non transacted sends until it happens to be enlisted in a transaction again. Clear the marker in LocalAndXATransaction.cleanup(). That method is only reached from ManagedConnection.cleanup(), which is the point at which the container recycles the physical connection, and it already discards the other transaction scoped state via TransactionContext.cleanup(). end(TMFAIL) still leaves the marker set for the session that took part in the failed transaction, so the behaviour asserted by AMQ-7485 is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
LocalAndXATransaction.end(xid, TMFAIL)marks theTransactionContextrollbackOnlysothat no further work is done in the transaction that has just failed, and
ActiveMQSession.send()refuses to send while that marker is set (AMQ-7485). The markeris cleared only by
TransactionContext.begin()andstart(xid, flags), that is, at thenext transaction boundary on that context, which matches the stated intent of AMQ-7485:
"failed ended transactions prevent further work till next transaction boundary".
In the resource adapter, though, the
TransactionContextis not transaction scoped.ActiveMQManagedConnectionholds it in afinalfield created from the physicalconnection, so it lives as long as the pooled connection, and every session handed out
from that managed connection observes it because
ManagedTransactionContext.isRollbackOnly()consults the shared context unconditionally:So when a container returns a managed connection to the pool after a rolled back
transaction and later hands it out for work that is not part of a transaction, there
is no next transaction boundary. Nothing clears the marker, and every send on that pooled
connection fails with:
The connection stays unusable for non transacted sends until it happens to be enlisted in
a transaction again.
ManagedConnection.cleanup()does not reset the marker, and becauseActiveMQManagedConnectionFactorydoes not implementValidatingManagedConnectionFactorythe container cannot detect or evict the affected connection either.
This is easy to hit on a JCA container. IronJacamar's
TxConnectionListener.delist()delists with
TMFAILwhenever the transaction status isSTATUS_MARKED_ROLLBACK, andNarayana's
XAResourceRecord.topLevelAbort()ends the branch withTMFAIL, so anyrolled back transaction that touched the RA leaves that pooled connection unusable for
subsequent non transactional sends. We hit this on JBoss EAP 8 / WildFly after upgrading
the client from 5.9.1 to 6.2.4; every
@TransactionAttribute(NOT_SUPPORTED)send in theapplication started failing intermittently, and the reported error mentions a transaction
even though no transaction is involved at the point of failure.
Fix
Clear the marker in
LocalAndXATransaction.cleanup().That method is only reached from
ManagedConnection.cleanup(), which is the point wherethe container recycles the physical connection, and it already discards the rest of the
transaction scoped state by delegating to
TransactionContext.cleanup()(which nullsassociatedXidandtransactionId, but notrollbackOnly).end(TMFAIL)sets the marker aftersetInManagedTx(false), so a session that tookpart in the failed transaction still sees it. The behaviour AMQ-7485 added, and the
testSendOnAbortedXATxtest that asserts it, are unchanged.Test
JmsXAQueueTransactionTest#testSendOnRecycledManagedConnectionAfterAbortedXATxaborts anXA transaction with
TMFAIL, recycles the managed connection viaManagedConnection.cleanup(), then obtains a fresh handle and sends outside of anytransaction.
Without the change it fails:
With the change the whole
activemq-ramodule passes:which includes
testSendOnAbortedXATx,JmsXARollback2CxTransactionTest,FailoverManagedConnectionTestandFailoverManagedClusterTest(the failover withpending acks scenario that AMQ-5854 originally introduced the marker for).
The change also cherry-picks cleanly onto
activemq-6.2.x.I was not able to open a JIRA issue for this (ASF JIRA no longer allows self service
account creation), so the commit carries no issue key. Happy to add one if a committer
creates the issue.