Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

SQL Server deadlock when only inserting new rows without performing any selects

I am seeing constant deadlocks in my app, even though it performs no select statements, no delete statements, and no update statements. It is only inserting completely new data.

TL;DR: It seems to be related to the foreign key. If I remove that then I don't get any deadlocks at all. But that is not an acceptable solution for obvious reasons.

Given the following table structure

CREATE TABLE [dbo].[IncomingFile]
(
    [Id] UNIQUEIDENTIFIER NOT NULL, 
    [ConcurrencyVersion] RowVersion NOT NULL,
    CONSTRAINT [PK_IncomingFile] PRIMARY KEY CLUSTERED([Id])
)
GO

CREATE TABLE [dbo].[IncomingFileEvent]
(
    [Id] UNIQUEIDENTIFIER NOT NULL, 
    [ConcurrencyVersion] RowVersion NOT NULL,
    [IncomingFileId] UNIQUEIDENTIFIER NOT NULL,
    CONSTRAINT [PK_IncomingFileEvent] PRIMARY KEY CLUSTERED([Id]),
    CONSTRAINT [FK_IncomingFileEvent_IncomingFileId]
        FOREIGN KEY ([IncomingFileId])
        REFERENCES [dbo].[IncomingFile] ([Id])
)
GO

When I hit a number of concurrent tasks inserting data, I always see a deadlock. READ_COMMITTED_SNAPSHOT is enabled in my DB options (even though I am not reading anyway).

Here is the code that will reproduce the problem. If you do not experience the problem, increase the NumberOfTasksPerCpu constant at the top of the program.

using System;
using System.Collections.Generic;
using System.Data.SqlClient;
using System.Diagnostics;
using System.Text;
using System.Threading;
using System.Threading.Tasks;

namespace SqlServerDeadlockRepro
{
    class Program
    {
        private const int NumberOfTasksPerCpu = 8; // Keep increasing this by one if you do not get a deadlock!
        private const int NumberOfChildRows = 1_000;
        private const string MSSqlConnectionString = "Server=DESKTOP-G05BF1U;Database=EFCoreConcurrencyTest;Trusted_Connection=True;";

        private static int NumberOfConcurrentTasks;

        static async Task Main(string[] args)
        {
            NumberOfConcurrentTasks = Environment.ProcessorCount * NumberOfTasksPerCpu;

            var readySignals = new Queue<ManualResetEvent>();
            var trigger = new ManualResetEvent(false);
            var processingTasks = new List<Task>();
            for (int index = 0; index < NumberOfConcurrentTasks; index++)
            {
                var readySignal = new ManualResetEvent(false);
                readySignals.Enqueue(readySignal);
                var task = CreateDataWithSqlCommand(trigger, readySignal);
                processingTasks.Add(task);
            }
            Console.WriteLine("Waiting for tasks to become ready");
            while (readySignals.Count > 0)
            {
                var readySignalBatch = new List<WaitHandle>();
                for(int readySignalCount = 0; readySignals.Count > 0 && readySignalCount < 64; readySignalCount++)
                {
                    readySignalBatch.Add(readySignals.Dequeue());
                }
                WaitHandle.WaitAll(readySignalBatch.ToArray());
            }
            Console.WriteLine("Saving data");
            var sw = Stopwatch.StartNew();
            trigger.Set();
            await Task.WhenAll(processingTasks.ToArray());
            sw.Stop();
            Console.WriteLine("Finished - " + sw.ElapsedMilliseconds);
        }

        private static int TaskNumber = 0;
        private static async Task CreateDataWithSqlCommand(ManualResetEvent trigger, ManualResetEvent readySignal)
        {
            await Task.Yield();
            using var connection = new SqlConnection(MSSqlConnectionString);
            await connection.OpenAsync().ConfigureAwait(false);
            var transaction = (SqlTransaction)await connection.BeginTransactionAsync(System.Data.IsolationLevel.ReadCommitted).ConfigureAwait(false);

            Console.WriteLine("Task " + Interlocked.Increment(ref TaskNumber) + $" of {NumberOfConcurrentTasks}  ready ");
            readySignal.Set();
            trigger.WaitOne();
            Guid parentId = Guid.NewGuid();
            string fileCommandSql = "insert into IncomingFile (Id) values (@Id)";
            using var fileCommand = new SqlCommand(fileCommandSql, connection, transaction);
            fileCommand.Parameters.Add("@Id", System.Data.SqlDbType.UniqueIdentifier).Value = parentId;
            await fileCommand.ExecuteNonQueryAsync().ConfigureAwait(false);

            using var fileEventCommand = new SqlCommand
            {
                Connection = connection,
                Transaction = transaction
            };
            var commandTextBulder = new StringBuilder("INSERT INTO [IncomingFileEvent] ([Id], [IncomingFileId]) VALUES ");
            for (var i = 1; i <= NumberOfChildRows * 2; i += 2)
            {
                commandTextBulder.Append($"(@p{i}, @p{i + 1})");
                if (i < NumberOfChildRows * 2 - 1)
                    commandTextBulder.Append(',');

                fileEventCommand.Parameters.AddWithValue($"@p{i}", Guid.NewGuid());
                fileEventCommand.Parameters.AddWithValue($"@p{i + 1}", parentId);
            }

            fileEventCommand.CommandText = commandTextBulder.ToString();
            await fileEventCommand.ExecuteNonQueryAsync().ConfigureAwait(false);
            await transaction.CommitAsync().ConfigureAwait(false);
        }
    }
}

UPDATE

Also tried making the primary key NONCLUSTERED and adding a CLUSTERED index based on the current date and time.

