Summary of System.Transactions and suggestions for NHibernate improvements

628 views
Skip to first unread message

Oskar Berggren

unread,
Nov 16, 2013, 11:45:46 PM11/16/13
to nhibernate-...@googlegroups.com
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.
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

Patrick Earl

unread,
Nov 24, 2013, 12:54:17 PM11/24/13
to nhibernate-development
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.


--
 
---
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-develo...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Michael Teper

unread,
Nov 24, 2013, 1:42:54 PM11/24/13
to nhibernate-...@googlegroups.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

Oskar Berggren

unread,
Nov 24, 2013, 2:04:52 PM11/24/13
to nhibernate-...@googlegroups.com
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>

ydi...@gmail.com

unread,
Apr 26, 2014, 4:46:44 PM4/26/14
to nhibernate-...@googlegroups.com
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


2013/11/24 Michael Teper <mte...@gmail.com>
To unsubscribe from this group and stop receiving emails from it, send an email to nhibernate-development+unsub...@googlegroups.com.

Ricardo Peres

unread,
Apr 27, 2014, 5:59:36 AM4/27/14
to nhibernate-...@googlegroups.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

Johannes Gustafsson

unread,
Apr 28, 2014, 8:24:24 AM4/28/14
to nhibernate-...@googlegroups.com
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


To unsubscribe from this group and stop receiving emails from it, send an email to nhibernate-develo...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

ydi...@gmail.com

unread,
May 14, 2014, 4:14:34 PM5/14/14
to nhibernate-...@googlegroups.com
Hello NH team!

I started my journey in the world of System.Transactions and in the inner workings of NH. After careful review of all issues, I identified 3 areas that need changes and / or improvements: connection management, thread safety of the session and Flush() timing.

I decided to start working on connection management first, and what I'm going to implement is a close of the connection when closing (or disposing of) the session, even when an ambient tx is active. As stated in MSDN, this should not be an issue (see http://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlconnection.close.aspx). This is the heart of the discussion in NH-2928, but deferring connection close causes other issues as well (concurrency caused by async close). 

I started writing a new transaction factory, but unfortunately, it seems difficult to support the current, buggy AdoNetWithDistributedTransactionFactory together with a new one that would fix the issues. That's why I had to drop the current tx factory. Hope this is not a too big issue. Any comments on this?

Regards,

Yves


2013/11/24 Michael Teper <mte...@gmail.com>
To unsubscribe from this group and stop receiving emails from it, send an email to nhibernate-development+unsubscri...@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+unsub...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Johannes Gustafsson

unread,
May 15, 2014, 7:59:34 AM5/15/14
to nhibernate-...@googlegroups.com
Do you happen to have the code public?

I agree that the connection should be disposed on session dispose. If the session opened the connection in the first place, it makes no sense to not dispose it IMHO. 

It might actually reduce the probability of promoting a tx to a distributed tx. In Sql server 2008 and above, if you have multiple identical sql connections, it can detect that they are the same and not promote the tx. However, for that to work they must only be used sequentially, which means you have to close (dispose) the connection before starting the second one.


To unsubscribe from this group and stop receiving emails from it, send an email to nhibernate-develo...@googlegroups.com.

Oskar Berggren

unread,
May 15, 2014, 8:10:23 AM5/15/14
to nhibernate-...@googlegroups.com
Thanks Yves for looking into this! Some comments below.


2014-04-26 22:46 GMT+02:00 <ydi...@gmail.com>:
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?

The connection is the resource manager for this purpose (i.e. you are free to use other connections). Flush on separate connection: I think this is a no-go solution due to the promotion issue you mention. Possibly as an option that can be easily activated for legacy code, to make it run stable but less performant. Also, I wonder how database engines react if there there has been locks taken on the first connection - will a second connection really be able to run within the scope of those locks just because they share the same distributed transaction? I doubt it - which means the second connection will just cause deadlock.
 
Regarding when to flush: The recommendation from NH for many years have been to always use the NH transaction even when running inside transaction scope (indeed this is really the only truly supported scenario). Such code works both with and without a surrounding ambient transaction. It also means that existing code conforming to this rule doesn't rely on the flawed flush-on-prepare behavior. Removing that feature should in general have no effect on such code. The understanding then would be that conforming code must EITHER use NH transaction inside session, and call its Commit() method, OR call Flush() on the session. The latter case would resemble EF's SaveChanges().

