I think it's reasonable to require a manual flush before the transaction scope commit. IMHO it would be okay as a breaking change to include with NH4.
On Sat, Nov 16, 2013 at 9:45 PM, Oskar Berggren <oskar.bergg...@gmail.com>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%20TransactionScope%20AND%20status%20in%20%28Open%2C%20%22In%20Progress%22%2C%20Reopened%2C%20Unconfirmed%29 > > > 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/groups/opt_out.
