dotnet / efcore

EF Core is a modern object-database mapper for .NET. It supports LINQ queries, change tracking, updates, and schema migrations.
https://docs.microsoft.com/ef/
MIT License
13.72k stars 3.17k forks source link

EF Core upgrades to a distributed transaction when the TransactionScope contains multiple dbcontexts even if enlist is false #31544

Open yxou opened 1 year ago

yxou commented 1 year ago

After migrating from ef6 to efcore, when there are multiple DbContexts in the TransactionScope (even if they share the same connection), it returns an error the second time SaveChanges, even if enlist is false.

Refer to the document after set up the Database. AutoTransactionBehavior = AutoTransactionBehavior. Never. It didn't work either.

ef6 does not have this problem.

Specific stack:

Unhandled exception. System.InvalidOperationException: A root ambient transaction was completed before the nested transaction. The nested transactions should be completed first.
   at Microsoft.EntityFrameworkCore.Storage.RelationalConnection.HandleTransactionCompleted(Object sender, TransactionEventArgs e)
   at System.Transactions.InternalTransaction.FireCompletion()
   at System.Transactions.TransactionStateAborted.EnterState(InternalTransaction tx)
   at System.Transactions.TransactionStatePromotedBase.ChangeStateAbortedDuringPromotion(InternalTransaction tx)
   at System.Transactions.TransactionStateDelegatedBase.EnterState(InternalTransaction tx)
   at System.Transactions.EnlistableStates.Promote(InternalTransaction tx)
   at System.Transactions.Transaction.Promote()
   at System.Transactions.TransactionInterop.ConvertToDistributedTransaction(Transaction transaction)
   at System.Transactions.TransactionInterop.GetExportCookie(Transaction transaction, Byte[] whereabouts)
   at Microsoft.Data.SqlClient.SqlInternalConnection.GetTransactionCookie(Transaction transaction, Byte[] whereAbouts)
   at Microsoft.Data.SqlClient.SqlInternalConnection.EnlistNonNull(Transaction tx)
   at Microsoft.Data.SqlClient.SqlInternalConnection.Enlist(Transaction tx)
   at Microsoft.Data.SqlClient.SqlInternalConnection.EnlistTransaction(Transaction transaction)
   at Microsoft.Data.SqlClient.SqlConnection.EnlistTransaction(Transaction transaction)
   at Microsoft.EntityFrameworkCore.Storage.RelationalConnection.ConnectionEnlistTransaction(Transaction transaction)
   at Microsoft.EntityFrameworkCore.Storage.RelationalConnection.HandleAmbientTransactions()
   at Microsoft.EntityFrameworkCore.Storage.RelationalConnection.Open(Boolean errorsExpected)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.Execute(IEnumerable`1 commandBatches, IRelationalConnection connection)
   at Microsoft.EntityFrameworkCore.Storage.RelationalDatabase.SaveChanges(IList`1 entries)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(IList`1 entriesToSave)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(StateManager stateManager, Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.<>c.<SaveChanges>b__107_0(DbContext _, ValueTuple`2 t)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.Execute[TState,TResult](TState state, Func`3 operation, Func`3 verifySucceeded)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges(Boolean acceptAllChangesOnSuccess)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChanges()
   at Program.<Main>$(String[] args) in C:\Users\yxcha\Desktop\EfcoreDemo\Program.cs:line 20

Code : Model

[Table("testTable", Schema = "dbo")]
    public class TestModel
    {
        public TestModel(string code, string name)
        {
            this.Code = code;
            this.Name = name;
            this.CreateTime = DateTime.Now;
        }

        [Key]
        [MaxLength(36)]
        public string Code { get; set; }
        [MaxLength(50)]
        public string Name { get; set; }
        public DateTime CreateTime { get; set; }
    }

DbContext

 public class TestDbContext:DbContext
    {
        protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        {
            optionsBuilder.UseSqlServer("server=localhost;database=testDb;User ID=test;Password=123456;enlist=false;TrustServerCertificate=true;");
            base.OnConfiguring(optionsBuilder);
        }

        public virtual DbSet<TestModel> TestModels { get; set; }
    }

Program

using (var tran = new TransactionScope())
{

    using (var context1 = new TestDbContext())
    {
        string code = Guid.NewGuid().ToString();
        context1.TestModels.Add(new TestModel(code, $"test_{code}"));
        context1.SaveChanges();
    }

    using (var context2 = new TestDbContext())
    {
        string code = Guid.NewGuid().ToString();
        context2.TestModels.Add(new TestModel(code, $"test_{code}"));
        context2.SaveChanges();
    }
}

Example: EfcoreDemo.zip

