1

I have the following methods

public async Task Foo()
    {
        try
        {

            //Do stuff
            bool inserted = false;
            int tries=0;
            while (!inserted && tries<2)
            {
                try
                {
                    inserted = await Bar();                        
                }
                catch (Exception ex)
                {
                    //log ex and continue
                }
                finally
                {
                  if(!inserted)
                  {
                     tries++;
                  }
                }
            }
        }
        catch (Exception ex)
        {
            //log ex and continue
        }
    }

and

public async Task<bool> Bar()
    {
        //setup opbject to be inserted to database

        try
        {
            //the table can not have auto incrememnt so we read the max value
            objectToBeAdded.id = Context.Set<object>().Max(o => o.id) + 1;
            await Context.Set<object>().AddAsync(objectToBeAdded);
            await Context.SaveChangesAsync();
            return true;
        }
        catch (Exception ex) {
            return false;
        }
    }

The code runs in a multi threaded environment and many times per minute so there is always a chance for the following exception.

Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details. ---> MySql.Data.MySqlClient.MySqlException: Duplicate entry 'XXXXX' for key 'PRIMARY' ---> MySql.Data.MySqlClient.MySqlException: Duplicate entry 'XXXXX' for key 'PRIMARY'

Unfortunately it's a very hard error to reproduce and our issue is that it crashes the application instead of retrying and moving on.

We can not change the table to support auto increment Primary Key.

Edit: The full stack trace as requested

