-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SaveChanges fails for SQL Server because of savepoints are not supported when MultipleActiveResultSets is enabled #23269
Comments
@roji Looks like this could be a regression caused by savepoints. |
Confirmed: in SQL Server, MARS and savepoints are incompatible. |
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;
}
} |
The current log warning says: 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? |
@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. |
So it seems like I am just as confused as @KanaHayama, ...it means when in a specific 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();
} |
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). |
Thanks @roji, so in my case I have to disable the warning. |
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 |
@KanaHayama yes, this has already been fixed in #24020. |
@roji options => options.UseSqlServer(connString)
.ConfigureWarnings(b => b.Ignore(SqlServerEventId.SavepointsDisabledBecauseOfMARS)) Still not sure about the value of this warning... |
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. |
@roji Now, facing the warning message of |
@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. |
Since 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;
}
} |
…et/efcore#23269 (comment) to fix performance issue with postgres subtransaction @ TbmDbContext.cs @ c#/shared
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
On 3.1.9 this code works without exceptions.
Include your code
Include stack traces
The text was updated successfully, but these errors were encountered: