Consider removing Transactions dependency from System.Data.Common

[weshaggard]

Currently System.Data.Common depends on System.Transactions because of DbConnection.EnlistTransaction. That API is currently not part of .NET Standard 2.0 so we should either include it along with Transactions to .NET Standard 2.0 or we should consider removing it from .NET Core and breaking this dependency on Transactions.

@terrajobst @stephentoub what are you guys thoughts?

[saurabh500]

@divega do you have any thoughts about this?
The issue #18918 talks about ambient transactions and DbConnection.EnlistTransaction is one of the APIs needed which allows connections to enlist into transactions apart from the Connection String parameter for the database driver provider.

[divega]

@saurabh500 as far as I understand we need to go in the opposite direction of what the title says and add back the APIs and auto-enlistment behavior related to System.Transactions into System.Data.Common and SqlClient on .NET Core.

If we don't do this the value of System.Transactions in .NET Core is very limited.

In my personal opinion the auto-listment behavior is more widely used than the explicit API.

If we add EnlistTransaction we should consider an async version as per #18225.

[jimcarley]

Please remember that distributed transactions are not yet supported in .NET Core for System.Transactions. You CAN use Transaction.EnlistPromotableSinglePhase and Transaction.EnlistDurable once and you will get a single phase commit. But if you try to use any of the TransactionInterop methods or try a second EnlistDurable call, you will get exceptions. For .NET Core, the initial support in System.Transactions is for local only/single phase commit transactions.

[saurabh500]

For System.Data.SqlClient, this means that a if ambient transactions are implemented, TransactionScope can encapsulate operations on a Single Database connection. If multiple connections are open in a transaction scope, the transaction is promoted to a distributed transaction, which based on @jimcarley 's comment cannot not be supported with the current support for .Net Core.
This functionality can be accomplished using DbTransaction objects.

@divega do you about other providers, if they upgrade to distributed transactions on multiple connections? With local only/single phase commit transactions does the support for ambient transactions offer any advantages over DbTransaction in terms of functionality? The programming model while consuming Sys.Transaction vs DbTransaction are different.

@jimcarley are distributed transactions support going to be available in .Net Core down the line?

The only value I see right now, is that the API allows enlisting into transactions using the TransactionScope programming model. The support for distributed transactions is not available in .Net Core. If an application on .Net core tries to use EnlistTransaction on .Net core for a use case where the transaction is meant to promoted to distributed transactions there would be runtime exceptions that will need to be dealt with.

Thoughts ?

[saurabh500]

@weshaggard Why was this issue created in the first place.
Is System.Data.Common having a dependency on System.Transaction problematic? I want to understand that if the API is removed and added in a later version, is there some additional work that needs to be done apart from fixing System.Data.Common dependencies?

[weshaggard]

@weshaggard Why was this issue created in the first place.

I filed the issue because I was looking at the layering and closure of all the corefx assemblies compared to .NET Core 1.1 and noticed this new dependency. System.Transactions in its current state in corefx doesn't pull in many more dependencies so it would be OK with me for this dependency to remain. I primarily wanted to pose the question to better understand the dependency and also to figure out if we want or need to expose Transactions in .NET Standard because of the API dependency in DbConnection. You guys are the experts in this space so I'm looking to you to provide a recommendation.

[divega]

does the support for ambient transactions offer any advantages over DbTransaction in terms of functionality?

I believe this was discussed in an internal thread which involved @YoungGah and others when @stephentoub started the port of System.Transactions to .NET Core. I don't remember all that was discussed but I remember there was some consensus that enabling ambient transactions was compelling even without the distributed transaction part.

I think this is an area in which it is ok for different database providers to have different levels of support although the API is in the base classes.

cc @roji in case he has some insights from the npgsql perspective.

[roji]

Not much to add except that yeah, IMHO System.Transactions is worth it even without distributed transactions - the fact the transaction is ambient and doesn't need to be handed down the stack is a good example.

[saurabh500]

Thanks @roji @divega
I had a discussion with the team.
It makes sense to have DbTransaction DbConnection.EnlistTransaction() in .Net Core System.Data.Common so that the providers can take advantage of the ambient transactions enlistment model.

@weshaggard Let me know if I can answer any questions

[weshaggard]

So you are suggesting we should just move DbTransaction and its closure into System.Data.Common instead of System.Transactions? If so can you please put together a full proposal for what types this would include?

[saurabh500]

Sorry bad comment from me. I edited it.
DbConnection.EnlistTransaction() should stay in System.Data.Common and it would bring in System.Transaction

[weshaggard]

OK I've opened dotnet/standard#157 to discuss adding System.Transactions to .NET Standard.

[jimcarley]

I haven't finished reading thru this, but I wanted to get @qizhanMS for visibility.

[weshaggard]

Transactions have been added to .NET Standard 2.0. dotnet/standard#168.