Fix transaction scopes handling

Description

There are a bunch of issues which relates to transaction handling with NHibernate. Most of them are more specifically bounded to ambient transactions (TransactionScope).

Those issues may be as bad as corrupting the connection pool, thus rendering the application completely unreliable.

We need to devise how to change current transaction handling in order to fix them.

Environment

None

Activity

Show:

Frédéric Delaporte September 7, 2017 at 9:45 AM

The currently merged in master rework of system transactions handling is overall a mix of proposed solutions here in previous comments.

Most features of the previous implementation have been preserved and stay enabled by default. Some of them are still troublesome and can be disabled if your usage pattern does not support them, see below.

Possible breaking changes (applying only to system transactions - aka transaction scopes -, not to native transactions) :

  • For databases supporting it, the connection used by the session will be switched for a new one during before transaction completion event, including during the flush on commit. This will usually promote the transaction to distributed. For avoiding this, either avoid performing any database related work during these events (see below for an option helping to do that), or you may use your own custom dialect overriding SupportsConcurrentWritingConnectionsInSameTransaction to false. The later may expose you to issues such as NH-3968.

  • After transaction events no more allow using the connection. Any action causing the session to try using the connection during them will throw.

  • The connection will now be actively enlisted in ambient system transaction by default. Previously, NHibernate was relying solely on the auto-enlistment feature (connection string parameter Enlist (for most db) not specified or true)), meaning a connection opened prior to the scope was not participating in it. For disabling this, you may use https://nhibernate.jira.com/browse/NH-4018#icft=NH-4018 (recommended), or switch the transaction factory for one not supporting system transaction at all like AdoNetTransactionFactory.

  • AdoNetWithDistributedTransactionFactory has been renamed AdoNetWithSystemTransactionFactory.

  • Nesting transaction scopes remains unsupported. The session will enlist in the current one, and will stay in it till transaction completion. In some cases, like opening the session in a sub scope, finishing the scope and trying to continue using the session in the parent scope, exceptions will now be explicitly raised.

  • We now strongly advise against using the feature "flush on transaction commit" when the session is disposed of before the scope. Please explicitly flush your session before disposing them. We may drop this feature in some later release. This feature still work, and should work better, but may still have issues such as NH-3023. For ensuring you do not use this feature inadvertently, you can disable it (see below).

  • ConnectionReleaseMode.AfterTransaction has no more an immediate effect, in order to avoid releasing the connection from a potentially unexpected thread. (This was occurring in distributed cases.) The connection will be actually released on the next session usage, or closing. If you keep your session idle a long time after a system transaction, you may manually perform this releasing by calling Disconnect then Reconnect on it.

  • A synchronisation mechanism has been added for avoiding having system transaction completion events running concurrently to session usages coded after the scope disposal. (Yes, a scope disposal may be left before those events have finished executing, especially in distributed cases, see here.) This synchronisation mechanism has a default timeout of five seconds, after which the session will throw. If your application appears to require more time for processing two phases commits, you can change this with the transaction.system_completion_lock_timeout setting (in milliseconds ; Environment.SystemTransactionCompletionLockTimeout).

  • NHibernate team stance about using system transactions together with native transaction (ISession.BeginTransaction) has not been well defined. Since doing so causes mixing system transactions with ADO transaction, which Microsoft advise against, this pattern is now discouraged. Please use either system transaction or native transaction, but not both at the same time.

As said previously, you now have an option for controlling connection enlistment, see NH-4018.

Flushing on system transaction commit was the main cause of bugs in system transaction handling, and can still triggers issues as stated above. You can disable this feature with transaction.use_connection_on_system_events setting (Environment.UseConnectionOnSystemTransactionPrepare). Set it to false.
You will then need to always explicitly flush your sessions before committing a system transaction (or before disposing them when inside a scope) for changes to be persisted.
Before transaction completion events will additionally no more be allowed to use the connection.

Some databases/configurations have not been fixed/tested:

  • Firebird with its official .Net provider: its current way of handling connection enlistment does not allow NHibernate test suit to be used with it. See DNET-764, DNET-765, DNET-766. Firebird data provider disables completely connection enlistment by default, and enabling it then requires to always use them inside scopes.

  • Oracle unmanaged driver: I was sadly unable to setup a working configuration with it. It was not even allowing enlisting in a distributed transaction. Oracle 12c managed driver is tested and working.

Beware that some database data provider have non full SQL-Server like behavior with connection enlistment, like completely disabling it with Enlist=false instead of just disabling auto-enlistment (Firebird official data provider, SqlServer Compact Edition, maybe others), having it disabled by default (older Npgsql, Firebird, some others, ...), having a different naming for it (MySql, ...).