The point of the previous paragraph would be that adding a FlushMode.Dispose would be a separate feature - that can be evaluated and possibly implemented some time later, but it would not be required for an initial fix of ambient transaction support.


- 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).



NH-transaction inside session inside ambient transaction is a supported scenario, and indeed the recommended way to work with System.Transactions. If an ambient transaction is present, the NH-transaction doesn't do much - its Commit() method will typically trigger Flush(), but it will not actually manage a DB transaction. Benefit: Code (library) using nh-transaction inside session should work and always use transaction WITH OR WITHOUT an ambient transaction (application code in a higher layer can make this choice). Drawback: It makes people confused and some people think this means that NH doesn't support System.Transactions.


/Oskar


Oskar Berggren

unread,
May 15, 2014, 8:18:36 AM5/15/14
to nhibernate-...@googlegroups.com
2014-05-14 22:14 GMT+02:00 <ydi...@gmail.com>:
Hello NH team!

I started my journey in the world of System.Transactions and in the inner workings of NH. After careful review of all issues, I identified 3 areas that need changes and / or improvements: connection management, thread safety of the session and Flush() timing.

I decided to start working on connection management first, and what I'm going to implement is a close of the connection when closing (or disposing of) the session, even when an ambient tx is active. As stated in MSDN, this should not be an issue (see http://msdn.microsoft.com/en-us/library/system.data.sqlclient.sqlconnection.close.aspx). This is the heart of the discussion in NH-2928, but deferring connection close causes other issues as well (concurrency caused by async close). 


I agree this looks like a good idea.

 

I started writing a new transaction factory, but unfortunately, it seems difficult to support the current, buggy AdoNetWithDistributedTransactionFactory together with a new one that would fix the issues. That's why I had to drop the current tx factory. Hope this is not a too big issue. Any comments on this?



I think it would be very good if the existing AdoNetWithDistributedTransactionFactory could remain with unchanged functionality it possible. Even if we can come up with a better solution that in theory should be fully backwards compatible, it would be comforting to know that one can switch back to AdoNetWithDistributedTransactionFactory if the new one causes problems in some scenario.


One more thing:

NH can be used in various ways, e.g. session inside ambient transaction or the other way around, short- or long-lived session, etc. Perhaps we don't need to support all these scenarios together with System.Transactions?


/Oskar

ydi...@gmail.com

unread,
May 15, 2014, 4:49:25 PM5/15/14
to nhibernate-...@googlegroups.com
Not yet, or at least nothing interesting yet. I'll push to my public repo on github, TxScope branch when I have something concrete. I've been mostly busy writing tests to assert and understand the behavior of TxScope, System.Transactions, and to get a grasp of the current NH design in the affected areas.
Indeed SqlClient does not promote the tx when many Open/Close connection (using same cn string, of course) are executed in sequence in same tx scope. I think (not sure though) Oracle has the same feature. Maybe this is something to consider in the Flush in prepare on second connection scenario, but that requires additional validation and testing, given the particular execution context of phase 0 Prepare.

Regards,
Yves
For more options, visit https://groups.google.com/d/optout.

Frédéric Delaporte

unread,
Jul 6, 2017, 8:44:43 AM7/6/17
to nhibernate-development
Hi,
Alexander has pointed me here, which summarize most of the findings I have (re)done while working on that subject. (I had search the dev list before starting tackling the subject but unfortunately not found that thread before, so long for me.)

Here are some complementary notes.

Concurrency

The first Oskar message is a great summary, but is a bit unclear about concurrency issues with transaction completion event and second phase callback (commit/rollback/in-doubt).
When the transaction is distributed, they can all execute concurrently to code following the scope disposal. Scope dispose does block only till all prepare phases have voted and MSDTC has recorded the outcome. After that, all remaining work (en of scope disposal, transaction complete event, second phase callbacks) run concurrently.

You can read more about that in following discussion with a Microsoft employee working on MSDTC.

Usage pattern

> The recommendation from NH for many years have been to always use the NH transaction even when running inside transaction scope (indeed this is really the only truly supported scenario). 

This is terribly unfortunate.

Quote from an old Msdn article, emphasis mine:

> As a rule, use System.Transactions in the general case, and use resource manager internal transactions only in specific cases where you are certain the transaction will not span multiple resources, and will not be composed into a larger transaction. Never mix the two.

Other point against mixing both, if a local transaction is started first, it is no more possible to enlist the connection inside a scope, as stated in Microsoft doc:

