Concurrency detection/exceptions and delete

[roji]

While thinking about optimizing deletion, I started wondering: does it make sense to apply our concurrency logic for deletions, in the same way as we do for updates?

First, let's assume there's no concurrency token defined. The only reason a delete could throw a concurrency exception, is if someone else deleted the row first. But if that happened, our delete is idempotent - we don't really care if our delete actually deleted, we only care that the row is gone.

Even if there's a concurrency token defined, is the user's reasonable intent really to delete the row only if it hasn't been updated since loading? With updates, concurrency tokens protect against "split" updates, where two concurrent updates on the same row can make it inconsistent. But with deletions there's no such danger, since the entire row is deleted.

This has a perf aspect, since if we don't need to do concurrency checking for deletions, we can do bulk deletion (#27550).

Repro without concurrency token

await using var ctx = new BlogContext();
await ctx.Database.EnsureDeletedAsync();
await ctx.Database.EnsureCreatedAsync();

var blog = await ctx.Blogs.SingleAsync();
ctx.Blogs.Remove(blog);

using (var ctx2 = new BlogContext())
{
    var blog2 = await ctx2.Blogs.SingleAsync();
    ctx2.Blogs.Remove(blog2);
    await ctx2.SaveChangesAsync();
}

await ctx.SaveChangesAsync();

public class BlogContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
        => optionsBuilder
            .UseSqlServer(@"Server=localhost;Database=test;User=SA;Password=Abcd5678;Connect Timeout=60;ConnectRetryCount=0")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();

    protected override void OnModelCreating(ModelBuilder modelBuilder)
        => modelBuilder.Entity<Blog>().HasData(new Blog { Id = 1, Name = "first" });
}

public class Blog
{
    public int Id { get; set; }
    public string Name { get; set; }
}

[stevendarby]

Something to consider is that an application might have some business rules around deletion. For example, only allowing deletion when a column such as AllowDeletion is true. A concurrency token on such a column might be pretty useful for preventing the deletion of a record which has just recently had AllowDeletion switched to false.

Another use case for it might be if there is an auditing process on delete which audits the last values of the record being deleted. A concurrency token on a row version column could ensure the record is only deleted with an accurate audit of the last values.

The auditing example is also an argument for continuing to throw when there are no concurrency tokens but the entity has already been deleted. Without throwing, you could commit a 2nd deletion audit record with the save.

Not necessarily saying these are all reasons to keep the current behaviour but hopefully (if I've understood the proposal correctly) they are at least valid scenarios in existing applications you may want to consider!

[roji]

@stevendarby thanks for your input - those scenarios indeed make sense and we discussed some variations of them offline.

On the other side of the argument, it's worth noting that the current behavior is also problematic for various scenarios, where the user just wants to delete a record regardless of the current value of the concurrency token; a concurrency exception needlessly complicated by requiring retries. In other words, it seems like there are valid scenarios for both having the concurrency check and not having it. A manual opt-in/opt-out could be the way to go here (#10443).

[roji]

I just realized that a bulk delete can be performed while keeping a (limited) form of concurrency checking:

DELETE FROM Foo WHERE id IN (1, 2, 3) RETURNING 1;

This returns a row with 1 for each row deleted. This allows us to detect when a concurrency issue, but does not allow us to know exactly which entry was the problematic one. When there's a concurrency token, we can use row values for most databases (but not SQL Server):

DELETE FROM Foo WHERE (id, token) IN ((1, 2), (3, 4)) RETURNING 1;

If we'd be OK with the limited concurrency exception details, that means that perf wouldn't be a consideration for this issue any more.

[roji]

Design discussion:

• It turns out that the concurrency checks aren't blocking for implementing bulk deletes (Implement bulk deletion #27550).
• Regardless of perf, we agree that doing concurrency checks isn't ideal as the default behavior, but at this point we don't think it's worth making the breaking change.

#10443 tracks a general mechanism for opting out of concurrency checking.