Sql-server – Merge statement deadlocking itself

deadlockmergesql serversql-server-2008-r2

I have the following procedure (SQL Server 2008 R2):

create procedure usp_SaveCompanyUserData
    @companyId bigint,
    @userId bigint,
    @dataTable tt_CoUserdata readonly
as
begin

    set nocount, xact_abort on;

    merge CompanyUser with (holdlock) as r
    using (
        select 
            @companyId as CompanyId, 
            @userId as UserId, 
            MyKey, 
            MyValue
        from @dataTable) as newData
    on r.CompanyId = newData.CompanyId
        and r.UserId = newData.UserId
        and r.MyKey = newData.MyKey
    when not matched then
        insert (CompanyId, UserId, MyKey, MyValue) values
        (@companyId, @userId, newData.MyKey, newData.MyValue);

end;

CompanyId, UserId, MyKey form the composite key for the target table. CompanyId is a foreign key to a parent table. Also, there is a non-clustered index on CompanyId asc, UserId asc.

It is called from many different threads, and I am consistently getting deadlocks between different processes calling this same statement. My understanding was that the "with (holdlock)" was necessary to prevent insert/update race condition errors.

I assume that two different threads are locking rows (or pages) in different orders when they are validating the constraints, and thus are deadlocking.

Is this a correct assumption?

What is the best way to resolve this situation (i.e. no deadlocks, minimum impact on multi-threaded performance)?

Query Plan Image
(If you view the image in a new tab, it is readable. Sorry for the small size.)

There are at most 28 rows are in the @datatable.
I have traced back through the code, and I cannot see anywhere that we start a transaction here.
The foreign key is set up to cascade only on delete, and there were no deletions from the parent table.

Best Answer

OK, after looking everything over a couple of times, I think that your basic assumption was correct. What's probably going on here is that:

The MATCH part of the MERGE checks the index for matches, read-locking those rows/pages as it goes.
When it has a row without a match, it will try to insert the new Index Row first so it will request a row/page write-lock ...

But if another user has also gotten to step 1 on the same row/page, then the first user will be blocked from the Update, and ...

If the second user also needs to insert on the same page, then they're in a deadlock.

AFAIK, there's only one (simple) way to be 100% sure that you cannot get a deadlock with this procedure and that would be to add a TABLOCKX hint to the MERGE, but that would probably have a really bad impact on performance.

It is possible that adding a TABLOCK hint instead would be enough to solve the problem without having to big an effect on your performance.

Finally, you could also try adding PAGLOCK, XLOCK or both PAGLOCK and XLOCK. Again that might work and performance might not be too awful. You'll have to try it to see.

Related Solutions

Sql-server – Can foreign keys cause deadlocks and hinder READ COMMITTED SNAPSHOT

If the SQLCAT team says that FK validation is done using read-committed isolation, then they must know what they're talking about. Emphasis on validation. The real question is Why would a report trigger FK validation? Validation occurs on writes, and reports are supposed to be reads. Either your reports are causing writes, in which case snapshot isolation levels will help nothing, either the cause of the deadlock is different.

The only way to make progress is to capture the deadlock graph.

As for the other question, how can you check if you operate under snapshot isolation: look in sys.dm_tran_active_snapshot_database_transactions.

Sql-server – Why are these two INSERTs deadlocking? Is it the trigger? What does this deadlock trace 1222 log tell me

I didn't get a chance to rerun the trace and capture the more nicely formatted XDL file, but in the meantime I was able to resolve the issue by removing all the isolation level and locking hints from the trigger and modifying the application code to run the INSERT itself from within a serializable transaction.

The app uses LINQ to SQL, so the INSERT statement is dynamically generated and run under the default READ COMMITTED isolation level. From the deadlock trace, I could tell that the locks taken by the INSERT were conflicting with the locks taken by the trigger. The deadlock was happening even after removing all the isolation and locking hints from the trigger, so I surmised that the trigger's UPDATE statements were causing stronger locks than the INSERT statement. And when the competing simultaneous transaction tried to do the same thing, the deadlock occurred.

We were definitely on the right track with our previous attempted solutions: using higher isolation levels and stronger locking in the trigger. The problem is that the weaker locks were already taken by the INSERT statement generated by the LINQ to SQL, before the trigger ever got called. The solution is to explicitly start a transaction in our .NET code using SERALIZABLE isolation level, do the insert in that transaction, let the trigger happen in the same transaction (without altering the isolation level or lock hints) and then commit and dispose the transaction. When I did that, I was no longer able to reproduce the issue (I even put it back to the old way and verified that the issue returned.) Here is the new code:

            ' Do the insert in a serializable transaction to prevent simultaneous inserts from deadlocking
            ' due to the trigger, which reaches out to other rows.
            Me.packageContext.Connection.Open()
            Try
                Using oTransaction = Me.packageContext.Connection.BeginTransaction(IsolationLevel.Serializable)
                    Me.packageContext.Transaction = oTransaction
                    Try
                        Me.packageContext.SubmitChanges()
                        oTransaction.Commit()
                    Catch ex As Exception
                        oTransaction.Rollback()
                        Throw
                    End Try
                End Using
            Finally
                ' We need to close the connection and null out the transaction to allow
                ' subsequent uses of the same DataContext to work properly outside this transaction.
                Me.packageContext.Connection.Close()
                Me.packageContext.Transaction = Nothing
            End Try

Normally we avoid opening transactions from within client side code, because they can run long if the code does remote API calls, causing excessive blocking. But in this case we are isolating a very small bit of code, and we have robust error/rollback handling around it.

An alternate solution would be to implement stored procedures that do the INSERTs within explicit SERIALIZABLE transactions, and then alter the DataContext to call the stored procedure. However, that solution is less maintainable as it requires updating both the stored procedure and the DataContext definition every time the schema changes.

Finally, another potential alternate solution would be to examine the deadlock log in detail and find a way to adjust our indexes or keys to avoid the deadlock in the first place. However, that is more difficult and may not be possible with our design.

I think the solution above is the simplest and most understandable for future developers.

Best Answer

Related Solutions

Sql-server – Can foreign keys cause deadlocks and hinder READ COMMITTED SNAPSHOT

Sql-server – Why are these two INSERTs deadlocking? Is it the trigger? What does this deadlock trace 1222 log tell me

Related Question