EF Core version:7.0.5 Database provider: (e.g. Microsoft.EntityFrameworkCore.SqlServer:7.0.5) Target framework: (e.g. .NET 6.0) Operating system: IDE: (e.g. Visual Studio 2022 17.5)

ajcvickers commented 1 year ago

@yxou Distributed transactions are only supported on .NET 7 or later.

@roji There is some strange behavior here based on "Enlist=false" being included in the connection string--see my code below for playing around with SqlClient and EF Core in this space. The exception above happens when the connection string contains "Enlist=false" and TransactionManager.ImplicitDistributedTransactions = true; has not been called.

using System.ComponentModel.DataAnnotations.Schema;
using System.Transactions;
using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Logging;

// TransactionManager.ImplicitDistributedTransactions = true;

using (var context = new TestDbContext())
{
    context.Database.EnsureDeleted();
    context.Database.EnsureCreated();
}

using (var tran = new TransactionScope())
{
    using (var context1 = new TestDbContext())
    {
        context1.Add(new TestModel { Id = 1 });
        context1.SaveChanges();
    }

    using (var context2 = new TestDbContext())
    {
        context2.Add(new TestModel { Id = 2 });
        context2.SaveChanges();
    }
}

// using (var tran = new TransactionScope())
// {
//     using (var connection = new SqlConnection(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow;Enlist=false"))
//     {
//         connection.Open();
//         connection.EnlistTransaction(Transaction.Current);
//         using var command = connection.CreateCommand();
//         command.Parameters.Add(new SqlParameter("p0", 1));
//         command.CommandText = @"SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; INSERT INTO [TestModels] ([Id]) VALUES (@p0);";
//         command.ExecuteNonQuery();
//         connection.Close();
//     }
//
//     using (var connection = new SqlConnection(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow;Enlist=false"))
//     {
//         connection.Open();
//         connection.EnlistTransaction(Transaction.Current);
//         using var command = connection.CreateCommand();
//         command.Parameters.Add(new SqlParameter("p0", 2));
//         command.CommandText = @"SET IMPLICIT_TRANSACTIONS OFF; SET NOCOUNT ON; INSERT INTO [TestModels] ([Id]) VALUES (@p0);";
//         command.ExecuteNonQuery();
//         connection.Close();
//     }
// }

using (var context3 = new TestDbContext())
{
    Console.WriteLine(context3.TestModels.Count());
}

public class TestModel
{
    [DatabaseGenerated(DatabaseGeneratedOption.None)]
    public int Id { get; set; }
}

public class TestDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseSqlServer(@"Data Source=(LocalDb)\MSSQLLocalDB;Database=AllTogetherNow;enlist=false")
            //.UseSqlServer("server=localhost;database=testDb;User ID=test;Password=123456;enlist=false;TrustServerCertificate=true;");
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    }

    public virtual DbSet<TestModel> TestModels { get; set; }
}
yxou commented 1 year ago

@ajcvickers I now is to rewrite the SqlServerConnection, let SupportsAmbientTransactions attributes according to Enlist assignment, temporarily not sure there is no other sequelae.

using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal;
using Microsoft.EntityFrameworkCore.Storage;

internal class TestEFCoreSqlServerConnection : SqlServerConnection
{
    public Seagull2EFCoreSqlServerConnection(RelationalConnectionDependencies dependencies) : base(dependencies)
    {
    }
    protected override bool SupportsAmbientTransactions => new SqlConnectionStringBuilder(this.ConnectionString).Enlist;
}
herme063 commented 9 months ago

I am not actually clear about the resolution here. I am facing the same issue upgrading from 2.14 to 6.0.9. We initially encountered an error using System.Transactions and being upgraded to distributed when multiple contexts are used. After adding "Enlist=false" to the connection string, we got the same error above.

roji commented 9 months ago

The problem here is that EF's RelationalConnection.Open() enlists the connection in the ambient Transaction if the provider has SupportsAmbientTransactions has been overridden to true - which it is in most cases. We don't check the connection string to see whether Enlist is true or not.

This is indeed problematic... We could look at the connection string in SupportsAmbientTransactions and return false if Enlist=false - we already do this to know whether MARS is enabled or not. However, this problem goes beyond just SQL Server, and this would force all providers to look at the connection string (and introduce a caching layer as for SQL Server MARS...).

In an ideal world, EF wouldn't be dealing with ambient transactions at all, that would purely be an affair of the lower-level ADO.NET provider. I took a (very) quick look at and I'm not quite sure why we track/enlist to ambient transactions (except maybe in order to throw NestedAmbientTransactionError?), I think @AndriySvyryd/@ajcvickers may know.