Alrighty. I've got "transactionSupport" ready. Apparently that hasn't
worked for some time.
The nice thing too is that gives the user an "out" if they want to revert
to the non-spec behavior: Right now, connection factories are non-xa, by
the spec says they should be by default. If someone upgrades and desires
the old non-xa behavior, they can set transactionSupport=none on the
connection factory.
I'm fairly confident in this merge request, the only part I really don't
know about is the enlistResource() when I enlist XASession. It works
though, and from the examples I've seen it seems to be the correct way to
enlist a resource. I've just never messed with that API before.
Anyway, eyes appreciated, after I update my PRs on github I'll give it a
day for review and then merge it in.
Thanks!
On Tue, Aug 27, 2019 at 9:27 PM Jonathan S. Fisher
wrote:
> Just noticed in JmsConnectionFactoryBuilder this is present already with
> the attribute "transactionSupport". I need to tie that into my patch before
> it's merged
>
> On Tue, Aug 27, 2019 at 4:30 PM Jonathan S. Fisher
> wrote:
>
>> I just checked Wildfly, they do the same thing as Liberty. I agree with
>> your statement for the "completely correct" fix, ideally that's the place
>> to do it, but might take awhile to get a release out.
>>
>> On another note: I know the spec says, "Ignore all arguments to
>> connection.create*(int mode)" methods. Yet I can think of a lot of
>> scenarios where having a non-JTA connection pool is very handy (for
>> instance, logging over JMS). We have the option to have non-JTA Database
>> connections, I feel though we should be able to declare whether or not a
>> jms connection pool participates in JTA.
>>
>> I'm thinking maybe we should have an `xa=true/false` parameter in the
>> connection pool declaration. Would that be ok?
>>
>>
>> On Tue, Aug 27, 2019 at 3:43 PM David Jencks
>> wrote:
>>
>>> I checked the Open Liberty TransactionSynchronizationRegistry, and it
>>> interprets “active transaction” to mean “any transaction on the thread, no
>>> matter it’s state”. So I think that it would be best to decide to do the
>>> same in the Geronimo TM, deciding that the java doc is ambiguous as to the
>>> meaning of “active” and the most useful meaning can be picked rather than
>>> the most literal.
>>>
>>> Whether this is practical for the next TomEE, I don’t know.
>>>
>>> David Jencks
>>>
>>> > On Aug 27, 2019, at 8:25 AM, David Jencks
>>> wrote:
>>> >
>>> > I think the java doc for getResource might have been written
>>> thoughtlessly, and more appropriate behavior would be an ISE only for
>>> STATUS_NO_TRANSACTION; literally the geronimo implementation is too lax, as
>>> “marked rollback” is not status “active”. Is there anyone who’s opinion we
>>> might ask?
>>> >
>>> > I rather thought the “ignore session type” logic was supposed to be
>>> put into the RA, but I don’t recall if or how I dealt with this in Geronimo.
>>> >
>>> > So I’d prefer these issues be dealt with elsewhere but don’t see much
>>> practical alternative to your implementation.
>>> >
>>> > Nice to see someone working on XA:-)
>>> >
>>> > thanks!
>>> > David Jencks
>>> >
>>> >> On Aug 26, 2019, at 1:45 PM, Jonathan S. Fisher
>>> wrote:
>>> >>
>>> >> I've narrowed down the problem to AutoConnectionTracker. It's not
>>> >> completing, which isn't allowing the connections to be returned to
>>> the pool.
>>> >>
>>> https://github.com/apache/tomee/blob/master/container/openejb-core/src/main/java/org/apache/openejb/resource/AutoConnectionTracker.java#L174
>>> >>
>>> >> getResource() is throwing an IllegalStateException. The JavaDoc (
>>> >>
>>> https://docs.oracle.com/javaee/7/api/javax/transaction/TransactionSynchronizationRegistry.html#getResource-java.lang.Object-
>>> )
>>> >> states it should throw an ISE if a current transaction is not Active.
>>> The
>>> >> transaction is in the state ROLLED_BACK when AutoConnectionTracker
>>> tries to
>>> >> call getResource().
>>> >>
>>> >> I think the Geronimo implementation (
>>> >>
>>> https://github.com/apache/geronimo-txmanager/blame/trunk/geronimo-transaction/src/main/java/org/apache/geronimo/transaction/manager/TransactionManagerImpl.java#L203
>>> )
>>> >> maybe be a little too strict. The JTA Spec pdf doesn't offer exact
>>> hints of
>>> >> which statuses (
>>> >> https://docs.oracle.com/javaee/7/api/javax/transaction/Status.html)
>>> should
>>> >> be have getResource _not_ throw an ISE unfortunately. I was thinking
>>> of
>>> >> changing Geronimo's implementation to check for anything
>>> >> but STATUS_UNKNOWN, and STATUS_NO_TRANSACTION.
>>> >>
>>> >> The other way is to cast Transaction to the Geronimo implementation
>>> and use
>>> >> Geronimo specific APIs to get call getResource(). Do you guys have any
>>> >> preference which route I should take to fix?
>>> >>
>>> >>
>>> >> On Mon, Aug 26, 2019 at 9:15 AM Jonathan S. Fisher <
>>> exabr...@gmail.com>
>>> >> wrote:
>>> >>
>>> >>> https://github.com/exabri
