DbContext.SaveChanges throws InvalidOperationException instead of DbUpdateConcurrencyException when command execution triggers pg serialization failure error 40001

[benjamincburns]

I'm writing a test for my project that is purposefully causing a concurrent update conflict in two parallel read-modify-write operations. I'm running my test at various isolation levels in order to validate that the expected concurrency error (DbUpdateConcurrencyException) is thrown in each case.

In less restrictive isolation levels (e.g. ReadCommitted, ReadUncommitted, and Unspecified), calling DbContext.SaveChangesAsync triggers the expected DbUpdateConcurrencyException, and all is well.

However at isolation levels where serialization anomaly prevention kicks in (RepeatableRead/Snapshot, and Serializable), the command execution results in a 40001 serialization_failure error. That error is then wrapped in two layers of exceptions and is caught by my test as an InvalidOperationException with the vague catch-all message: An exception has been raised that is likely due to a transient failure.

I understand that the mechanism by which the InvalidOperationException is triggered is different from the less restrictive isolation levels, however given the nature of the error I still would have expected this to have been wrapped in a DbUpdateConcurrencyException. For example, when this failure occurs, I see the following message in the database logs and in the innermost PostgresException: could not serialize access due to concurrent update.

Unfortunately I don't have a minimum executable reproduction for this, but I can probably knock one together tomorrow. In the meantime, I share the relevant exception details below. Also for reference this is targeting netcore 6, with Microsoft.EntityFrameworkCore and related packages all version 6.0.26. The Npgsql.EntityFrameworkCore.PostgreSQL package is version 6.0.22.

InvalidOperationException has the following stack trace:

   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlExecutionStrategy.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)
   at MyDbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken) in <redacted>/MyDbContext.cs:line 103

Its inner exception is DbUpdateException with message An error occurred while saving the entity changes. See the inner exception for details. It has the following stack trace:

   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(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.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(StateManager stateManager, Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlExecutionStrategy.ExecuteAsync[TState,TResult](TState state, Func`4 operation, Func`4 verifySucceeded, CancellationToken cancellationToken)

Finally, the DbUpdateException has inner exception PostgresException with Code = 40001, message 40001: could not serialize access due to concurrent update, and the following stack trace:

   at Npgsql.Internal.NpgsqlConnector.<ReadMessage>g__ReadMessageLong|223_0(NpgsqlConnector connector, Boolean async, DataRowLoadingMode dataRowLoadingMode, Boolean readingNotifications, Boolean isReadingPrependedMessage)
   at Npgsql.NpgsqlDataReader.NextResult(Boolean async, Boolean isConsuming, CancellationToken cancellationToken)
   at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
   at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
   at Npgsql.NpgsqlCommand.ExecuteDbDataReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Storage.RelationalCommand.ExecuteReaderAsync(RelationalCommandParameterObject parameterObject, CancellationToken cancellationToken)
   at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken)

[benjamincburns]

For now my ugly workaround is to apply the below pattern to my SaveChanges and SaveChangesAsync overrides:

try
{
    // unrelated project-specific logic here
}
catch (InvalidOperationException e)
{
    // Work around an error reporting bug in Npgsql.EntityFrameworkCore:
    // For details, see https://github.com/npgsql/efcore.pg/issues/3084
    if (e.InnerException is DbUpdateException
        {
            InnerException: PostgresException { SqlState: "40001" } postgresException
        })
    {
        throw new DbUpdateConcurrencyException(postgresException.Message, postgresException);
    }

    throw;
}

[roji]

That's an interesting question... At least at the moment, DbUpdateConcurrencyException is only thrown for concurrency errors that EF itself detects, i.e. when EF's own optimistic concurrency mechanism detects a problem (via the concurrency token). This does not cover database errors which represent concurrency issues.

Can you please open this on https://github.com/dotnet/efcore so we can discuss the general strategy here at the EF level? I think this is definitely something we could consider, but it isn't a PG-only question.

[benjamincburns]

@roji I can, although it might be better if you open it, as I suspect that you'd be able to shape it toward the conversation you'd want to have a bit better than I would and therefore save a bit of back-and-forth around clarifying the issue.

Edit:

In the case where you'd still prefer that I open it, can you please clarify the design concern/question you'd like me to raise?

I think it's some version of:

How should provider-specific concurrent update errors (e.g. PostgreSQL transaction serialization error 40001) be reported to the caller? DbUpdateConcurrencyException is the best candidate for an existing exception to be thrown, but at present it is only used for optimistic concurrency update conflicts arising due to concurrency tokens.

[roji]

@benjamincburns it's always OK to post an issue on the EF repo - if a few back and forths are needed for clarifications that's also not a problem.

But in any case, I've posted this as dotnet/efcore#33068 - note that I've added some downsides to the proposal as well (notably the breaking change aspect).

Thanks for raising this!

[benjamincburns]

Thanks for that @roji. As I suspected, you wrote it up better than I could have, and frankly I think given your status in the community it's less likely to go unnoticed.

[roji]

@benjamincburns oh it really doesn't work like that... If the proposal has merit we discuss it either way - I promise.

[benjamincburns]

@roji I just noticed that you closed this. I just wanted to ask - do you consider the exception that's being thrown, and especially its exception message, to be by design here? Personally I still feel that some improvement is necessary due to the misleading nature of the exception message. I mean, ideally a more specific exception type would be thrown, or the nature of the error would be more discernable from the top-level exception, but at the very least I don't think this should be portrayed as a transient issue.

[benjamincburns]

I'll bring this comment over to the other ticket as well, as it's likely relevant there.