Skip to content
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

Invalid record created when handling Concurrency using transaction in Microsoft Azure Sql #19792

Closed
RaguMP opened this issue Feb 4, 2020 · 6 comments
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@RaguMP
Copy link

RaguMP commented Feb 4, 2020

I have handled Optimistic concurrency control in my case and used transactions. I am facing an issue where when an concurrency is occurred and while re-updating the record, creating an invalid row with the old value which I have read before concurrency occurred.

For example: I have three tables as follows.

ProjectCategorySequenceMapper - To maintain Sequence value based on project category
ProjectCategory - To maintain Project Category
Projects - To maintain Project details based on project category

I have used "RowVersion" column in "ProjectCategorySequenceMapper" table for concurrency check.

My goal is to read the sequence value based on project category from "ProjectCategorySequenceMapper" table when I am creating a new project and need to increment the sequence value by one and update in "ProjectCategorySequenceMapper" table.

In this scenario, when I try to create a new project (i.e. expected sequence Id - 3) under category (i.e. CategoryId - 1), once I read the value from database and concurrency occurred and in database sequence Id is updated as 4. I have handled to re-update concurrency and an invalid entry with Project Id 3 is created based on sequence value which is read before concurrency and new expected record created with Project Id 4 when I am using transaction. Without transaction everything works fine as expected whereas an invalid record created when using transaction.

Note:
In Microsoft Azure Sql Database only this invalid record created when using transaction whereas in normal Sql Database this issue not reproduced.

Kindly let me know is this an issue or anything done wrongly in my side.

Further technical details

EF Core version: 3.1.0
Database provider: Microsoft.EntityFrameworkCore.SqlServer
Target framework: .NET Core 3.1
IDE: Visual Studio 2019 16.4.4

@RaguMP RaguMP added the type-bug label Feb 4, 2020
@RaguMP RaguMP changed the title Invalid record created when handling Concurrency using transaction Invalid record created when handling Concurrency using transaction in Microsoft Azure Sql Feb 4, 2020
@ajcvickers
Copy link
Member

@RaguMP This sounds like the kind of behavior that happens if SaveChanges fails and then is called again without first rolling back the current transaction and starting a new one. In other words, it's generally not valid to call SaveChanges again in the same transaction after it has already failed once. Instead a new transaction is needed.

However, I don't believe the behavior here should be different between SQL Server and SQL Azure. If you post a small, runnable project that reproduces the behavior you are seeing then we can take a deeper look,

@RaguMP
Copy link
Author

RaguMP commented Feb 17, 2020

@ajcvickers: I have created a sample which reproduce the reported issue. I have created this sample using local database and attached the necessary SQL queries in the sample project too. Please find the uploaded sample in this github location and you can create the table in Azure SQL to check the issue by using the attached DB queries.

Note: Initially when I reported this issue I could not reproduce this issue in SQL server and so stated like the issue occurred in Azure SQL, but in this sample it is producing even in local database too. I am not able to find in which scenario this is not reproduced in SQL server previously.

Kindly check the sample and let me know if any additional detail needed from my end.

@ajcvickers
Copy link
Member

@RaguMP As I wrote above, the transaction needs to be rolled back then the DbUpdateConcurrencyException is caught. The retry should then create a new transaction for the next attempt. So basically, you just need to move the code that creates the transaction to inside the retry loop.

@ajcvickers ajcvickers added closed-no-further-action The issue is closed and no further action is planned. and removed type-bug labels Feb 17, 2020
@RaguMP
Copy link
Author

RaguMP commented Feb 18, 2020

@ajcvickers: I have tried to rollback the transaction once DbUpdateConcurrencyException occurred by creating transaction for each retry process, but still I am facing the same issue. i.e. an invalid entry created. I have updated the same sample by creating a new method AddProjectsUsingTransactionsWithRollback() in SQLServerController where the transaction had been rolled back in the catch block.

Note:
If I try to roll back the transaction as last statement once after refreshing the original values to bypass next concurrency check, still I am resulted with two invalid entries i.e. two times exception occurred and two invalid entries created. To reproduce this scenario, kindly replace the below code in the catch block.

catch (DbUpdateConcurrencyException ex)
{
       retryCount++;                        
       EntityEntry exceptionEntry = ex.Entries.Single();
       var databaseEntry = exceptionEntry.GetDatabaseValues();
       exceptionEntry.OriginalValues.SetValues(databaseEntry);
       await transaction.RollbackAsync().ConfigureAwait(false);
}

Kindly check the sample and let me know if I had done anything wrong.

@smitpatel smitpatel reopened this Feb 18, 2020
@ajcvickers
Copy link
Member

@RaguMP I spent some time debugging into your code to understand what you are trying to do. Consider this code:

var sequenceIdDetails = await context.TrProjectSequenceMapper.FirstOrDefaultAsync(i => i.IsActive && i.BrandId == brandId)
    .ConfigureAwait(false);

projectDetails.BrandProjectId = sequenceIdDetails.SequenceId;
sequenceIdDetails.SequenceId++;

The fist time through this code loads the sequenceDetails entity from the database and then uses and increments the SequenceId value. This is fine.

The second time through (i.e. in the retry) the context is always tracking the sequenceDetails. This means it doesn't get re-queried from the database. SequenceId on that object is then incremented again.

Ways to fix this:

@RaguMP
Copy link
Author

RaguMP commented Feb 24, 2020

@ajcvickers : Thanks for checking into this problem and for your workaround suggestion. But still I am facing the same issue. I have tried to detach the sequenceIdDetails details in catch block as a last statement once after refreshing the original values to bypass next concurrency check in sequenceIdDetails.

Note: If I don't refresh original values once after concurrency is occurred I am not getting updated with the latest value (i.e. In this sample TrProjectSequenceMapper table doesn't get updated with concurrency value.) I have added the below code in catch block by handling both concurrency check and detach of sequenceIdDetails details.

                    catch (DbUpdateConcurrencyException ex)
                    {
                        retryCount++;                        
                        await transaction.RollbackAsync().ConfigureAwait(false);
                        EntityEntry exceptionEntry = ex.Entries.Single();
                        var databaseEntry = exceptionEntry.GetDatabaseValues();
                        exceptionEntry.OriginalValues.SetValues(databaseEntry);
                        context.Entry(sequenceIdDetails).State = EntityState.Detached;
                    }

Kindly let me know whether I am in a proper way as per your suggestion or done any wrongly. If so kindly share me sample code snippet to overcome this problem.

@ajcvickers ajcvickers reopened this Oct 16, 2022
@ajcvickers ajcvickers closed this as not planned Won't fix, can't repro, duplicate, stale Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

3 participants