-Error- Failed executing DbCommand (8ms) [Parameters=[@p0='?' (DbType = Int64), @p1='?' (DbType = Boolean), ....., @pN='?' (DbType = Decimal)], CommandType='Text', CommandTimeout='600'] INSERT INTO table (id, col1, ....colN) VALUES (@p0, @p1, .... @pN); -Error- An exception occurred in the database while saving changes for context type 'Entities'. Microsoft.EntityFrameworkCore.DbUpdateException: An error occurred while updating the entries. See the inner exception for details. ---> MySql.Data.MySqlClient.MySqlException: Duplicate entry 'XXXXX' for key 'PRIMARY' ---> MySql.Data.MySqlClient.MySqlException: Duplicate entry 'XXXXXX' for key 'PRIMARY' at MySqlConnector.Core.ServerSession.TryAsyncContinuation(Task1 task) in C:\.......\mysqlconnector\src\MySqlConnector\Core\ServerSession.cs:line 1248 at System.Threading.Tasks.ContinuationResultTaskFromResultTask2.InnerInvoke() at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state) --- End of stack trace from previous location where exception was thrown --- at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot) --- End of stack trace from previous location where exception was thrown --- at MySqlConnector.Core.ResultSet.ReadResultSetHeaderAsync(IOBehavior ioBehavior) in C:........\mysqlconnector\src\MySqlConnector\Core\ResultSet.cs:line 42 --- End of inner exception stack trace --- at MySql.Data.MySqlClient.MySqlDataReader.ActivateResultSet(ResultSet resultSet) in C:........\mysqlconnector\src\MySqlConnector\MySql.Data.MySqlClient\MySqlDataReader.cs:line 80 at MySql.Data.MySqlClient.MySqlDataReader.ReadFirstResultSetAsync(IOBehavior ioBehavior) in C:........\mysqlconnector\src\MySqlConnector\MySql.Data.MySqlClient\MySqlDataReader.cs:line 302 at MySql.Data.MySqlClient.MySqlDataReader.CreateAsync(MySqlCommand command, CommandBehavior behavior, ResultSetProtocol resultSetProtocol, IOBehavior ioBehavior) in C:........\mysqlconnector\src\MySqlConnector\MySql.Data.MySqlClient\MySqlDataReader.cs:line 287 at MySqlConnector.Core.TextCommandExecutor.ExecuteReaderAsync(String commandText, MySqlParameterCollection parameterCollection, CommandBehavior behavior, IOBehavior ioBehavior, CancellationToken cancellationToken) in C:........\mysqlconnector\src\MySqlConnector\Core\TextCommandExecutor.cs:line 37 at Microsoft.EntityFrameworkCore.Storage.Internal.RelationalCommand.ExecuteAsync(IRelationalConnection connection, DbCommandMethod executeMethod, IReadOnlyDictionary2 parameterValues, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken) --- End of inner exception stack trace --- at Microsoft.EntityFrameworkCore.Update.ReaderModificationCommandBatch.ExecuteAsync(IRelationalConnection connection, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Update.Internal.BatchExecutor.ExecuteAsync(DbContext _, ValueTuple2 parameters, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteImplementationAsync[TState,TResult](Func4 operation, Func4 verifySucceeded, TState state, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.Storage.ExecutionStrategy.ExecuteImplementationAsync[TState,TResult](Func4 operation, Func4 verifySucceeded, TState state, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(IReadOnlyList`1 entriesToSave, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.ChangeTracking.Internal.StateManager.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken) at Microsoft.EntityFrameworkCore.DbContext.SaveChangesAsync(Boolean acceptAllChangesOnSuccess, CancellationToken cancellationToken)

Alexander Talavari
  • 418
  • 2
  • 9
  • 24
  • Probably.. two threads grab the same max value and try to insert/update that max value + 1 into the primary key. – Paul Spiegel Feb 24 '19 at 12:17
  • @PaulSpiegel that's a given. The problem is that the program crashes with an unhandled exception. But we actually envelop the saveasync in 3 try catch block and not one of them had their code executted – Alexander Talavari Feb 24 '19 at 12:23
  • Then the error might be in the catch block :-) – Paul Spiegel Feb 24 '19 at 12:25
  • @PaulSpiegel the above code is an anonymized version of our production code. Even the full stack trace shows that SaveAsync is the culpit. – Alexander Talavari Feb 24 '19 at 12:29
  • 1
    What if you use some hardcoded id value like `objectToBeAdded.id = 1234;`. This should easily reproduce and you should be able to debug. – Ivan Stoev Feb 24 '19 at 12:55
  • @IvanStoev that's a good idea actually – Alexander Talavari Feb 24 '19 at 12:58
  • If that is really a full stack trace that crashes your application then it seems like you start some async operation without `await`ing it and have `ThrowUnobservedTaskExceptions enabled="true"` [set](https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.taskscheduler.unobservedtaskexception?view=netcore-2.0) , though I do not see anything similar in this code. Though, perhaps you have just slightly overanonymized the stacktrace. – Eugene Podskal Feb 24 '19 at 13:08
  • You don't tell anything about the lifecycle of `Context`. It's hard to tell if it's accessed by multiple threads. Also, the `try-catch` structure makes it hard to tell whether any exception makes it to the final catch at all. You should remove the `try-catch` from `Bar` entirely and in `Foo` remove `inserted`, and everything related to it, and in the `catch` just re`throw` when `tries >= 2`. – Gert Arnold Feb 24 '19 at 14:30

2 Answers2

0

As far as I can see, you do not remove the added object from the DbContext, so the duplicate key is still there.

You should

  1. Either remove(detach) it
  2. Or create a new DbContext from scratch each time just to be sure.
Eugene Podskal
  • 10,270
  • 5
  • 31
  • 53
  • you stand corrected that we should remove it from the context since every subsequent call to save async will try to add it in the database. But this does not explain why it was not caught by any of the try catch blocks – Alexander Talavari Feb 24 '19 at 12:27
  • Well, without [MCVE](https://stackoverflow.com/help/mcve) (and|or at least a full stack trace) it is difficult to say how could that happen. While making MCVE for race condition is difficult, in this particular case it is not impossible as you can synchonize the code in a way that alway leads to an exception. P.S.: Also, you have only 2 retries that seems too few. I'd have made at least 10, perhaps with small random delays in-between. P.S.2: And actually, if the risk of contention is too big, I'd even propose to serialize creation of such objects through some message queue or etc. – Eugene Podskal Feb 24 '19 at 12:32
  • About the retries is not a fixed number but rather an environment variable just wanted to show the retry logic. MCVE is hard to provide without breaking some kind of a clause in my contract.As for the full stack trace i am gonna annonymize it and post it in my question – Alexander Talavari Feb 24 '19 at 12:41
  • @AlexanderTalavari You can just create a MCVE with some dummy `MyCar` table that you try to insert to(while conditionally adding a new entity just before saving) and give us that stacktrace. There is a non-zero chance you may spot the issue by yourself in the process, and there is a zero chance to violate your contract in such a way. Making MCVE is a tedious and unrewarding process, unfortunately, but it is usually extremely effective. – Eugene Podskal Feb 24 '19 at 12:46
  • i added the stacktrace. if this doesnt help i will look to add a close enough MCVE – Alexander Talavari Feb 24 '19 at 12:52
0

I guess, what makes this error hard to reproduce is, that your codes runs asynchronously. You're getting the new max id by querying the current max id in the datacontext, which, in turn, may be changed right after you queried it, since another thread might also create a new entity.

What you should do, is putting a lock statement around the logic in your Bar()-method. By doing so, your second insert is being processed after your first one finished and thus all items share the same max id.

Something along the lines below should help, I didn't check it in visual studio, if it actually compiles, but you get the idea.

private object _lockObject = new object();
public async Task<bool> Bar()
{
   //setup object to be inserted to database

    try
    {
        // lock your changes, so they run in a safe order
        lock (_lockObject)
        {
            //the table can not have auto incrememnt so we read the max value
            objectToBeAdded.id = Context.Set<object>().Max(o => o.id) + 1;
            await Context.Set<object>().AddAsync(objectToBeAdded);
            await Context.SaveChangesAsync();
        }
        return true;
    }
    catch (Exception ex) {
        return false;
    }
}
T.M.
  • 79
  • 6
  • 1
    Locking unfortunately is not an option either. The table can be changed by other software as well multiple instances of the same program. We have accepted that duplicate key exception can happen. What we want is to mitigate it and not allow it to crash our whole application. My latest theory is something has to do with async – Alexander Talavari Feb 24 '19 at 12:33
  • Is implementing a stored procedure, which gives you the max id, an option? – T.M. Feb 24 '19 at 13:27