Timothy Shields
Timothy Shields

Reputation: 79601

How to atomic increment when row might not exist?

Suppose I have MyContext derived from DbContext with a DbSet<Item> Items, where Item is defined as follows.

public class Item
{
    [Key]
    public string Key { get; set; }

    [ConcurrencyCheck]
    public int Value { get; set; }
}

Given a key, I want to atomically increment the corresponding value, where keys that are not yet in the table have an implicit value of 0. That is, atomically incrementing a key not present in the table results in a value of 1. Here is my current approach:

public static async Task IncrementAsync(string key)
{
    using (var context = new MyContext())
    {
        while (true)
        {
            var item = await context.Items.FindAsync(key);
            if (item == null)
            {
                context.Items.Add(new Item { Key = key, Value = 1 });
            }
            else
            {
                item.Value++;
            }
            try
            {
                await context.SaveChangesAsync();
                break;
            }
            catch (DbUpdateException)
            {
                continue;
            }
        }
    }
}

This fails with a live-lock situation when many calls to IncrementAsync are running concurrently.

My entity framework experience is basically query-only, so if you could explain the finer details of what I'm doing wrong in this code I would really appreciate it.

Edit
Because the selected answer doesn't make it explicit, I'll place the corrected code here. Notice how the context is never reused after a DbUpdateException.

public static async Task IncrementAsync(string key)
{
    while (true)
    {
        using (var context = new MyContext())
        {
            var item = await context.Items.FindAsync(key);
            if (item == null)
            {
                context.Items.Add(new Item { Key = key, Value = 1 });
            }
            else
            {
                item.Value++;
            }
            try
            {
                await context.SaveChangesAsync();
                break;
            }
            catch (DbUpdateException)
            {
                continue;
            }
        }
    }
}

Upvotes: 3

Views: 296

Answers (2)

Muhammad Nagy
Muhammad Nagy

Reputation: 226

An alternative to the given answer is to use stored procedure to do all the work like the following example. Then you can call it from the application in a single line instead of the code above.

CREATE PROCEDURE SP_IncrementValue
    @ValueKey NVARCHAR(100)
AS
BEGIN
    BEGIN TRAN
       UPDATE Item WITH (SERIALIZABLE) SET Value = Value + 1
       WHERE [Key] = @ValueKey

       IF @@ROWCOUNT = 0
       BEGIN
          INSERT Item ([Key], Value) VALUES (@ValueKey, 1)
       END
    COMMIT TRAN
END
GO

This approach gives you a better performance and less error-prone.

Edit: To call the stored procedure from C# add the following method in EF ApplicationDbContext class

    public int IncrementValue(string valueKey)
    {
        return this.Database.ExecuteSqlCommand("EXEC SP_IncrementValue @ValueKey", new SqlParameter("@ValueKey", valueKey));
    }

Then you can call it from any instance of the DBContext Class

Upvotes: 2

Guvante
Guvante

Reputation: 19221

You need the context to not be shared between attempts. If at all possible don't do anything with a context that had a DbUpdateException as if you don't explicitly clean up the context it may never return to a normal state.

I would expect the outer context to cause issues. If concurrent calls to a single key happen depending on the timing you could create a bad context setup (which would be continually ignored due to your error handler.

Unless I am mistaken the fact that the Key exists in the database won't remove the "to be added" version. You will end up with one of these contexts:

Add "1", 2

or

Add "1", 1
Update "1", 2

Depending on whether your second iteration grabs the first iteration's object or a fresh one.

Neither of these can succeed so you end up with a continuous error.

Upvotes: 1

Related Questions