Reputation: 79601
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.
while
loop be outside the using
, so that every attempt gets a new context? I tried this and it makes everything work, but I feel like I'm being inefficient creating and destroying so many contexts.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
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
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