Frédéric Delaporte May 15, 2017 at 12:26 PM
Edited

Putting it all together

Due to the different used patterns, I think we need to provide different transaction factories for supporting them all correctly. No single factory will support them all alone.

Moreover, some changes including breaking ones are required for a complete solution.

Changes

  1. Flushes will be mandatory before session disposal if we want the session changes to be taken into account by an ongoing transaction. We may try to support a legacy mode here but I would rather get rid of the code required for it.

  2. Flushes from transaction scopes will happen only for non disposed sessions and will require the AfterStatement connection release mode to be safe. Using a transaction factory supporting flushes from transaction scopes with another connection release mode should then either be considered as an error or as unsupported (and warned in logs). Using a transaction factory flushing from transaction scopes should anyway be discouraged because it will have chances to promote the transaction to distributed.

  3. The AfterStatement release mode will be supported. But if there is an explicit transaction (ISession.BeginTransaction(), it will be suspended till the explicit transaction end (since explicit transaction cannot support it).

  4. The AfterTransaction release mode must cease supporting releasing connection at transaction scope end. It would only support explicit transaction end. The AfterTransaction should then no more release connection in auto-commit mode: this behavior would be covered by AfterStatement, and that would allow AfterTransaction to not care at all about transaction scope.

  5. The transaction factories will have to explicitly enlist the session already acquired connection (if any) with the current ambient transaction, when not already participating in a local (explicit) transaction. Otherwise the session spanning transaction scope pattern will no more work, unless the user explicitly releases the connection at each scope end. For supporting a mix of transaction scoped usages with non transaction scoped usages, it may be even required to enlist with `null` when no transaction scope are there. Since some connections (like SqlCeConnection and SQLiteConnection) do not support this, we may, instead of enlisting, release the connection if we detect it was acquired with another transaction scope (or lack of).

  6. Nested transaction scopes used with a previously acquired connection will no more be silently ignored but will cause a failure. (We can preserve the current behavior easily, but that does not look as a good thing to me.)

  7. A new method should be added on ISession for allowing users to explicitly trigger a connection release. (This is not a Disconnect, which requires a later reconnect call.) This would allow the user to optimize its connection pool usage in the Session spanning transaction scope case while using AfterTransaction release mode. This may allow a safe usage of AfterTransaction release mode with a transaction factory flushing changes from scope, provided the user explicitly releases the connection before completing the scope. In case the connection is in use (explicit transaction ongoing, opened ressources, ...), an HibernateException would be raised.

  8. Transaction completion actions currently handled from the session should be re-aligned on Hibernate actions queue. Those which can be executed before transaction completion, from first phase without knowing the success or failure outcome, should be done there for avoiding having to render them thread-safe. Others should be registered for execution at second phase of the 2PC or from TransactionCompleted event when a transaction scope is ongoing. Those actions should not include any operation using a connection, and will need to be thread safe.

  9. An AdoNetWithFlushedAmbientTransactionFactory will be added. It will be documented as requiring AfterStatement release mode or explicit release of session connections before transaction completion. And its documentation will also warn for the high probability of transaction promotion to distributed bound to its usage. When it performs a flush, it will itself then trigger an explicit release of the supposedly newly acquired connection (hopping the user has correctly done it too ; or we may put an internal for knowing if the session already hold a connection, warn in such case, try to use it without releasing it).

  10. An AdoNetWithAmbientTransactionFactory will be added. It will be documented as neither performing automatic flushes from transaction scope completion, nor triggering any connection release from scope completion. User will be encouraged to explicitly release connection on the session after completing a scope, when they are using the session spanning transaction scope pattern, unless they have chosen the AfterStatement release mode. This factory purpose will be to handle transaction completion actions required for features like the second level cache, when using transaction scope.

  11. The current AdoNetWithDistributedTransactionFactory would be obsoleted or completely removed. (I prefer the later, for code simplicity sake.)

  12. The auto connection release mode configuration value would choose between AfterStatement and AfterTransaction according to the used transaction factory.

  13. For minimizing the behavioral change, NHibernate should probably use as default AdoNetWithFlushedAmbientTransactionFactory. For performances and robustness sake, AdoNetWithAmbientTransactionFactory would be a better default. I am more for the later, but not strongly.

  14. Those transaction factory are factories only for one of their methods indeed, for all the others they are transaction managers or coordinators, maybe should we rename them.

Prerequisites

To go there, I see two prerequisites doable independently:

  • Reworking the transaction completion actions queue, probably by porting current Hibernate code.

  • Reworking the connection release mode by re-aligning AfterTransaction on Hibernate behavior, and supporting AfterStatement. Porting the on close mode with acquisition on opening does not seem adequate to me: no advantages for our current situation, and would call for an async session opening when NHibernate would support async, since with such an option session opening will open a connection. This change will require adding connection explicit enlistment in ambient transaction for avoiding breaking the session spanning transaction scope pattern when it is used mixed with some out of transaction session usages.

Then we will have to do the remaining changes probably in one PR.

Frédéric Delaporte May 14, 2017 at 6:54 PM
Edited

Conclusions?

The crux of NHibernate transaction troubles is the attempts at interacting with the database at transaction scope completion.

It looks it mostly works, at least for SQL Server users and when the transaction is not distributed. But I think we will not get it right, at least for the connection releasing part, because of this issue of transaction completion executing potentially in a concurrent thread.

Probable dead-ends

We may try to identify the "legit" thread for knowing if we can safely manipulate the connection or not, but this is probably doomed in case of async/await usages or under a web context (due to Asp.Net thread agility). And when NHibernate would detect the transaction completion is on a "bad thread", what could it do? Give-up flushing/releasing and leaving the app with unexpectedly dropped changes?

https://nhibernate.jira.com/browse/NH-2237#icft=NH-2237 moreover indicates that some other after transaction completion actions can not occur from the TransationCompleted event, like the downgrade of entity locks. Otherwise they sometime concurrently occurs with next usages of session.

Safer proposition

So I think the safer would be to enable a transaction handling mode which will be a mix of NHibernate "implicit without enlistment" mode and Hibernate "implicit with JTA". This could be done with a new ITransactionFactory, transaction scope aware, but which will no more try to flush the session or release the connection on transaction completion. It will only trigger other transaction completion actions as seems to do Hibernate, like updating the second level cache. Since https://nhibernate.jira.com/browse/NH-2176#icft=NH-2176 tests in various PR have shown both TransactionCompleted event and second phase of 2PC can both happen after the transaction scope disposal, those operations will need to be thread safe. (Or made at first phase end if this can be valid, as maybe downgrading entities locks.) The disposal of session will no more be delayed. Committing the scope will no more be able to trigger a flush, flushes will need to be done explicitly (excepted those auto-done before query, which can be supported).

For this, we will probably need to port current Hibernate transaction completion actions.

The current transaction factory could be left for legacy usage and facilitating the transition, since with the new one it would be required to check all session usages for flushing them before disposal when disposed within a transaction scope.

Transaction scope spanning session:

For this use case, this looks to me as a very sensible solution. Does it really makes sens to support delaying flush after a session disposal? Mandating a flush before session disposal for changes to be included in the ongoing transaction seems very understandable.

The connection release will be done at session disposal, and will happen even earlier than with current implementation, which is better. Transactions scopes support connection releasing, and this will avoid some promotion to distributed cases.

Session spanning transaction scope:

For this use case, this is a bit more arguable. This will mean the FlushMode.Commit is unsupported by NHibernate with transaction scope, and that an explicit flush is required before completing the scope.

(Remember though, that auto-flush before query may still be supported by this case, meaning it will not be equivalent to FlushMode.Manual either.)

Moreover, the connection release will no more occur after transaction, but will in most cases wait for session closing. This will not promote good connection pool usage.

For mitigating the connection release delay, we may then need to introduce a new method on ISession for allowing the user to manually trigger the connection releasing after transaction scope completion.

Riskier proposition

If we can ascertain that the first commit phase may not occur concurrently 1, contrary to the transaction completion event or second phase, we may keep the ability to auto-flush before commit for sessions which are not already disposed.

But for connection releasing, I do not see any safe way to trigger it from transaction scope end. The connection itself may be enlisted as participant even before the session prepare phase. So we can not release it from the 2PC of the session, since the connection may be executing its own 2PC. And we know for sure that releasing it from the TransactionCompleted event or second phase is prone to concurrency issues with usages of session following the transaction.

Moreover, since the connection may be already itself enlisted with the transaction, flushing from transaction 2PC causes https://nhibernate.jira.com/browse/NH-3968#icft=NH-3968 2. For avoiding this, we will have to ensure the flushing from 2PC uses a newly acquired connection. This can promote the transaction to distributed depending on the used database, and some will not support it.

(We may detect in prepare phase if the transaction was promoted to distributed, and if not trigger the flush, since it looks like there is no concurrency issue when not distributed, but again documentation lacks about it. And that will cause flushing behavior to be less predictable I think.)

Notes:

1: It is unfortunate I can not find any precise documentation about that. Msdn states in the remarks section of TransactionScope.Dispose():

This method is synchronous and blocks until the transaction has been committed or aborted.

So if we can actually trust that, the two phases commit should never be concurrent with following session usages. And that would be only the TransactionCompleted event which may be executed concurrently to following session usages.

But while testing various solutions in https://nhibernate.jira.com/browse/NH-2176#icft=NH-2176 PRs, it appears the second phase for the session enlistment could be executed after transaction scope disposal. Maybe this is specific to volatile ressources, but anyway, this causes us to be able to assume non-concurrency only for the first phase.

2: https://nhibernate.jira.com/browse/NH-3968#icft=NH-3968 indicates that we still have an issue there. In that prepare phase, as explained in NHibernate state, we open a transaction scope with a clone of the transaction in which the session is enlisted and then perform the flush. This mean we are (in most cases) re-using a connection, that was already enlisted in the transaction, for performing some additional work (the flush). And it looks like this is unsupported, in some cases the connection may be already prepared for commit and no more usable.

So it means we have to use a new connection for performing the flush inside the prepare phase. There, an AfterStatement release mode would avoid the trouble. (There are no way to control in which order the prepare phase is sent to each participant, so the session connection could be prepared before the flush or even concurrently to the flush. Re-using the session connection in the session prepare phase without ensuring it was not already enlisted seems to be a mistake.)

Frédéric Delaporte May 14, 2017 at 6:18 PM
Edited

Others related subjects

There are some subjects I have excluded from previous one for simplifying a bit.

Child sessions

A session may spawn other sessions. With NHibernate v4.1, those sessions where bounded to their parent session, flushed and closed from it.

With NH-4003, this has been realigned on Hibernate. A session opened from another may just reuse its settings, while not staying bound to the originating sessions. In such case it just behave like any other session, especially in regards to connection releases, flushes and closing.

User may also choose to share the "connection", causing the newly opened session to re-use the originating session's ConnectionManager (NHibernate; that is JdbcCoordinator and LogicalConnectionImplementor on Hibernate side). In such case, the closing of the newly opened session does not close the ConnectionManager. But the closing of the originating session still does, rendering dependent sessions unusable but not closing them.

Sharing the connection is interesting with explicit transaction, for participating in the transaction while being flushed or cleared distinctly. Within a transaction scope, this permits avoiding promoting a transaction to distributed by avoiding the acquisition of a new connection.

Currently this mechanism does not looks implied with current transaction issues. But we must not forget to keep it working when changing the transaction handling.

(Previously I was thinking there were some solutions to find for our transaction issues in Hibernate current handling of this "connection" sharing. So I have ported them.)

Connection releases suspension

There is also on Hibernate side an ability to suspend connection releases, with "suspension depth". On NHibernate side, it is only a flag. (With an additional one only for "DTC" case, which may be used on another thread.)

A pending recently merged PR ports it as a way to fix NH-2145. (NH-2145 could have been fixed by realigning AfterTransaction release modes on Hibernate instead, while still not enabling AfterStatement mode.)

Implicit transaction usage pattern

There is at least two usage patterns with implicit transaction, and NHibernate has to support both.

Transaction spanning session:

In this usage pattern, a transaction scope is opened first, then one or many sessions are opened and used inside it sequentially.

using (var scope = new TransactionScope()) { using (var s1 = OpenSession()) { ... // s1.Flush(); // optional currently } using (var s2 = OpenSession()) { ... // s2.Flush(); // optional currently } scope.Complete(); }

With this pattern and current NHibernate implementation, the connection acquired by s1 is not released until the transaction scope is disposed. This causes s2 usage to promote the transaction to distributed, even with SQL Server 2008 (which supports non distributed transaction having used many connections provided they were sequentially used).

Using child sessions for sharing the connection is here a workaround for avoiding promotion to distributed.

For this pattern to avoid promotion to distributed whatever the database is, only one connection should be acquired during the session life-cycle. So an AfterStatement connection release mode would not be recommended with this pattern.

Session spanning transaction:

In this usage pattern, a session is opened first, then one or many transaction are opened and used inside it sequentially.

using (var s = OpenSession()) { using (var scope1 = new TransactionScope()) { ... scope1.Complete(); } using (var scope2 = new TransactionScope()) { ... scope2.Complete(); } }

This pattern highly relies on connection releases. The session/transaction factory does not explicitly enlist the connection in the ambient transaction. The connection will participate in the current scope only if it is acquired within it. So this pattern should always fail with OnClose release mode. And this pattern requires the OnTransaction current behavior of releasing connection after an auto-commit statement, otherwise any session usage done before a transaction scope, without any transaction, will cause its connection to already be acquired and to not participate in the transaction scope.

It appears to be the pattern having the dire issues currently, due to the flush/connection release being triggered sometime on a concurrent thread from scope1 completion, while scope2 has already started and is using the connection.

Frédéric Delaporte May 14, 2017 at 5:53 PM
Edited

Hibernate state

NHibernate has ceased following closely Hibernate since Hibernate v3 (3.2 maybe). But even at that point it was having some discrepancies about transaction and connection handling. Here I will write about current Hibernate state. Here inaccuracies may be huge, I have no significant experiences with Java.

SQL connection life-cycle

The connection release mode setting has been deprecated in favor of a PhysicalConnectionHandlingMode setting. This introduces only one additional mode, and under the hood this is still the connection release mode which is internally used. So for the sake of simplicity, I will go on with connection release mode naming, but with that additional mode included.

  • AfterStatement: if the connection provider supports connection releases with ongoing transaction, will release the connection after each statement.

  • AterTransaction: releases the connection after each transaction. Does not apply to auto-commit mode: does not release connection after a statement when the session is in auto-commit mode. NHibernate differs on that point.

  • OnClose: same as NHibernate.

  • OnClose with "AcquiredOnOpen": same as OnClose, but the connection is immediately acquired on session opening.

With all those modes but the last, the connection is acquired only when the session begins to need it. No connections are acquired when opening a session unless the last mode is chosen. The connection is almost always released at latest on session close. It may not be the case when JPA is involved (see later), case in which they have done something similar to NHibernate dispose while enlisted in a transaction scope, which flag the session for auto-closing on transaction completion.

As with NHibernate, the configured value is auto by default. But with that value, depending on whether the connection provider supports releasing connection with ongoing transaction or not, it will choose between AfterStatement and AfterTransaction modes respectively. Reading this blog, it looks the AfterStatement mode is here for supporting some Java servers like WebSphere.

Flush modes

I have seen nothing worth mentioning in regards to what do NHibernate with its own flush modes.

Transactions

Hibernate transaction handling can either be auto-commit, explicit, implicit using JTA, implicit using JPA.

Auto-commit:

It sounds a bit like NHibernate auto-commit mode, excepted that Hibernate has to directly interact with the connection, enabling and disabling the mode on it. So there is maybe quite more under the hood.

Unlike NHibernate, it does not releases the connection if the connection release mode is AfterTransaction but only if it is AfterStatement.

Explicit:

I have seen nothing worth mentioning in regards to what do NHibernate with its own explicit mode.

Implicit with JTA:

JTA looks as the closest to what does TransactionScope. There are some major differences, like the ability to be directly manipulated by Hibernate (committed or rollback-ed).

Hibernate has an auto-join parameter which can be set to false on session opening. Otherwise, it joins automatically JTA transactions. I do not know whether not joining is equivalent to NHibernate "implicit without enlistment" mode or not, I mean, whether the statements will be transacted anyway or not.

Once joined in a JTA transaction, it will be notified of its life-cycle (commit, rollback). If a session is closed while a JTA transaction it has joined is ongoing, the session is closed nonetheless (and connection released). Its changes must have been flushed for being persisted. But the session may have register for the JTA transaction some after transaction completion actions, and those actions will still be executed on transaction completion. (Thus probably enabling correct behavior of second level cache.)

It does not look like Hibernate have any threading issues to handle there.

Implicit with JPA:

JPA is a specification for persistance API as ORM. It seems mainly inspired from the Hibernate API, but with discrepancies. Hibernate was implementing it as a separated extension, and has recently merged it in the core.

I have not studied it much. From a NHibernate user perspective, it looks like an additional layer mostly duplicating the ORM API for the sake of conformance to a Java standard.

In the case of JPA transaction, a session enlisted with it will not actually close on close call if the transaction is still ongoing, but will delay its closing to the transaction completion event. It does not seem there are any threading issues there either.

Fixed

Details

Assignee

Reporter

Labels

Fix versions

Affects versions

Priority

Who's Looking?

Open Who's Looking?
Created May 14, 2017 at 1:33 PM
Updated September 20, 2017 at 3:03 AM
Resolved September 7, 2017 at 9:45 AM
Who's Looking?