marc fleury wrote:
>
> |You are talking about Transaction.registerSynchronization()
> |throwing a RollbackException if the transaction is marked
> |rollback only?
>
> yes that is one and also the fact that you had added a "check" somewhere to
> fix the JTA warning that essentially threw us off...
My bad: I wanted the TM to log a warning in this case,
but I didn't want the warning logged where the code was
already fixed. So I added this check here to make the
code behave as if an exception was thrown.
> |Personally I do not like it, as I think it makes no sense.
> |After all, we might still want afterCompletion called
> |after the rollback has happened.
>
> yes, that is a solution
Yes, but IMHO not the right solution, as JTA doesn't
say so.
> |On my first reading of the JTA specification I thought that
> |it was quite explicit and consistent about this: RollbackException
> |_must_ be thrown in this case.
> |For comparison, CORBA OTS takes a more sensible stand on
> |this: The RollbackException _may_ be thrown in this case.
> |J2EE RI _always_ throws a RollbackException in this case.
>
> well that is the other solution, the container will take a decision based on
> it.
>
> Either way I want to know, either through the callback, either through the
> exception that I should free the context. As it was neither was true.
>
> |But after having another look on the JTA specification and
> |the JTA javadoc, I am not so sure any more. It seems that
> |it is the same sentence that has been cut-n-pasted:
> |"RollbackException: Thrown to indicate that the transaction
> |has been marked for rollback only."
> |Please note that is does not explicitly say that this
> |exception _must_ be thrown.
>
> Yes, I understand. Again either way is fine, but let there be one :)))
OK. Where JTA is not quite clear, I think we should try
to mirror the behaviour of J2EE RI.
That means: registerSynchronization() should _always_
throw a RollbackException when called on a transaction
that has been marked for rollback only.
The code for this is already present (TxCapsule line 647),
but has so far been commented out.
Of course removing this comment means that the
Synchronization will not be invoked later on when a
RollbackException is thrown from the
registerSynchronization() method.
That is the way J2EE RI works.
But: I think that removing this comment will make the
code in EntitySynchronizationInterceptor.java line 115
behave like it did before the check was removed. (The
exception catching code does exactly the same as if my
check failed.)
Still I think that the TxCapsule line 647 comment should
be removed so our JTA implementation behaves like J2EE
RI. But probably some other change is needed in
EntitySynchronizationInterceptor for this to work.
> |> we have a "fix" which basically is to remove the Ole "check" but
> |it remains
> |> Aaron that I don't think Minerva should rollback transactions in
> |his corner,
> |> that decision is up to the application or container. At any rate it will
> |> show that being bearer of bad news usually means bad news for
> |one-self ;-).
> |
> |While I do not know if the TX rollback in Minerva is right
> |or wrong, it should not be a problem.
>
> it is not right or wrong, what was wrong was enable the extra check (now
> under SA FIXME I believe)
Looks like Sebastien has commented out this check in
EntitySynchronizationInterceptor.java line 115.
It might as well be completely removed.
> |Why? Because asynchronous transaction rollbacks happen, and
> |there is very little we can do about it when we interoperate
> |with other transaction managers.
> |JTA throws RollbackException and IllegalStateException all
> |over the place mostly because of this. It should be simple
> |for a JTA user to just check getStatus() to avoid getting
> |one of these exceptions, but it is not because of the
> |asynchronous rollback.
>
> sure, again as long as the container has a way either actively (call a
> method) or passively ( a callback or an exception) then I have a way to do
> the proper thing in the container which is ok... Again that part is ok right
> now (with the "check" removed :)
OK. Then it should be an exception (like in J2EE RI).
This is the only way we are going to interoperate with
other TMs without (IMHO unsolvable) problems.
> |I did this in the transaction timeout, and this has now been
> |changed to just marking for rollback only.
> |Even if we interoperate with a transaction manager that has
> |checked transaction behaviour, this TM may rollback while
> |calls are active in jBoss. This TM and the code that starts
> |and terminates transactions may be completely outside our
> |control. If we try to play games like always interposing and
> |delaying rollback requests until all invocations in the TX
> |are done, the delay introduced by this will make jBoss look
> |faulty and be generally unacceptable for most people who want
> |to do distributed transactions in large systems.
>
> you lost me there, if the tx is rollback ALL I want is to be notified.
> Real-time (with an exception) is ok.
The point I am trying to make is that an exception is the
only viable solution if we are going to:
a) Keep the jBoss container code independent of the
JTA implementation used.
b) Interoperate with other distributed TMs.
> |Making TX timeout only mark the TX for rollback only, and
> |making registerSynchronization() never throw RollbackException
> |may be a good thing to do now we are so close to the deadline.
> |
> |But I consider these only fixes for the symptoms of a larger
> |problem: jBoss has problems with asynchronous TX rollbacks.
>
> I don't see that... if you let the manager either callback or throw an
> exception then I will be notified that another thread has touched the tx...
> bear in mind that we don't make assumptions about single thread ( ;-) of
> control in the TX. All we need is the proper notification.
I think notification in the form of an exception is
done everywhere in the TM, except for the registerSynchronization()
method where the code is commented out and replaced by a warning.
As for whether the container code is fully able to handle
TM exceptions everywhere: I do not know this code well enough
to really tell you, but you should check up on it with Sebastien.
He has changed the transaction timeout method to just do the same
as setRollbackOnly() on timeout instead of doing the same as
rollback(). Probably this is due to some problem with an
asynchronous rollback that does not occur with an asynchronous
setRollbackOnly().
> I need to accept the "registration" of a synchronization on a "marked for
> rollback transaction". If you want to throw an exception right away, we are
> ok with that, if you want to allow registration and allow the "commit" to go
> to a "afterCompletion(false)" which it does now with the fixed CVS, that is
> fine as well. But maybe I miss your point.
I think you got my point.
Just be sure you realize that when an exception is thrown
from the TM interface implementations it generally means
that the requested operation was _not_ done. Only exception
to this rule is the heuristic exceptions that can mean
partially done.
So if you do tx.registerSynchronization(sync) and get
a RollbackException, it will (when the TxCapsule comment
is removed) mean that sync was _not_ registered with the
transaction.
Best Regards,
Ole Husgaard.
