eShopOnContainers icon indicating copy to clipboard operation
eShopOnContainers copied to clipboard

Possible double rollback issue.

Open mcuzmenco opened this issue 3 years ago • 5 comments

Hi,

In the following code you are attempting to Rollback transaction if it fails to be committed:

https://github.com/dotnet-architecture/eShopOnContainers/blob/873cdf8cefa46fa7bb4eb455925eee67101e44a2/src/Services/Ordering/Ordering.Infrastructure/OrderingContext.cs#L86-L95

However, what I notice is that transaction seems to be rolled back automatically if Commit fails and calling Rollback explicitly throws InvalidOperationException.

Is my understanding correct?

mcuzmenco avatar Mar 09 '21 22:03 mcuzmenco

Hi @mcuzmenco, thank you for reaching out.

Yes, your understanding is correct. The transaction should only rollback when it's failed to commit the changes. Could you please provide some more details on what scenario you are observing Rollback is throwing an exception ?

sughosneo avatar Mar 11 '21 05:03 sughosneo

I'm using very similar code in the project. When our clients execute API sometimes they are trying to create the same data concurrently which we prevent having the proper constraints in place. But what happens is that the victim transaction becomes deadlocked and fails on Commit rather than on SaveChangesAsync and is rolled back automatically. So when I'm calling RollbackTransaction after this I get a message that transaction is no longer usable with InvalidOperationException

mcuzmenco avatar Mar 11 '21 21:03 mcuzmenco

We are using Postgres, but I think the behavior would be the same for MSSQLServer. The error on commit:

Npgsql.PostgresException (0x80004005): 40P01: deadlock detected\n   at void Npgsql.NpgsqlConnector+<>c__DisplayClass160_0+<<DoReadMessage>g__ReadMessageLong|0>d.MoveNext()\n   at void Npgsql.NpgsqlConnector+<>c__DisplayClass160_0+<<DoReadMessage>g__ReadMessageLong|0>d.MoveNext()\n   at async Task Npgsql.NpgsqlConnector.ExecuteInternalCommand(byte[] data, bool async)\n   at async Task Npgsql.NpgsqlTransaction.Commit(bool async)

mcuzmenco avatar Mar 11 '21 21:03 mcuzmenco

Hi @mcuzmenco, I could think of the following possible reasons because of which you may see above error message.

  • DB Connection closed/lost for any reason (either through connection close command or network issues)
  • Multiple commits in code logic (probably a loop)
  • Error post commit in try catch with a rollback once the connection is closed

For sqltransaction related issue, you also may want to check the discussion This SqlTransaction has completed; it is no longer usable

sughosneo avatar Mar 29 '21 06:03 sughosneo

Hi @sughosneo

I'm pretty sure it's not any of the above. As I mentioned before we're using Postgres. It has a feature of "deferred constaints". (https://www.postgresql.org/docs/9.1/sql-set-constraints.html) As per documentation:

DEFERRED constraints are not checked until transaction commit.

This means that you can successfully call SaveChanges multiple times "temporarily" violating the constraints and then the final validation check is done on transaction.Commit. So it's not really related to connection issues/multiple commits and incorrect code flows. This is a real world project handling real users traffic. Exception can occur on transaction.Commit and attempt to rollback tries to rollback a transaction which has already been rolled back automatically.

mcuzmenco avatar Mar 29 '21 21:03 mcuzmenco

Hi @mcuzmenco, If an error occurs in a statement in the try block, execution transfers to the catch block where the transaction can be rolled back. For more information, you can refer to the section of documentation https://learn.microsoft.com/en-us/aspnet/web-forms/overview/data-access/working-with-batched-data/wrapping-database-modifications-within-a-transaction-cs#an-overview-of-transactions. I am closing this ticket, however, feel free to continue the conversation here.

erjain avatar Oct 06 '22 08:10 erjain