CREATE TABLE [dbo].[IncomingFile]
(
    [Id] UNIQUEIDENTIFIER NOT NULL, 
    [ConcurrencyVersion] RowVersion NOT NULL,
    [CreatedUtc] DateTime2 DEFAULT GETDATE(),
    CONSTRAINT [PK_IncomingFile] PRIMARY KEY NONCLUSTERED([Id])
)
GO
CREATE CLUSTERED INDEX [IX_IncomingFile_CreatedUtc] on [dbo].[IncomingFile]([CreatedUtc])
GO

CREATE TABLE [dbo].[IncomingFileEvent]
(
    [Id] UNIQUEIDENTIFIER NOT NULL, 
    [ConcurrencyVersion] RowVersion NOT NULL,
    [IncomingFileId] UNIQUEIDENTIFIER NOT NULL,
    [CreatedUtc] DateTime2 DEFAULT GETDATE(),
    CONSTRAINT [PK_IncomingFileEvent] PRIMARY KEY NONCLUSTERED([Id]),
    CONSTRAINT [FK_IncomingFileEvent_IncomingFileId]
        FOREIGN KEY ([IncomingFileId])
        REFERENCES [dbo].[IncomingFile] ([Id])
)
GO
CREATE CLUSTERED INDEX [IX_IncomingFileEvent_CreatedUtc] on [dbo].[IncomingFileEvent]([CreatedUtc])
GO

UPDATE 2

I tried a sequential guid taken from here, which made no difference.

UPDATE 3

It seems to be related to the foreign key. If I remove that then I don't get any deadlocks at all.

UPDATE 4

A reply from Sql Server Product Group with some suggestions has been posted on my original github issue.

https://github.com/dotnet/efcore/issues/21899#issuecomment-683404734​​​​​​​

like image 878
Peter Morris Avatar asked Aug 08 '20 09:08

Peter Morris


2 Answers

The deadlock is due to the execution plan needed to check referential integrity. A full table scan of the IncomingFile table is performed when inserting a large number (1K) rows into the related IncomingFileEvent table. The scan acquires a shared table lock that's held for the duration of the transaction and leads to the deadlock when different sessions each hold an exclusive row lock on the just inserted IncomingFile row and are blocked by another sessions exclusive row lock.

Below is the execution plan that shows this:

plan with full scan

One way to avoid the deadlock is with an OPTION (LOOP JOIN) query hint on the IncomingFileEvent insert query:

    var commandTextBulder = new StringBuilder("INSERT INTO [IncomingFileEvent] ([Id], [IncomingFileId]) VALUES ");
    for (var i = 1; i <= NumberOfChildRows * 2; i += 2)
    {
        commandTextBulder.Append($"(@p{i}, @p{i + 1})");
        if (i < NumberOfChildRows * 2 - 1)
            commandTextBulder.Append(',');

        fileEventCommand.Parameters.AddWithValue($"@p{i}", Guid.NewGuid());
        fileEventCommand.Parameters.AddWithValue($"@p{i + 1}", parentId);
    }
    commandTextBulder.Append(" OPTION (LOOP JOIN);");

This is the plan with the hint:

plan with hint

On a side note, consider the changing the existing primary key to the one below. This is more correct from a data modeling perspective (identifying relationship) and will improve performance of both insert and selects since related rows are physically clustered together.

CONSTRAINT [PK_IncomingFileEvent] PRIMARY KEY CLUSTERED(IncomingFileId, Id)
like image 105
Dan Guzman Avatar answered Oct 11 '22 16:10

Dan Guzman


I wrote the following extension to solve the problem for EF Core.

protected override void OnConfiguring(DbContextOptionsBuilder options)
{
    base.OnConfiguring(options);
    options.UseLoopJoinQueries();
}

Using this code...

    public static class UseLoopJoinQueriesExtension
    {
        public static DbContextOptionsBuilder UseLoopJoinQueries(this DbContextOptionsBuilder builder)
        {
            if (builder is null)
                throw new ArgumentNullException(nameof(builder));

            builder.AddInterceptors(new OptionLoopJoinCommandInterceptor());
            return builder;
        }
    }

    internal class OptionLoopJoinCommandInterceptor : DbCommandInterceptor
    {
        public override Task<InterceptionResult<DbDataReader>> ReaderExecutingAsync(DbCommand command, CommandEventData eventData, InterceptionResult<DbDataReader> result, CancellationToken cancellationToken = default)
        {
            AppendOptionToSql(command);
            return Task.FromResult(result);
        }

        public override InterceptionResult<DbDataReader> ReaderExecuting(DbCommand command, CommandEventData eventData, InterceptionResult<DbDataReader> result)
        {
            AppendOptionToSql(command);
            return result;
        }

        private static void AppendOptionToSql(DbCommand command)
        {
            const string OPTION_TEXT = " OPTION (LOOP JOIN)";
            string[] commands = command.CommandText.Split(";");

            for (int index = 0; index < commands.Length; index++)
            {
                string sql = commands[index].Trim();
                if (sql.StartsWith("insert into ", StringComparison.InvariantCultureIgnoreCase)
                    || sql.StartsWith("select ", StringComparison.InvariantCultureIgnoreCase)
                    || sql.StartsWith("delete ", StringComparison.InvariantCultureIgnoreCase)
                    || sql.StartsWith("merge ", StringComparison.InvariantCultureIgnoreCase))
                {
                    commands[index] += OPTION_TEXT;
                }
            }

#pragma warning disable CA2100 // Review SQL queries for security vulnerabilities
            command.CommandText = string.Join(";\r\n", commands);
#pragma warning restore CA2100 // Review SQL queries for security vulnerabilities
        }
    }
like image 31
Peter Morris Avatar answered Oct 11 '22 15:10

Peter Morris