-
15. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
It sounds like the proposal is for the reaper to perform two sweeps for a transaction? i.e.:
1. At txTimeout mark the transaction as rollback only. Unfortunatley this has no effect on the synchronizations as they are not called back at this point, neither are call backs issued from the transaction manager to the XAR as they do not have a rollback only method. The only thing this does is internally prevent commit of a tx.
As an aside, for resource managers that handle transaction timeouts internally, it could cause those branches to be invalidated and allow them to return errors on in flight connections linked to those branches meaning that even if the transaction manager did nothing whatsoever (no SRO or RB) then those connections may be accessed concurrently by resource manager alone.
2. At "reaper" timeout, we would concurrently rollback the transaction, pretty much as it happens today/.
It is for this reason that I do not see how calling setRollbackOnly followed by a mandatory concurrent rollback later if the app thread has not received control back by then helps as it may just delay the calling of the concurrent rollback? A functionally similar alternative would be to for example just double the delay before calling rollback on the TX from the reaper?
-
16. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
Scott Marlow wrote:
Closing the EM, is a specification requirement for the EE (JPA) container. Detaching any loaded entities, is a spec requirement for the JPA persistence provider (only if transaction was rolled back). Detaching entities from a non-application thread will violate EntityManager concurrency. In your proposal, which thread calls the TxAssociationListener callbacks (reaper/TM communication threads or application thread that previously owned the transaction that rolled back?
It would be code added to the CMT interceptor that link wildfly/CMTTxInterceptor.java at master · wildfly/wildfly · GitHub that should mean it would be the app thread.
The reaper would call afterCompletion from a reaper thread but the TxAssociationListener would call it from the app thread. It would be up to your implementation to decide to act at afterCompletion (if its not a reaper say) or at TxAssociationListener if you detected it was not the original app thread that called afterCompletion.
-
17. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
The reaper would call afterCompletion from a reaper thread but the TxAssociationListener would call it from the app thread. It would be up to your implementation to decide to act at afterCompletion (if its not a reaper say) or at TxAssociationListener if you detected it was not the original app thread that called afterCompletion.
Sounds like we are back to the distributed problem of not know if the thread is reaper thread or not. When checking if the current thread is a reaper thread, we also need to know if we are in a communications thread that is called from a remote jvm that is a reaper thread. As far as checking for original app thread, the app thread can vary on every bean invocation when its a remote transaction.
-
18. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
The other problem is whether to handle tx timeout for non-EJB components (e.g. servlets). Wiring each different container to handle Synchronizations instead of the 3rd party jars that are used that need Transaction Synchronizations would probably require a special transaction synchronization registry that is used instead of the transaction manager TSR.
-
19. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
Scott Marlow wrote:
Detaching entities from a non-application thread will violate EntityManager concurrency.
We can of course add methods which are safe to use concurrently, for example as long as your only goal is to "terminate" the current session:
SessionImplementor#transactionTerminated( String niceErrorMessage )
this would set a volatile in the Session, and when any other exception is triggered in use code, this volatile would replace the errors with a more user friendly explanation.
Replacing the exceptions is trivial as the code for that is in place already: depending on the environment we might need to re-wrap internal Hibernate exceptions into JPA specific exceptions, so there is an interception point to allow replacing any exception with a more suited variation.
Locking the Session is a big no-go as you would lock the TransactionManager out, not making it possible to timeout runaway operations.
Discussed here: [hibernate-dev] [renamed] multiple invocations on same transaction, separate thread is not working unless is changed to …
-
20. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
Hi Scott,
Please see below for a png that might be a bit clearer. You can see how the sync gets two call backs which should be enough for you to go on whether it is safe to release the item marked as "NonConcurrentResource". If its not clear from the diagram, the if statement during afterCompletion would evaluate to false and you would not issue the close() that see in afterCompletion whereas the if statement in NEWAPI-txDisassociated would evaluate to true and you would call close()
A few details:
- In a single VM case you would release it in NEWAPI-txDisassociated regardless of whether it is timeout or none-timeout case as we know that rollback would be called before the CMT interceptor finishes so might as well leave it alone till then.
- In a multi VM case you would similarly release it in NEWAPI-txDisassociated if the timeout occurs while the Container is in use (detected by NEWAPI-txAssociated not having finished). If on the other hand control had returned to JVM1 you would have had NEWAPI-txDisassociated called some time ago and would then release it during afterCompletion.
Neither case requires you to be aware of being distributed of course. Basically you release the resource when all three of NEWAPI-txAssociated, NEWAPI-txDisassociated and afterCompletion have been called.
What do you think?
Tom
-
21. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
We can of course add methods which are safe to use concurrently, for example as long as your only goal is to "terminate" the current session:
SessionImplementor#transactionTerminated( String niceErrorMessage )
this would set a volatile in the Session, and when any other exception is triggered in use code, this volatile would replace the errors with a more user friendly explanation.
Replacing the exceptions is trivial as the code for that is in place already: depending on the environment we might need to re-wrap internal Hibernate exceptions into JPA specific exceptions, so there is an interception point to allow replacing any exception with a more suited variation.
If we are not protecting against concurrency violations and one occurs, throwing an application level friendly explanation, is better than an unexpected exception but not by much. Thinking more about this, I think that we need to avoid the concurrency violation of the Hibernate session.
Locking the Session is a big no-go as you would lock the TransactionManager out, not making it possible to timeout runaway operations.
Discussed here: [hibernate-dev] [renamed] multiple invocations on same transaction, separate thread is not working unless is changed to …
Why would locking the Hibernate Session also lock the TransactionManager out? The transaction manager rolled the transaction back already, so the application thread should soon return from the Hibernate session invocation (the application may make more calls to the Hibernate session, at which time the transaction manager thread has the Hibernate Session lock but that is only until the invalidate completes). The further calls into Hibernate should detect that the transaction status is rolledback and should fail.
-
22. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
Same question as before, how does NEWAPI-txDisassociated know that the timeout occurred?
-
23. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
We would need Status timed out state, so that the synchronization.afterCompletion(int status) knows that timeout occurred.
-
24. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
Hi Scott
Scott Marlow wrote:
Same question as before, how does NEWAPI-txDisassociated know that the timeout occurred?
It doesn't need to, the sync should keep a state machine.
Here is an example implementation:
public SyncImpl implements Synchronization, NewAPI {
public void txAssociated() {
associated = true;
}
public void txDissasociated() {
associated = false;
if (afterCompleteCalled) {
noneConcurrentResource.close();
}
}
public void afterCompletion() {
afterCompleteCalled = true;
if (!associated) {
noneConcurrentResource.close();
}
}
}
You will see I didn't need to use synchronization. This is because I know Narayana synchronizes rollback so it should not be possible for txDissasociated and afterCompletion to be called concurrently.
Tom
-
25. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
Locking the Session is a big no-go as you would lock the TransactionManager out, not making it possible to timeout runaway operations.
Discussed here: [hibernate-dev] [renamed] multiple invocations on same transaction, separate thread is not working unless is changed to …
Why would locking the Hibernate Session also lock the TransactionManager out? The transaction manager rolled the transaction back already, so the application thread should soon return from the Hibernate session invocation (the application may make more calls to the Hibernate session, at which time the transaction manager thread has the Hibernate Session lock but that is only until the invalidate completes). The further calls into Hibernate should detect that the transaction status is rolledback and should fail.
Actually, synchronizing the Hibernate session doesn't help, as the application could still change the Hibernate session state after the rollback occurs which would also violate the entity manager concurrency.
-
26. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
Would that also work when the EE containers are not in charge of transactions (e.g. UserTransaction initiated transaction)? Sounds like we need something that is more closely tied to the transaction boundaries and is constrained to only being activated in (foreground) application threads.
-
27. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
Scott Marlow wrote:
If we are not protecting against concurrency violations and one occurs, throwing an application level friendly explanation, is better than an unexpected exception but not by much. Thinking more about this, I think that we need to avoid the concurrency violation of the Hibernate session.
Sure you shouldn't access it concurrently at all. Just Narayana need to have a valid way to access it concurrently: that doesn't mean that all of public API on a Session needs to be threadsafe or locked, it just means that the methods that Narayana will invoke are a valid invocation in parallel to the user thread.
That's why I'm proposing a specific method: only that method would be designed to be safely invoked from a different thread.
Scott Marlow wrote:
Why would locking the Hibernate Session also lock the TransactionManager out? The transaction manager rolled the transaction back already, so the application thread should soon return from the Hibernate session invocation (the application may make more calls to the Hibernate session, at which time the transaction manager thread has the Hibernate Session lock but that is only until the invalidate completes). The further calls into Hibernate should detect that the transaction status is rolledback and should fail.
I am not exclusively thinking of the case in which the transaction was already rolled back. The TransactionManager might also need to interrupt the current execution, I was expecting it to invoke something on Session but I realise it could also work on the JDBC connection. Still, invoking a specific method on SessionImplementor would be cleaner I think as there might be more things in Hibernate which need to be done in this case, it would be much easier for us to do some house cleaning than to intercept some exception coming down from the database link.
So since I'm expecting you to ultimately want the TransactionManager to invoke some method on Hibernate, this shouldn't be pessimistically locking all its access points so that the TXM doesn't need to block waiting for it to finish its queries.. that would defeat the point of having a reaper.
-
28. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
Scott Marlow wrote:
Would that also work when the EE containers are not in charge of transactions (e.g. UserTransaction initiated transaction)? Sounds like we need something that is more closely tied to the transaction boundaries and is constrained to only being activated in (foreground) application threads.
Well, the same API could be used but yes what I was only showing where CMT can invoke the necessary callbacks. Please can you link me to where the registerSync call is for EE/JPA under a UT and I will see if I can spot the place that makes sense for that use case? -
29. Re: Design Discussion: Changing the reaper to use setRollbackOnly() instead of rollback()
One call site, is EntityManager.joinTransaction() which starts in Hibernate here. joinTransaction() is called for both container managed (extended persistence context when used in a UT) and also when applications call EM.joinTransaction() for an application managed persistence context.
The extended persistence context call side is here which calls into internalAssociateWithJtaTx().
Of course, the above internalAssociateWithJtaTx() may be calling into a 3rd party persistence provider as well (all JPA compliant persistence providers are expected to work).
