I've also been following this thread for a while and I would like to add my support for a fix. We are getting more and more problems due to the issues with DTC (crashes, connection leaks etc), so a fix for this would be very much appreciated.
I'm not sure I can be of any help regarding core development, but if you need any help with testing or anything else, then I will try to do my best. However, regarding flush behaviour: IMHO, if you create a NH session inside a scope it should be the users responsibility to make sure Flush is called. Just like it's the users responsibility to call Commit if you use a TX within a session. If the tx scope is created within the session, then maybe the tx.Complete() should trigger a flush, but not the other way around. Just my 2 cents. Anyway, I really appreciate the hard work you're putting into this. Regards, Johannes 2014-04-27 11:59 GMT+02:00 Ricardo Peres <rjpe...@gmail.com>: > Good work, Yves! > > IMO, this is one of the most important things to fix in NH. > TransactionScope is the de facto standard for transactions everywhere in > the .NET world. > > RP > > On Saturday, April 26, 2014 9:46:44 PM UTC+1, ydi...@gmail.com wrote: >> >> I have been following those issues around TransactionScope for years now, >> and I was again recently facing problems regarding TS and NH. So I decided >> to finally have a look at the NH code involved in tx management, and wrote >> some test cases to confirm some assumptions on the behavior of TS and the >> DTC. I then found your post and I'm glad you summarized the situation well. >> I agree with your 3 conclusions: unnecessary use of TxCompleted event >> handler, flushing in Prepare() is bad, and NH session still needs to enlist >> (mostly because of 2nd level cache, but not exclusively: entity locks, >> connections release (?), firing interceptors). >> >> Regarding the proposed solutions, here are some comments: >> >> - no TransactionCompleted event handler: fully agree, it is redundant >> with IEnlistmentNotification >> >> - no Flush from Prepare(): I would say no Flush() on a connection that >> has already been enlisted in the same transaction. So that leaves us with 3 >> options: Flush() from Dispose when in TxScope (as you also propose), >> explicit Flush() required by app code (somehow breaks the current >> programming model), or Flush from Prepare on another, new connection. The >> latter is possible if I refer to your comment on the behavior of Prepare(): >> we could enlist a new connection and flush on this one. Now the question >> is: what is really a resource manager? A DB connection, or the DB server >> engine behind it? Because in the second case, that option is not doable. A >> drawback of the third option is also that there is a great chance that the >> tx will be promoted to distributed if it wasn't already, plus we have to >> open a new connection, and both are very costly. At first sight, I would >> vote for Flush() from Dispose, with new FlushMode option, but it does not >> solve the issue for code scenario 2. That's why we should maybe consider >> flushing on another connection if possible? >> >> - I guess we will not be able to avoid all code in Commit/Rollback. I >> agree with your idea of having a lock (thinking about a >> ManualResetEventSlim indicating that no 2PC sequence is ongoing, that is >> actually how I partly solved this issue on one of my recent projects). >> However, there is a supplementary challenge here. Indeed, in case of tx >> timeout, Rollback will be called asynchronously (on a thread pool's thread, >> since a System.Threading.Timer is firing), without Prepare being called >> first... Again, we have possible concurrent access to the session, since >> user code may be running on another thread at the same time. That means >> that we might need an additional lock on top of the one that would signal >> an ongoing Prepare/Commit or Rollback sequence, because there is no Prepare >> in case of timeouts. >> >> - of course AfterTransactionCompletion can (and will in case of >> distributed tx) be called asynchronously >> >> - finally, if possible, I would like to make use of the occasion to make >> things clear about the usage of TxScope and explicit NH transactions. There >> has always been much confusion about their use together, and no definitive >> answer: compatible? required to use Begintx with Txscope? why? That >> confusion has moreover been exacerbated by the buggy support of TxScope in >> NH. IMHO, an application should be designed to manage transactions using a >> single, consistent API, and never mix two different tx APIs. So basically, >> I would require the NH user to make a choice: either use TxScope (with the >> appropriate TxFactory!), and then making calls to >> ISession.BeginTransaction() forbidden (throw), or use BeginTx(), with a >> txfactory that does not support TxScope, and will not enlist the session in >> any ambient transaction (leave it up to the programmer if the underlying >> ADO connection automatically enlists if there is an ambient tx, or >> throw/warning if possible). >> >> I would be happy to help implementing a definitive solution, because I >> have the impression that this issue has not received the attention it >> deserves until now. This caused some loss of confidence in the NH user >> community, as the TxScope API is a de facto standard in the .NET world (and >> recommended over explicit tx management), and supporting it correctly is a >> must IMHO. >> >> Regards, >> >> Yves >> >> On Sunday, November 24, 2013 8:04:52 PM UTC+1, Oskar Berggren wrote: >>> >>> I would guess that new behaviour should be implemented as a new >>> transaction factory, so that the old sometimes-unstable behaviour can be >>> used on a case-by-case basis. >>> >>> It should be feasible to have the session auto-flush on dispose if >>> inside a transaction scope. This helps with the >>> session-inside-transactionscope >>> pattern, but not the other way around. (FlushMode.Commit would mean >>> flush-on-session-dispose-if-inside-transaction). Possibly there should >>> be new FlushModes to control this. >>> >>> It might also be feasible to have a dirty check on transaction scope >>> disposal (i.e in the prepare phase) but instead of flushing to the >>> database, it would just throw an error if any dirty state is found. This >>> would be a double dirty check and could have a significant negative impact >>> on performance. >>> >>> As for unavoidable... When it comes to the abilities of >>> System.Transaction I have spent some time researching it. I could be wrong, >>> or maybe there is a different strategy I haven't thought about. I would be >>> very happy if anyone could come up with a better approach and prove me >>> wrong. >>> >>> >>> >>> >>> 2013/11/24 Michael Teper <mte...@gmail.com> >>> >>>> I would really like to have TransactionScope support fixed, but >>>> requiring a manual Flush is a very high price to pay. We would need to >>>> track down, fix, and test literally hundreds of places. Is this truly >>>> unavoidable? >>>> >>>> Thanks! >>>> -Michael >>>> >>>> >>>> On Saturday, November 16, 2013 8:45:46 PM UTC-8, Oskar Berggren wrote: >>>> >>>>> We have several issues relating to TransactionScope documented in >>>>> Jira, and there has been, over the years, multiple, sometimes heated, >>>>> discussions on this in the mailing lists. The following is an attempt to >>>>> summarize the abilities and possibilities. >>>>> >>>>> https://nhibernate.jira.com/issues/?jql=labels%20%3D%20Trans >>>>> actionScope%20AND%20status%20in%20%28Open%2C%20%22In%20Progress%22%2C% >>>>> 20Reopened%2C%20Unconfirmed%29<https://www.google.com/url?q=https%3A%2F%2Fnhibernate.jira.com%2Fissues%2F%3Fjql%3Dlabels%2520%253D%2520TransactionScope%2520AND%2520status%2520in%2520%2528Open%252C%2520%2522In%2520Progress%2522%252C%2520Reopened%252C%2520Unconfirmed%2529&sa=D&sntz=1&usg=AFQjCNE4qup8-JuSqUVyXundHxl2db2yZA> >>>>> >>>>> >>>>> SUMMARY OF WHAT System.Transactions PROVIDES: >>>>> Based on official documentation, blogs, and experiments. >>>>> >>>>> IEnlistmentNotification.Prepare() >>>>> May be called on a thread different from the one calling >>>>> Transaction.Dispose(). >>>>> Transaction.Dispose() will not return until all enlisted Prepare() >>>>> have completed. >>>>> Can be used to enlist more resource managers, but it cannot touch >>>>> already >>>>> enlisted resource managers, since those may have already been >>>>> prepared and >>>>> could now be locked for further changes in waiting for the second >>>>> phase. >>>>> >>>>> IEnlistmentNotification.Commit() >>>>> May be called on a thread different from the one calling >>>>> Transaction.Dispose(). >>>>> Might not run until after Transaction.Dispose() has returned. >>>>> >>>>> IEnlistmentNotification.Rollback() >>>>> May be called on a thread different from the one calling >>>>> Transaction.Dispose(). >>>>> I suspect it might not run until after Transaction.Dispose() has >>>>> returned. >>>>> Will not run at all if our own Prepare() initiated the rollback. >>>>> >>>>> Transaction.TransactionCompleted >>>>> May be called on a thread different from the one calling >>>>> Transaction.Dispose(). >>>>> Might not run until after Transaction.Dispose() has returned. >>>>> Called for both successful and unsuccessful transactions. >>>>> Is documented as an alternative to a volatile enlistment: >>>>> "You can register for this event instead of using a volatile >>>>> enlistment to get outcome information for transactions." >>>>> /MSDN >>>>> The same MSDN docs also notes that using this event has a negative >>>>> performance impact. >>>>> >>>>> >>>>> >>>>> CURRENT CODE STATE >>>>> In NHibernate 3.3 (AdoNetWithDistributedTransactionFactory): >>>>> >>>>> AdoNetWithDistributedTransactionFactory adds a handler for the >>>>> TransactionCompleted event and also implements IEnlistmentNotification. >>>>> >>>>> A session opened inside a TransactionScope will automatically enlist. >>>>> It will not flush on dispose, but when the TransactionScope is disposed, a >>>>> flush will be triggered from the Prepare() phase in >>>>> AdoNetWithDistributedTransactionFactory. Surprisingly, this sometimes >>>>> works. >>>>> >>>>> Depending on in what order things are done, whether or not the >>>>> transaction was lightweight or distributed from the start, or if promotion >>>>> have occurred etc., the code in Prepare() will sometimes hang for 20 >>>>> seconds when trying to flush (waiting on a call to ADO.Net) and then abort >>>>> (NH-2238). I believe this is because the ADO.Net connection is also a >>>>> resource manager, enlisted with the transaction, and in some circumstances >>>>> the transaction manager might have called ADO.Net's Prepare() before >>>>> NHibernate's. Further attempts to use that connection then blocks while >>>>> the >>>>> connection is waiting for its Commit() or Rollback() notification. >>>>> Generally speaking, I would say that while it is allowed to enlist more >>>>> resource managers during the prepare phase, it is _not_ allowed to touch >>>>> other already enlisted resource managers, because there is no ordering >>>>> guarantee. >>>>> >>>>> The current code is written with the assumption that >>>>> TransactionCompleted and Commit()/Rollback() is triggered before >>>>> Transaction.Dispose() returns. They call ISessionImplementor. >>>>> AfterTransactionCompletion() and CloseSessionFromDistributedTransaction(). >>>>> This causes race conditions in the session if the user keeps working with >>>>> it (NH-2176). It is also a problem for the ADO.Net connection, since that >>>>> is also not thread safe and so while >>>>> CloseSessionFromDistributedTransaction() >>>>> is trying to close the connection, the transaction manager may have used a >>>>> different thread to issue the commit phase on the connection. >>>>> >>>>> Since they may in fact be called later, this causes multi-thread >>>>> issues with both the ISession and the ADO.Net connection. This is because >>>>> this separate thread will call >>>>> ISessionImplementor.AfterTransactionCompletion() >>>>> and CloseSessionFromDistributedTransaction() which will touch the >>>>> session (that the application itself may already use for something else >>>>> (NH-2176)) and also try to close the connection, which might cause >>>>> connection pool corruption (NH-3023). >>>>> >>>>> >>>>> CONCLUSIONS >>>>> * It is redundant to both implement IEnlistmentNotification and listen >>>>> for TransactionCompleted. >>>>> >>>>> * Flushing changes to the ADO.Net connection in >>>>> IEnlistmentNotification.Prepare(), as we currently do, IS SIMPLY NOT >>>>> SUPPORTED, because we cannot touch the database connection since that is >>>>> itself a resource manager already enlisted. >>>>> >>>>> * We might need to still enlist with the transaction to release second >>>>> level cache locks. >>>>> >>>>> >>>>> CODE SCENARIO 1: SESSIONS INSIDE TRANSACTION >>>>> >>>>> using (transactionscope) >>>>> { >>>>> // WITH EXPLICIT NH TRANSACTION. >>>>> using (new session) >>>>> using (session.BeginTransaction) >>>>> { >>>>> DoEntityChanges(); >>>>> >>>>> // This will flush the above changes, but leave the underlying >>>>> db >>>>> // connection and transaction open. >>>>> tx.Commit(); >>>>> } >>>>> >>>>> // WITHOUT EXPLICIT NH TRANSACTION. >>>>> using (new session) >>>>> { >>>>> DoEntityChanges(); >>>>> >>>>> // This will flush the above changes, but leave the underlying >>>>> db >>>>> // connection and transaction open. >>>>> session.Flush(); >>>>> } >>>>> >>>>> // Because of the current flush during the prepare phase, this >>>>> change might >>>>> // be committed despite being performed outside a transaction. It >>>>> might also >>>>> // cause a deadlock. Without this change, the flush-in-prepare >>>>> will have nothing >>>>> // to do, and so will not cause a problem. >>>>> entity.AdditionalChange() >>>>> >>>>> transactionscope.Complete(); >>>>> } >>>>> >>>>> >>>>> (Improved) Support for sessions inside TransactionScope is subject to >>>>> the following limitations: >>>>> * Changes must be flushed explicitly before reaching >>>>> TransactionScope.Dispose(). >>>>> >>>>> * We need to fix the code so that we also never touch the ADO.Net >>>>> connection after >>>>> TransactionScope.Dispose() has been reached. To achieve this, the >>>>> session should >>>>> release the connection no later than its own Dispose(). NH-2928. >>>>> This would also >>>>> reduce the need for transaction promotion when two non-overlapping >>>>> sessions are >>>>> used within the same TransactionScope. >>>>> >>>>> * If desired, it should be possible to have the session auto-flush >>>>> from its Dispose() >>>>> when used inside a TransactionScope (NH-2181). Drawback is that the >>>>> session >>>>> would behave very differently compared to not using TransactionScope >>>>> (increased >>>>> complexity). >>>>> >>>>> * Entity changes performed outside an active transaction would not be >>>>> committed, >>>>> just as when not using TransactionScope. Unless the entity is >>>>> reattached to a >>>>> new session. >>>>> >>>>> >>>>> >>>>> CODE SCENARIO 2: TRANSACTIONS INSIDE SESSION >>>>> >>>>> using (new session) >>>>> { >>>>> using (new transactionscope) >>>>> { >>>>> DoEntityChanges(); >>>>> >>>>> transactionscope.Complete(); >>>>> } >>>>> >>>>> // Unknown amount of time before the transaction commit phase is >>>>> done with the session. >>>>> >>>>> using (new transactionscope) >>>>> { >>>>> DoEntityChanges(); >>>>> >>>>> transactionscope.Complete(); >>>>> } >>>>> } >>>>> >>>>> >>>>> (Improved) Support for TransactionScope inside sessions is subject to >>>>> the following limitations: >>>>> * We simply cannot rely on the TransactionScope to tell us when to >>>>> flush, so again this can >>>>> only work with explicit flush (either Flush() or Commit() on NH's >>>>> transaction). >>>>> >>>>> * To support consecutive TransactionScopes (NH-2176), or really any >>>>> use of the session after we have reached >>>>> Dispose() on the first TransactionScope, the application must be >>>>> able to reliable detect when the >>>>> out-of-band Commit()/Rollback in IEnlistmentNotification have >>>>> completed. >>>>> >>>>> >>>>> >>>>> COMPARISON WITH OTHER SOFTWARE >>>>> It has been claimed that other software "works with >>>>> System.Transactions automatically and doesn't require the use of something >>>>> like NH's ITransaction". These statements seem to be both correct and >>>>> wrong. Looking at eg. Linq2SQL and Entity Framework, it's true that they >>>>> have no counterpart to NH's ITransaction. However, they do require >>>>> explicit >>>>> calls to e.g. SaveChanges() before TransactionScope.Complete() and >>>>> Dispose() is called. So they too have no automatic >>>>> flush-on-TransactionScope.Dispose() behaviour. >>>>> >>>>> Within the context of a TransactionScope, one can perceive the >>>>> ITransaction.Commit() as one possibly way to trigger the explicit flush of >>>>> changes. The other one is of course session.Flush(). So it's really not >>>>> that different. >>>>> >>>>> >>>>> >>>>> IDEAS FOR PROPOSED SOLUTION >>>>> * Use only IEnlistmentNotification, not TransactionCompleted. >>>>> >>>>> * Remove the flushing from IEnlistmentNotification.Prepare(). The >>>>> application must flush earlier or loose the changes. >>>>> >>>>> * If we cannot move all code from the Commit/Rollback/TransactionCompleted >>>>> stage into Prepare(), >>>>> we must instead grab some sort of lock on the session at the start >>>>> of Prepare(). The lock would be >>>>> released at the end of the Commit or Rollback phase. In the >>>>> meantime, all other attempts to use >>>>> the session should block waiting for the lock to be released. (I >>>>> suspect this is what happens when use of >>>>> the ADO.Net connection blocks when flushing from Prepare().) >>>>> >>>>> * Document that IInterceptor.AfterTransactionCompletion() and similar >>>>> may be called >>>>> asynchronously on a different thread. >>>>> >>>>> * Optionally, we can also add automatic flush in the session's >>>>> Dispose() method if an ambient >>>>> transaction is detected. >>>>> >>>>> >>>>> I'm still a bit unclear on the interaction with the second level cache. >>>>> >>>>> >>>>> /Oskar >>>>> >>>>> -- >>>> >>>> --- >>>> You received this message because you are subscribed to the Google >>>> Groups "nhibernate-development" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to nhibernate-development+unsubscr...@googlegroups.com. >>>> For more options, visit https://groups.google.com/groups/opt_out. >>>> >>> >>> -- > > --- > You received this message because you are subscribed to the Google Groups > "nhibernate-development" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to nhibernate-development+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > -- --- You received this message because you are subscribed to the Google Groups "nhibernate-development" group. To unsubscribe from this group and stop receiving emails from it, send an email to nhibernate-development+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
