SaveChanges fails for SQL Server because of savepoints are not supported when MultipleActiveResultSets is enabled

[xiety]

File a bug

After upgrading from 3.1.9 to 5.0.0, calling SaveChangesAsync inside foreach throws SqlException, even when MultipleActiveResultSets is set to True

The transaction operation cannot be performed because there are pending requests working on this transaction.

On 3.1.9 this code works without exceptions.

Include your code

using Microsoft.EntityFrameworkCore;

using var db = new SampleContext();
await db.Database.EnsureDeletedAsync();
await db.Database.EnsureCreatedAsync();

var transaction = await db.Database.BeginTransactionAsync();

db.Add(new Table1());
await db.SaveChangesAsync();

foreach (var table1 in db.Table1)
{
    db.Add(new Table2());
    await db.SaveChangesAsync(); //exception here
}

public class SampleContext : DbContext
{
    const string connectionString =
        @"Server={server};Database={database};Trusted_Connection=True;MultipleActiveResultSets=true";

    public DbSet<Table1> Table1 { get; set; }
    public DbSet<Table2> Table2 { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => base.OnConfiguring(optionsBuilder.UseSqlServer(connectionString));
}

public class Table1
{
    public int Id { get; set; }
}

public class Table2
{
    public int Id { get; set; }
}

Include stack traces

Unhandled exception. Microsoft.Data.SqlClient.SqlException (0x80131904): The transaction operation cannot be performed because there are pending requests working on this transaction.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.SqlInternalConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at Microsoft.Data.SqlClient.SqlCommand.InternalEndExecuteNonQuery(IAsyncResult asyncResult, Boolean isInternal, String endMethod)
   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryInternal(IAsyncResult asyncResult)
   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryAsync(IAsyncResult asyncResult)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
--- End of stack trace from previous location ---
   at Microsoft.EntityFrameworkCore.Storage.RelationalTransaction.CreateSavepointAsync(String name, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTransaction.CreateSavepointAsync(String name, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(IEnumerable`1 commandBatches, IRelationalConnection connection, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IList`1 entriesToSave, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(DbContext _, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.SqlServer.Storage.Internal.SqlServerExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)```

### Include provider and version information

EF Core version: 5.0.0
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET 5.0

[ajcvickers]

@roji Looks like this could be a regression caused by savepoints.

[roji]

Confirmed: in SQL Server, MARS and savepoints are incompatible.

[roji]

Here's a workaround that disables savepoints which can be used with 5.0.0, reverting to the 3.1 behavior:

public class MyDbContext : DbContext
{
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(....)
            .ReplaceService<IRelationalTransactionFactory, NoSavepointsTransactionFactory>();
}

public class NoSavepointsTransactionFactory : IRelationalTransactionFactory
{
    public virtual RelationalTransaction Create(
        IRelationalConnection connection,
        DbTransaction transaction,
        Guid transactionId,
        IDiagnosticsLogger<DbLoggerCategory.Database.Transaction> logger,
        bool transactionOwned)
        => new NoSavepointsTransaction(connection, transaction, transactionId, logger, transactionOwned);

    class NoSavepointsTransaction : RelationalTransaction
    {
        public NoSavepointsTransaction(
            IRelationalConnection connection,
            DbTransaction transaction,
            Guid transactionId,
            IDiagnosticsLogger<DbLoggerCategory.Database.Transaction> logger,
            bool transactionOwned)
            : base(connection, transaction, transactionId, logger, transactionOwned)
        {
        }

        public override bool SupportsSavepoints => false;
    }
}

[KanaHayama]

The current log warning says: Savepoints are disabled because Multiple Active Result Sets (MARS) is enabled. If 'SaveChanges' fails, then the transaction cannot be automatically rolled back to a known clean state. Instead, the transaction should be rolled back by the application before retrying 'SaveChanges'. See https://go.microsoft.com/fwlink/?linkid=2149338 for more information. To identify the code which triggers this warning, call 'ConfigureWarnings(w => w.Throw(RelationalEventId.SavepointsDisabledBecauseOfMARS))'.

I am new to ef core, I do not understand what does this log warning exactly means.

I want the behaviour that if any thing goes wrong, the DB tables are leave unchanged. I think transaction is the right approach. I need DB generated id, so I have to call SaveChanges() several times in a transaction. Intermediate savepoints are not needed in my case.

I know transactions with savepoints enabled is newly introduced in ef core 5, and been disabled in ef core 5.0.3 for SQL server MARS. So I think this is an additional feature, that if I do not use savepoints, I do not need to care about it. I suppose transactions can be used by wrapping statements between BeginTransaction() and Commit().

While this log massage makes me confuse. I am not using any savepoint, and I am using SQL Server with MARS enabled. Will the behavior of transaction with savepoints disabled different from what I supposed in this case? Will I still get my tables unchanged if any step in a transaction goes wrong rather than get into some intermediate state? Should I manually invoke something additionally to rollback the transaction to its initial state if any error occurs in this case? What is the behavior of transactions in ef core 3.1? Please help make this warning message clear.

And also, how can I disable this warning log message?

[roji]

@KanaHayama there is more information in the doc page linked to from this warning.

If you perform multiple SaveChanges on a manually-managed transaction (which seems to be the case), EF Core by default uses savepoints; that way, if something goes wrong in a specific SaveChanges, anything already done in that SaveChanges can be rolled back and you can safely retry (or do something else). If savepoints are disabled, then it's possible for certain operations to have been applied within the transaction before the error occurred; that means that the transaction is now in an unknown state.

You can either disable MARS - allowing EF Core to create the savepoints - or suppress the warning as per the docs, and execute without savepoints. I hope that clarifies things.

[msschl]

@roji

So it seems like I am just as confused as @KanaHayama,

...it means when in a specific SaveChanges something goes wrong, anything in that specific SaveChanges can be rolled back.
However, when I don't want to continue and rather want to rollback the entire transaction, there is no need for the savepoint feature, or is there?

So this should be totally valid and work as expected?

using var transaction = await dbContext.Database.BeginTransactionAsync(cancellationToken);

var total = 0;
var count = 0;

try
{
    await foreach (var entry in DegreeDaysParser.ParseAsync(stream, Encoding.UTF8, site.Date,
        DegreeDaysImporter.BATCH_SIZE, cancellationToken))
    {
        await dbContext.DegreeDays.AddAsync(entry, cancellationToken);
        count++;

        if (count == BATCH_SIZE)
        {
            await dbContext.SaveChangesAsync(cancellationToken); // NOTE: No need for a savepoint
            total += count;
            count = 0;
        }
    }

    if (count > 0)
    {
        // Save remaining entities...
        await dbContext.SaveChangesAsync(cancellationToken); // NOTE: No need for a savepoint
        total += count;
    }

    await transaction.CommitAsync(cancellationToken);
}
catch (Exception ex)
{
    this.logger.LogError(ex, "Error while importing degree days. Aborting this commit and rolling back.");
    await transaction.RollbackAsync(cancellationToken); // NOTE: Rollback entire transaction
}
finally
{
    await transaction.DisposeAsync();
}

[roji]

Yes, that's correct; if upon failure of any specific SaveChanges, you want to roll back the entire transaction, then savepoints indeed aren't helpful and can be disabled. Savepoints can help in case you want to retry individual SaveChanges within a transaction.

Specifically regarding the code above... If you want to have a maximum saving batch size, you can simply configure EF Core to do that for you: see the docs on batching. In other words, you can simply add all instances to the context and call SaveChanges once, and the batching would be handled for you under the hood.

(You may still want to batch yourself if you don't want to wait to get all the results from DegreeDaysParser.ParseAsync before sending data to the database).

[msschl]

Thanks @roji,

so in my case I have to disable the warning.

[KanaHayama]

Thank you @roji !

From the explanation above. disabling this warning is what I need. Hope this warning message can be updated. And I just found SavepointsDisabledBecauseOfMARS is a member of SqlServerEventId not RelationalEventId as in the warning message.

[roji]

@KanaHayama yes, this has already been fixed in #24020.

[changhuixu]

@roji
In .net 6 and EF Core 6, we still need to do this:

options => options.UseSqlServer(connString)
                  .ConfigureWarnings(b => b.Ignore(SqlServerEventId.SavepointsDisabledBecauseOfMARS))

Still not sure about the value of this warning...
Since MARS is enabled, why does EF worry about SavePoints?

[roji]

Savepoints are important in order to roll back the partial effects of SaveChanges in case an error occurs in it - when a user transaction is in progress. If you don't use user transactions, then that warning can be safely disabled, but otherwise this could affect program behavior; so the warning is there to make people aware of that.

[changhuixu]

@roji
Thank you. Yes, I know the use cases for Savepoints that allows us to stage one or more points and then we can conditionally restore to a specific point if needed: transaction.CreateSavepoints("GoodState") and transaction.RollbackToSavepoint("GoodState")

Now, facing the warning message of Savepoints are disabled because Multiple Active Result Sets (MARS) is enabled., I am wondering if the key/value MultipleActiveResultSets=True is needed in the SQL connection string. But without MultipleActiveResultSets=True, will the transaction behave differently?

[roji]

@changhuixu whether or not MultipleActiveResultSets=True is needed on the connection string is a question that's unrelated to savepoints; some applications rely on having multiple command resultsets at the same time. It's generally recommended to avoid MARS as there are various known issues and performance problems with it.

However, if you do enable MARS, and you use user transactions, then EF won't be able to create a savepoint at the start of SaveChanges. That means that if there's a failure in the middle, things will be left in an inconsistent state without the possibility to undo the SaveChanges and try it again. If that's OK with you, suppress the warning.

[n0099]

#23269 (comment)

Here's a workaround that disables savepoints which can be used with 5.0.0, reverting to the 3.1 behavior:

Since v6.0.0-preview.1.21102.2 the ctor of RelationalTransaction added a new param ISqlGenerationHelper sqlGenerationHelper: https://github.com/dotnet/efcore/pull/23036/files#diff-360dcdb9e8ca48b34987b29265a194977f7eedf1e1d8c7c4a597141df6b0b443R69, so we need something like https://github.com/dotnet/efcore/pull/9792/files#diff-e50873e4aeea395fe6aa9df7d1aed8b9e2e699a96fdfbc7dc3bceedd6f488b56R25:

protected class NoSavePointTransactionFactory(RelationalTransactionFactoryDependencies dependencies)
    : IRelationalTransactionFactory
{
    protected virtual RelationalTransactionFactoryDependencies Dependencies { get; } = dependencies;

    public virtual RelationalTransaction Create(
        IRelationalConnection connection,
        DbTransaction transaction,
        Guid transactionId,
        IDiagnosticsLogger<DbLoggerCategory.Database.Transaction> logger,
        bool transactionOwned)
        => new NoSavePointTransaction(
            connection, transaction, transactionId, logger, transactionOwned, Dependencies.SqlGenerationHelper);

    private sealed class NoSavePointTransaction(IRelationalConnection connection,
        DbTransaction transaction,
        Guid transactionId,
        IDiagnosticsLogger<DbLoggerCategory.Database.Transaction> logger,
        bool transactionOwned,
        ISqlGenerationHelper sqlGenerationHelper)
        : RelationalTransaction(
            connection, transaction, transactionId, logger, transactionOwned, sqlGenerationHelper)
    {
        public override bool SupportsSavepoints => false;
    }
}