> EnlistTransaction throws an exception if the connection has already begun a transaction using the connection's BeginTransaction method.

This means that starting a local transaction then a scope cause at best the scope to be ignored (current NHibernate state) or at worst will cause the session to fail (if it starts trying explicitly enlisting the connection into ambient scope).

Transaction completion event

About replacing transaction completion for second phase enlistment, I have tried it and that was causing the issues to be worst. I guess the trouble is closing the connection from second phase callbacks tends to more frequently causes issues than from transaction completion event.

Multiple connections to same database inside a transaction

The semantic for locks is normally to be bound to the transaction, not to the connection. Having concurrent connections in the same transaction should not cause them to lock themselves.

About the SQL Server behavior of not promoting to distributed when sequentially using many connection, I can tell Npgsql supports this too. (Its pool keeps track of connections enlisted in transaction, returned to pool, and yield them back if a connection is requested with the same ambient transaction.)


What I have done so far

There is a PR which fixes all distributed transaction failing test cases, adds a lot more tests and supports them. It still needs some cleanup and discussion, but that looks quite more robust than current NHibernate state.

It does not add a new transaction factory, but it fixes the existing one for most cases with (I think) sensible possible breaking changes. By default it does not completely fix the troubles bound to connection release from transaction completion, but it provides an option for that. That option causes "flush on commit" to be disabled for transaction scope, and so require the changed to be explicitly flushed.

Note that even without "flush on commit" disabled, no tests are failing. But some unwanted promotion to distributed may occur, and in some corner cases bad performances can be observed. (This occurs when nesting session life-cycle inside scope, keeping the flush from commit enabled, encountering a deadlock, trying again with new session, encountering a deadlock again, ... In this pattern, the connection releasing is more and more slow. But the connection pool corruption does not more occurs since .Net Framework 4.0. It seems the pool has become more robust.)

Notable changes:
  • When flushing from commit (so in prepare phase), it will use a new connection dedicated to that, unless the database does not support it (like SQLite). Can be "hacked back" to previous behavior by overriding the dialect for it to tell it does not support concurrent connection inside the same transaction.
    May trigger undesired promotion to distributed.
    The recommended way to avoid that is to disable the "flush from commit" functionality for system transaction, see below.
  • It will lock session usage from the point where a system transaction cease to be active to the end of completion event tracked by the NHibernate resource. This works even in cases where the prepare phase is not called, and prevents session usage after the scope disposal, while the transaction completion is not yet ended. The code using the session after the scope is blocked from executing till completion end.
  • It will forbid any session connection usage from ISession/IInterceptor AfterTransactionCompletion event when it is raised from system transaction completion. Attempt to use the connection will throw. (Unless hacking for getting a reference on it soon enough and use it directly instead of going through connection manager.)
  • The connection will no more be actually released from ConnectionManager AfterTransaction event when called from system transaction completion. It will instead be flagged for releasing, and will be released at next connection manager GetConnection usage (or when connection manager is closed).
    With this changes, in most cases, they will be no more connection releases from system transaction completion threads.
    If some user code was retaining the session without acting on it for a long period after the last transaction, this could be a detrimental change, causing the connection to be released quite later.
  • The session will now actively enlist its connection inside the ambient transaction scope if any. This can be disabled by switching to the transaction factory unaware of system transaction, or by using an option at session opening which causes system transaction to be completely ignored by NHibernate. For consistency, the connection string should then specify Enlist=false.
    So this can be a possible breaking change if some user code was opening a NHibernate transaction, then a scope, then was using the session. Previously the scope was ignored and the connection not participating in it, essentially meaning the user transaction was silently broken. Now it will throw.
  • An "UseConnectionFromSystemTransactionEvents"  option is added, true by default.
    When set to false, this forbid using the connection from ISession BeforeTransactionCompletion event when raised from system transaction, which disable flush from commit. (No exception, just no flush.) If nesting session life-cycle inside transaction scope, this option must be disabled for avoiding having the session actually closed from transaction completion (and retaining its connection till there).
    When set to false, session disposed inside a scope will be actually disposed of. But their transaction completion events will still be called, notably allowing second level cache to still works (I have put tests for ascertaining this). Of course this is a bit debatable to still call some methods on a disposed object. Thus it could be better to port the transaction event queue from current Hibernate, which seems to allow decoupling a bit more those transaction events from the (potentially disposed) session.

Reply all
Reply to author
Forward
0 new messages