Don't y'all believe in summarization? ;) I'll read through this tonight/tomorrow...
On Thu, Aug 7, 2014 at 10:02 AM, Sanne Grinovero <sa...@hibernate.org> wrote: > 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 > _______________________________________________ hibernate-dev mailing list hibernate-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev