[mmusgrov]

Mark Little wrote:

Michael Musgrove wrote:

Mark Little wrote:

Wouldn't make sense to add this to BA if it looks and feels like a synchronisation specific thing but is called Fred, for instance.

Why are thread association changes synchronisation specific. If you argue that they are then the logical consequence would be that ThreadActionData calls into BasicAction are misplaced.

I didn't say thread association was synchronisation specific. What I meant was really two things: first if the class/interface has a name which implies synchronisations then it shouldn't go into BA; second if the class/interface has extensions that only make sense for, or imply, synchronisations then it shouldn't into BA.

We haven't agreed on a name nor have we agreed that it should extend Synchronization. Also we haven't yet agreed on an interface. The only fact is that we need a callback mechanism when a transaction is associated/disassociated.

But I do hear what you are saying with regards to the performance overhead if we make this thing too generic. Currently we have the code in TransactionImple in the JTA layer and the code that does the callback checks is in BaseTransactionManagerDelegate both of which minimize the impact on other users of ArjunaCore. Since this approach solves Scotts problem should we avoid making this mechanism too generic?

[marklittle]

BTW check out CurrentImple.java and ThreadAssociationControl.java

[marklittle]

My temptation would be to just solve the problem at hand - optimise for this case and don't impose any overhead on other users/applications. If you take a look at those two classes I mentioned, you'll see we did do something similar to this in the JTS but it was generic and it was put into the class applications were meant to use for driving transaction creation and termination, i.e., not in the transaction class itself.

[smarlow]

Could the transaction level resource map, always be available to the last callback (whether its via ThreadAssociationListener.threadDisassociated() or Synchronization.afterCompletion(int status)?  That is where the deferred (JPA) callbacks need to be referenced from for what I hope to propose as our JPA level extension that persistence providers could use in the future. 

[mmusgrov]

Isn't this a private implementation concern. The contract is: you register a callback with us and we invoke it at the appropriate points.

If you need to associate some information with the call backs then we could modify the registration interface to allow you to pass in some data which we will then pass back to you during the callback - this is similar to what Jesper had in his proposal.

[smarlow]

The easiest would be to make the transaction level resource map also available to the registration interface but if its easier to pass separate data into the registration interface, that could also work. 

[marklittle]

Scott Marlow wrote:

The easiest would be to make the transaction level resource map also available to the registration interface but if its easier to pass separate data into the registration interface, that could also work.

Got an example pseudo-code to illustrate?

[smarlow]

I meant, that it would be easiest for the WildFly (JPA) integration level, to have access to the transaction level resource map but a separate data object passed into the registration could also work.

[smarlow]

I do not have any pseudo-code to illustrate the TM facing changes. 

[mmusgrov]

Scott Marlow wrote:

The easiest would be to make the transaction level resource map also available to the registration interface but if its easier to pass separate data into the registration interface, that could also work.

Separate data would be preferable since then I can explore an implementation based on a solution Mark mentioned in comment #16 which would not use resource maps. The internal implementation would just store the listeners in a map keyed by the thread.

[smarlow]

Was just thinking that if we called ThreadAssociationListener.threadDisassociating() before the thread is disassociated from the transaction instead of calling ThreadAssociationListener.threadDisassociated() after it is disassociated, then we would just need to make sure the tx resource map is available during the ThreadAssociationListener.threadDisassociating().

This is also important to ensure that the thread environment is the same as it would of been during the Synchronization.afterCompletion call (except for ordering of course, since the other afterCompletion callbacks have already been called.)

If there are other needs for passing data into the registration call, I'm not against that but I think the above will help me (as long as the tx resource map is always available to the ThreadAssociationListener.threadDisassociating()). 

[tomjenkinson]

To allow access to resources in the TSR still we will need to invoke the callback immediately prior to narayana/AtomicAction.java at master · jbosstm/narayana · GitHub

In terms of naming that would mean we should use something like: beforeDisassociated()

Alternatively we could provide a representation of the transaction resources back to the client. The problem with that is that there is more than just TSR resources in narayana/TransactionImple.java at master · jbosstm/narayana · GitHub

Its deprecated code but org.jboss.tm.TransactionLocal stores some locks in there, whether WFLY still uses that I don't know.

[tomjenkinson]

Having thought some more about this, I think that calling the disassociated() method so late is a problem as it will never be called before afterCompletion - never giving JPA the chance to clean up the EM at the correct point. If we want to keep the facility within Narayana we need consider how to get the callback currently known as "disassociated()" called prior to the Sync::afterCompletion() calls.

I am instead wondering whether we should look to extend: https://github.com/jbosstm/jboss-transaction-spi/blob/master/src/main/java/org/jboss/tm/usertx/UserTransactionListener.java

We could add a userTransactionCompleting() method to be called prior to jboss-transaction-spi/ServerVMClientUserTransaction.java at master · jbosstm/jboss-transaction-spi · GitHub and https://github.com/jbosstm/jboss-transaction-spi/blob/master/src/main/java/org/jboss/tm/usertx/client/ServerVMClientUserTransaction.java#L182

We would also need a TransactionManagerListener with similar callbacks at commit and rollback as above, plus a callback for suspend when CMT is returning control to another app server. If we do this instead, we no longer need the threadAssociated calls but the interface is much less related to thread association and more about if a user (or CMT) is completing the transaction versus the reaper. Its also possible to implement all this for BMT solely in the SPI, a change to WFLY CMTTxInterceptor would also be needed to invoke the callbacks for CMT. It would have no Narayana code changes.

[marklittle]

Why were you thinking about AtomicAction.java?

[mmusgrov]

1) What's wrong with the client passing the information required for cleanup during registration.

2) If the purpose of ensuring that disassociate is called before afterCompletion is so that the TSR resources are available to the listener then we could cache it for him but this is resource hungry and not as generic as 1).

3) You say that you want disassociate called before afterCompletion. Narayana code keeps the transaction associated when afterCompletion is called so we could never implement this unless we really do disassociate the transaction from the thread prior to calling afterCompletion.