How to deal with a DbContext after a transaction fails?

[devedse]

I'm currently working on an application that processes events. It could happen though that a specific event fails to process.

I've setup a simple example of this in the following repository:
https://github.com/devedse/TransactionCausesDuplicateKey

I process all events in a transaction to ensure that no incomplete changes can be done to the data.
Once a specific event fails to be processed due to an error coming from SQL (e.g. a duplicate key) this the change apparently remains as a change inside the DbContext.

Thus, if I start a new transaction that tried the same operation again (or even something completely different) the transaction still fails on that initial change I did outside of the transaction scope I'm currently expected to be in.

Steps to reproduce

I reproduced this with the following code https://github.com/devedse/TransactionCausesDuplicateKey/blob/master/TransactionCausesDuplicateKey/Services/PutThingsInDbService.cs:

Code that sends events:

        public async Task StartAsync(CancellationToken cancellationToken)
        {
            //This is the first event and should complete successfully
            await ProcessNewhireEvent("TEST1", "John", "0001", "0001");

            //This is an event with a unique event id but the same person details. It should fail on a duplicate key for the Employee Table
            await ProcessNewhireEvent("TEST2", "John", "0001", "0001");
            //This is as expected

            //When we now submit a new completely unrelated event, it still fails on trying to add the previous event
            await ProcessNewhireEvent("TEST3", "Henk", "0002", "0002");
        }

Code that processes events:

        public async Task ProcessNewhireEvent(string eventId, string personName, string personId, string personNumber)
        {
            Console.WriteLine($"Processing new hire event: EventId: {eventId}, PersonName: {personName}, PersonId: {personId}, PersonNumber: {personNumber}");

            var employeeToAdd = new DbEmployee()
            {
                PersonName = personName,
                PersonId = personId,
                PersonNumber = personNumber
            };

            var eventToAdd = new DbEvent()
            {
                UniqueEventId = eventId
            };

            try
            {
                //Default transaction level is read committed
                using (var trans = await _dbContext.Database.BeginTransactionAsync().ConfigureAwait(false))
                {
                    _dbContext.Employees.Add(employeeToAdd);
                    _dbContext.Events.Add(eventToAdd);

                    await _dbContext.SaveChangesAsync().ConfigureAwait(false);

                    await trans.CommitAsync().ConfigureAwait(false);

                }

                Console.WriteLine("Completed successfully :)");
            }
            catch (Exception ex)
            {
                _logger.LogError(ex.InnerException.Message);
            }
        }

Summary, questions and discussion

To me it feels strange that even though you're expecting to be in some kind of isolated scope due to the transaction you're apparently still saving / commiting data which has previously failed to commit to the database.

I expected that data to be gone and handled in my exception logic. (E.g. retry mechanisms).

So what would be the suggested approach for this?
Is there a way to isolate changes happening inside a transaction to make sure only these are committed during the SaveChanges happening inside that transaction?

Or should you simply dispose of the DbContext after every save changes due to the risk of it failing?

If so, what's the use of all the documentation describing on how to Inject a DbContext inside an ASP.NET service?

It seems that once you use it inside a HostedService (which is a singleton) the DbContext becomes a Singleton as well, does this cause part of the issues I'm running into?

If the approach for a hosted service would be to create a new DbContext every time you try to write something to the Database, what would be the suggested approach for creating this? Would that be a simple factory like this?:

    public class EmployeeDbFactory : IEmployeeDbFactory
    {
        public IConfiguration Configuration { get; }

        public EmployeeDbFactory(IConfiguration configuration)
        {
            Configuration = configuration;
        }

        public EmployeeDatastoreDbContext CreateDbContext()
        {
            var optionsBuilder = new DbContextOptionsBuilder<EmployeeDatastoreDbContext>();
            var config = Configuration.GetConnectionString("employeeConnectionstring");
            optionsBuilder.UseSqlServer(config);

            return new EmployeeDatastoreDbContext(optionsBuilder.Options);
        }
    }

Not trying to blame anyone of say the software works incorrectly right now, but to me everything regarding this is quite confusing. So hopefully someone can explain on if my findings are valid, or if my way of implementing this is simply wrong 😄.

Further technical details

EF Core version: 3.1.2
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.1
Operating system: Win-10
IDE: Visual Studio 2019 16.4.5

[ajcvickers]

@devedse If SaveChanges fails due to a DbUpdateException, then the entities tracked by the the context should remain in the same state as before SaveChanges was called. This facilitates retries, which is especially important when handling concurrency issues.

However, I don't think its unreasonable to instead throw everything away and start again, and I agree this is made problematic by the default injection pattern for injecting the context. #18575 and/or #15577 should help with this.

[devedse]

@ajcvickers , I was discussing this topic with some colleagues and all the main consensus was that we all expected that incase of a transaction, the lifetime of these changes would be the same as the actual SQL transaction. So if the transaction fails then changes in the DbContext would've also been rolled back.

But I can also imagine that if you're working with changing actual C# objects rather then just doing some add queries, this could also have some bad sideeffects somewhere else.

So yeah, I guess this issue is mainly due to the fact of EntityFramework being an ORM which caches changes and applying them on 'SaveChanges' rather then doing SQL queries when you execute actions on the items.

[ajcvickers]

Closing as question. #18575 and/or #15577 should make this much better.