[hibernate-dev] [renamed] multiple invocations on same transaction, separate thread is not working unless is changed to false...

Thu Aug 7 11:02:29 EDT 2014

Steve,
we had a conversation about this on IRC, I think we came to a
 hopefully
nice

solution
.

[15:18] <sannegrinovero> andrigtmiller: I still think you're locking out
the TXM, not making it possible to legitimately timeout queries..
[15:19] <sannegrinovero> (but I don't have the full picture.. not really my
area)
[15:19] <smarlow> that is a transaction manager concern but they already
handled calling other synchronization call backs.  currently, Arjuna holds
a lock on the collection of synchronizations at that time, so there should
be no conflict
[15:20] <smarlow> I asked about your concern yesterday to make sure we
wouldn't deadlock
[15:20] <sannegrinovero> I wasn't actually thinking about deadlock, I just
don't see what you're aiming at with the single lock on the Session
[15:22] <smarlow> to avoid calling Hibernate session.clear from a
background thread while the application thread is also doing a Hibernate
session invocation.  which can lead to random exceptions
[15:22] <smarlow> I'm aiming at solving the concurrency concern ^
[15:22] <sannegrinovero> But I guess you'd need to talk with sebersole,
this lock thing you're proposing comes out of the blue for me :)
[15:22] <sannegrinovero> yes I get that
[15:23] <sannegrinovero> but you should allow the session.clear() to
proceed, not lock it out
[15:23] <smarlow> sure, I'm just trying to talk to who ever listens and
have feedback.  Before I get anyway near code changes, Sebersole needs to
be on board (he isn't yet)
[15:24] <sannegrinovero> I'm neither :) willing to discuss for sake of
interest, but can't replace sebersole on this I think :)
[15:24] <smarlow> I posted on as7 dev mailing list a few years ago and the
only feedback that I got was that only Hibernate was not handling
concurrency but there could be others
[15:25] <sannegrinovero> what is the effect you expect to have users see
when the background thread kills the current session?
[15:25] <smarlow> I cross posted as well
[15:25] <smarlow> users shouldn't get NPE errors or unexpected exceptions
[15:26] <sannegrinovero> sure, but what kind of effect would you propose?
[15:26] <sannegrinovero> I guess a different exception with a better error
message?
[15:26] <smarlow> my goal is ensuring that only one thread is invoking the
Hibernate session at a time
[15:27] <sannegrinovero> No that's not true, as you're saying that you want
to allow the TXM to rollback the transaction
[15:27] <smarlow> well, want is a strong word :)
[15:28] <smarlow> IBM/Oracle/JBoss all do that
[15:28] <sannegrinovero> and that's an implementation detail what you just
explained :)
[15:28] <sannegrinovero> what I'm asking is what you expect the user to
experience when this needs to happen
[15:29] <smarlow> when the transaction is rolled back, depending on where
the application code is in the Hibernate session invocation, a JDBC error
might occur or something related to that.  The goal is to avoid mutating
the Hibernate session from two different threads concurrently
[15:30] <sannegrinovero> you're not answering my question, that's an
implementation detail :)
[15:30] <smarlow> if we can avoid violating concurrency of the hibernate
session, I think we will be more robust.
[15:31] <sannegrinovero> The goal of the TXM timeout, is to kill stuff
which is taking too long.. not allowing it to finish in a safe lock.
andrigtmiller am I understanding the basics correctly?
[15:31] <smarlow> currently, what the user will experience is poor.  The
scope of turning that into a more pleasant experience is a good question
but involves many moving and separate parts
[15:32] <sannegrinovero> so you actually _need_ to allow concurrent access,
to kill and rollback the operations which are being done
[15:32] <smarlow> the goal of TXM timeout handling in JBoss/Oracle/IBM is
to also handle deadlocks that might not otherwise be recovered from
[15:32] <smarlow> the concurrent access happens at the resource level
[15:33] <smarlow> from the background, with components that handle
concurrency.  Hibernate doesn't
[15:33] <sannegrinovero> that's just one part of the things
[15:33] <sannegrinovero> and we can definitely have Hibernate handle some
concurrent events
[15:33] <smarlow> Hibernate sessions are not supposed to handle
concurrency, so its a design flaw
[15:33] <sannegrinovero> the TXM shouldn't invoke clear() (which is public
API) but invoke a specific method which could provide the needed semantics
[15:33] <smarlow> its not some events, its many events
[15:34] <sannegrinovero> why do you say it's a design flaw? it's not, for
the "normal" public API usage
[15:34] <smarlow> the TXM, calls the Hibernate
Synchronization.afterCompletion(int) and Hibernate detaches entities
[15:34] <smarlow> and Hibernate does other cleaning up as well
[15:35] <smarlow> its a design flaw as its not handled in our system
[15:35] <sannegrinovero> I don't feel I'm making progress in the
conversation if you keep repeating the implementation details, we can fix
that as we please
[15:35] <sannegrinovero> but you have to explain the expected user
experience
[15:36] <smarlow> you mean if we didn't have any constraints of any of the
existing specs that we implement?
[15:36] <sannegrinovero> yes, just explain what you think it should do
please, that would help me understand, and I think I can propose you
something more concrete
[15:37] <smarlow> if we ignore XA, JTA + JPA, I can answer the question
better and will attempt that
[15:37] <sannegrinovero> the problem is the user sees an NPE right?
[15:38] <smarlow> could be an NPE or any unexpected exception that comes
from using a thread-unsafe component from multiple threads
[15:38] <sannegrinovero> so the basic question is what do you think the
user should "see".. I guess another exception, say
HibernateRolledBackException("The Transaction Manager decided we run out of
time")
[15:38] <sannegrinovero> sure. would you like the above proposal ^
[15:39] <sannegrinovero> Because implementing that is easy :)
[15:41] <sannegrinovero> ? If you're busy I'm happy to talk later, I am 3h
late with my lunch
[15:41] <smarlow> in a perfect world, 1) the user registers an application
event listener that tells that the transaction timed out.  2) whether the
application thread catches a signal that the transaction is about to be
rolled back.  3)  The application thread then catches a signal that the
transaction was rolled back
[15:41] <sannegrinovero> that doesn't explain what you expect the client
code to experience.
[15:41] <smarlow> 4)  the application thread then catches a signal that
Synchronization call backs are going to happen to clean up after the roll
back
[15:42] <sannegrinovero> say the user was incoking a "Query.list()" .. what
do we return?
[15:42] <smarlow> what could they expect?  They could be in the middle of
code that doesn't use the Hibernate session or could be in code that does
[15:43] <sannegrinovero> Exactly my point, so we'd return an exception like
I proposed above right?
[15:43] <smarlow> if you say so but no one knows how to do that
[15:43] <smarlow> I mean, I'm not sure what returns that
[15:44] <smarlow> we had tried doing what you suggest but it was too
performance expensive and covered too few cases
[15:44] <smarlow> but sure, throwing an EJB rolledback exception is ideal
[15:45] <sannegrinovero> you can do it in an efficient way, but yes I agree
you'd have to patch several areas of code.
[15:45] <sannegrinovero> I'd do it incrementally though, start with the
Session and Query APIs, see if people like it
[15:45] <smarlow> our previous attempt that failed, added a pre-check at
the front of every Hibernate session call, tail end and middle
[15:46] <smarlow> but that didn't work because it didn't handle remote
transactions or distributed transactions
[15:46] <sannegrinovero> the EntityManager implementation has some kind of
"exception translator"
[15:46] <smarlow> and performance suffered
[15:46] <sannegrinovero> all you need to do is catch exceptions, and
re-throw the better explanation
[15:46] <smarlow> so, we have less of the checking today and still don't
handle remote transactions and distributed transactions
[15:47] <sannegrinovero> the TXM just needs to flag a volatile field in the
Session to inject the error it wants us to tell
[15:47] <sannegrinovero> so on the optimal path you just have a volatile
field read operation, which is a zero cost essentially
[15:47] <sannegrinovero> (optimal path I mean the case in which there are
no errors)
[15:48] <smarlow> then we need to poll for that flag in the
start/middle/end (or so) if every Hibernate method
[15:48] <sannegrinovero> no
[15:48] <smarlow> or maybe in the finally clause of every method
[15:48] <sannegrinovero> you just need to catch exceptions
[15:48] <sannegrinovero> and we happen to already do that, because you need
to translate all Hibernate specific exceptions to JPA specific ones
[15:49] <sannegrinovero> so it's actually a very simple patch with zero
overhead I think
[15:49] <smarlow> at worse, we catch exceptions (including NPE) and notice
the flag and then eat the cause exception?
[15:49] <sannegrinovero> +1
[15:49] <smarlow> if we show the cause, its confusing
[15:49] <sannegrinovero> you don't need the finally method either though
[15:49] <smarlow> if we eat it, its confusing
[15:49] <sannegrinovero> right, don't show the cause.
[15:49] <smarlow> I hate when exceptions are eaten
[15:50] <sannegrinovero> well.. the TXM should provide you a sensible error
message, like "aborted because of ... ", and we take that as explanation.
[15:50] <smarlow> but only if that flag is set
[15:50] <sannegrinovero> of course!
[15:50] <smarlow> sounds like a worthwhile idea, thanks
[15:51] <sannegrinovero> np, I'm very happy if it works :)
[15:51] <sannegrinovero> the exception translation is already done
somewhere in the EM project
[15:51] <sannegrinovero> I don't remember the details, but I'm sure you're
more familiar with it
[15:51] <sannegrinovero> just make sure you look that up
[15:52] <sannegrinovero> it catches all exceptions from Hibernate to wrap
them in something else, to satisfy the spec requirements

-- Sanne