SQL deadlock on nonclustered key caused by two INSERTs and a CHECK CONSTRAINT on the same table

Question

Been struggling with deadlocking on a table during INSERTs. It's a multi-tenant database and Read Committed Snapshot Isolation (RCSI) is enabled.

There is a CHECK CONSTRAINT upon INSERT to ensure there can be no overlapping bookings (by smalldatetime regardless of event) which executes a Scalar Valued Function and checks for a result of 1. This constraint looks up the same table with a READCOMMITTEDLOCK hint to check for violations of the logic where the ID (PK/clustered index) doesn't equal the ID of the newly inserted row.

The READCOMMITTEDLOCK hint was used due to RCSI being enabled and wanting to ensure we don't skip rows, which could lead to overlapping bookings.

The constraint does an INDEX SEEK on the index causing the deadlock: idx_report_foobar.

Any assistance would be greatly appreciated.

Here is the XML (which has been adjusted to remove some of the logic and names of table fields which are in the database):

<deadlock>
 <victim-list>
  <victimProcess id="process91591c108" />
 </victim-list>
 <process-list>
  <process id="process91591c108" taskpriority="0" logused="1328" waitresource="KEY: 9:72057594095861760 (c2e966d5eb6a)" waittime="3046" ownerId="2628292921" transactionname="user_transaction" lasttranstarted="2018-03-09T14:24:13.820" XDES="0x708a80d80" lockMode="S" schedulerid="10" kpid="8964" status="suspended" spid="119" sbid="2" ecid="0" priority="0" trancount="2" lastbatchstarted="2018-03-09T14:24:13.823" lastbatchcompleted="2018-03-09T14:24:13.820" lastattention="1900-01-01T00:00:00.820" clientapp=".Net SqlClient Data Provider" hostname="SERVERNAMEHERE" hostpid="33672" loginname="DOMAIN\USERHERE" isolationlevel="read committed (2)" xactid="2628292921" currentdb="9" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
   <executionStack>
    <frame procname="mydb.dbo.CheckForDoubleBookings" line="12" stmtstart="920" stmtend="3200" sqlhandle="0x0300090018ef9b72531bea009ea8000000000000000000000000000000000000000000000000000000000000">
IF EXISTS (SELECT * 
                 FROM   dbo.bookings a WITH (READCOMMITTEDLOCK)
                 WHERE  a.id &lt;&gt; @id 
                        AND a.userID = @userID 
                        AND @bookingStart &lt; a.bookingEnd 
                        AND a.bookingStart &lt; @bookingEnd
                        AND a.eventID = @eventID
    </frame>
    <frame procname="adhoc" line="1" stmtstart="288" stmtend="922" sqlhandle="0x020000005ed9af11c02db2af69df1d5fb6d1adb0e4812afb0000000000000000000000000000000000000000">
unknown    </frame>
    <frame procname="unknown" line="1" sqlhandle="0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000">
unknown    </frame>
   </executionStack>
   <inputbuf>
(@0 datetime2(7),@1 datetime2(7),@2 int,@3 int,@4 int,@5 int,@6 int,@7 nvarchar(4000),@8 datetime2(7),@9 nvarchar(50),@10 int,@11 nvarchar(255))INSERT [dbo].[bookings]([bookingStart], [bookingEnd], [userID], [eventID], [TypeId], [Notes], [Timestamp], [AddedById])
VALUES (@0, @1, @2, @3, @4, @5, @6, @7, @8, NULL, @9, @10, @11, NULL, NULL)
SELECT [Id]
FROM [dbo].[bookings]
WHERE @@ROWCOUNT &gt; 0 AND [Id] = scope_identity()   </inputbuf>
  </process>
  <process id="processca27768c8" taskpriority="0" logused="1328" waitresource="KEY: 9:72057594095861760 (3ba50d420e66)" waittime="3048" ownerId="2628280537" transactionname="user_transaction" lasttranstarted="2018-03-09T14:24:04.063" XDES="0xa555403b0" lockMode="S" schedulerid="6" kpid="12776" status="suspended" spid="124" sbid="2" ecid="0" priority="0" trancount="2" lastbatchstarted="2018-03-09T14:24:04.070" lastbatchcompleted="2018-03-09T14:24:04.063" lastattention="1900-01-01T00:00:00.063" clientapp=".Net SqlClient Data Provider" hostname="SERVERNAMEHERE" hostpid="33672" loginname="DOMAIN\USERHERE" isolationlevel="read committed (2)" xactid="2628280537" currentdb="9" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
   <executionStack>
    <frame procname="mydb.dbo.CheckForDoubleBookings" line="12" stmtstart="920" stmtend="3200" sqlhandle="0x0300090018ef9b72531bea009ea8000000000000000000000000000000000000000000000000000000000000">
IF EXISTS (SELECT * 
                 FROM   dbo.bookings a WITH (READCOMMITTEDLOCK)
                 WHERE  a.id &lt;&gt; @id 
                        AND a.userID = @userID 
                        AND @bookingStart &lt; a.bookingEnd 
                        AND a.bookingStart &lt; @bookingEnd
                        AND a.eventID = @eventID
    </frame>
    <frame procname="adhoc" line="1" stmtstart="288" stmtend="922" sqlhandle="0x020000005ed9af11c02db2af69df1d5fb6d1adb0e4812afb0000000000000000000000000000000000000000">
unknown    </frame>
    <frame procname="unknown" line="1" sqlhandle="0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000">
unknown    </frame>
   </executionStack>
   <inputbuf>
(@0 datetime2(7),@1 datetime2(7),@2 int,@3 int,@4 int,@5 int,@6 int,@7 nvarchar(4000),@8 datetime2(7),@9 nvarchar(50),@10 int,@11 nvarchar(255))INSERT [dbo].[bookings]([bookingStart], [bookingEnd], [userID], [eventID], [TypeId], [Notes], [Timestamp], [AddedById])
VALUES (@0, @1, @2, @3, @4, @5, @6, @7, @8, NULL, @9, @10, @11, NULL, NULL)
SELECT [Id]
FROM [dbo].[bookings]
WHERE @@ROWCOUNT &gt; 0 AND [Id] = scope_identity()   </inputbuf>
  </process>
 </process-list>
 <resource-list>
  <keylock hobtid="72057594095861760" dbid="9" objectname="mydb.dbo.bookings" indexname="idx_report_foobar" id="locke83fdbe80" mode="X" associatedObjectId="72057594095861760">
   <owner-list>
    <owner id="processca27768c8" mode="X" />
   </owner-list>
   <waiter-list>
    <waiter id="process91591c108" mode="S" requestType="wait" />
   </waiter-list>
  </keylock>
  <keylock hobtid="72057594095861760" dbid="9" objectname="mydb.dbo.bookings" indexname="idx_report_foobar" id="lock7fdb48480" mode="X" associatedObjectId="72057594095861760">
   <owner-list>
    <owner id="process91591c108" mode="X" />
   </owner-list>
   <waiter-list>
    <waiter id="processca27768c8" mode="S" requestType="wait" />
   </waiter-list>
  </keylock>
 </resource-list>
</deadlock>

The index:

CREATE NONCLUSTERED INDEX [idx_report_foobar] ON [dbo].[bookings]
(
    [eventID] ASC
)
INCLUDE (   [bookingStart],
    [bookingEnd],
    [userID]) WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, SORT_IN_TEMPDB = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON, FILLFACTOR = 80)
GO

The scalar function is used to ensure a user can have no double bookings (datetime's don't overlap, regardless of eventID):

BEGIN
  DECLARE @Valid bit = 1;
  IF EXISTS (SELECT *
    FROM dbo.bookings a WITH (READCOMMITTEDLOCK)
    WHERE a.id <> @id
    AND a.userID = @userID
    AND @bookingStart < a.bookingEnd
    AND a.bookingStart < @bookingEnd
    AND a.eventID = @eventID)
    SET @Valid = 0;
  RETURN @Valid;
END;

Solomon Rutzky · Accepted Answer · 2018-03-15T02:02:39.150

It would be easier to narrow this down if the parameter values being passed into the dbo.CheckForDoubleBookings Scalar UDF were available, but I think there is enough info here to offer an educated guess:

You have two different rows being fought over. The process itself does the following:

Insert a row
CHECK CONSTRAINT fires: Within the explicit / implicit or auto-commit transaction that is the INSERT statement, verify the condition via the scalar UDF.
If the UDF returns a 1, implicitly issue a ROLLBACK and exception.
COMMIT

So, in two different sessions, at the same time, and for the same userID, you have Step 1 (above) occur, which inserts a row and maintains an eXclusive lock on that row. Then, in each session, prior to completing the INSERT, Step 2 occurs which processes the Check Constraint that checks for potentially matching rows (same four properties but with a different id). You are only reading "committed" data, but the new rows have not committed yet since each one is waiting upon verification of their respective Check Constraint.

The problem here is that the verification process is happening at the wrong point in time / sequence in the workflow. Because it is being handled via a CHECK CONSTRAINT, the row has already been added (just not Committed) which is how it gets an id assigned to pass into the Scalar UDF. And then the process doesn't want to Commit until it is certain that no matching entry exists, yet it can't see the other entry since that one also has not yet committed for the same reason.

Here are some options:

The simplest, yet certainly non-ideal option, is to switch to reading Uncommited / dirty data. For this you would switch to using the NOLOCK hint instead of the current READCOMMITTEDLOCK. The problem here is that while it would work most of the time, you could have a scenario where either:
1. both sessions see the other and decide to abort, or
2. one session sees the other, decides to abort, but then the first session has it's INSERT aborted for a completely different reason.
In both cases, neither entry gets saved.
Try removing the CHECK CONSTRAINT and adding the IGNORE_DUP_KEY = ON option to that non-Clustered Index. This would cause the situation where if both happen at the same time, one would commit and the other would silently fail (well, you would get a warning) and not insert anything. That should be fine since you check @@ROWCOUNT afterwards.

The requirement is that the index needs to be UNIQUE. If eventID by itself is unique, then just add the UNIQUE keyword to the index definition. If eventID by itself is not unique, then move what are currently the INCLUDE columns into being key columns until you can establish uniqueness.

Handle this logic outside of the INSERT statement. You can test ahead of time for the existence of a matching row, and if nothing is found then do the INSERT. And, since sometimes that "check" will fail if it executes at the exact same moment as another INSERT of those same values, then one will fail. So, that is handled by wrapping the INSERT in a TRY...CATCH construct and ignoring the error if it occurs:

DECLARE @NewID INT;
BEGIN TRY
  IF (NOT EXISTS(SELECT * FROM dbo.Table WHERE columns = @parameters))
  BEGIN
    INSERT INTO dbo.Table (columns) VALUES (@parameters);

    SET @NewID = SCOPE_IDENTITY();
  END;
END TRY
BEGIN CATCH
  -- Either return custom error, or handle in a different way, such as
  -- selecting and returning the `id` matching the same criteria that
  -- was the basis of the failed `INSERT`, such as:
  SELECT @NewID = [id]
  FROM   dbo.Table
  WHERE  columns = @parameters
END CATCH;

If you need to allow for variations in the "start" and "end" dates for the @userID, @eventID combination and the @userID, @eventID combination is not unique (hence option #2 above won't work), then you might need to consider using an app_lock where the "resource" is a custom string comprised of either the @userID, @eventID combination or maybe just userID. This would prevent more than one lock at a time for that combination. But, that combination already existing in the Table for past dates prior to the currently requested dates would not be an issue because an app_lock only locks the "resource", which is a specific string, and has nothing to do with the data in the table (in fact, the 'app lock" wouldn't even know of the Table).

The idea here is to first create the "app lock" to force single-threading for just this userID so that the process can a) check to see if overlapping dates exist, and b) insert if no overlapping dates are found. Other sessions for the same userID will be blocked until the "app lock" is released, at which point they (one at a time per userID) will check to see if overlapping dates exist.

For more info on app locks, please see the following two answers of mine, also here on DBA.SE (both of which have links to the "app lock" documentation):
- Resolving deadlock from 2 tables only related through indexed view
- Strategies for “checking out” records for processing
Now that it has been clarified (in comments on this answer) that:

Users cannot have bookings which overlap.

Bookings are tied to an eventID, and multiple events with different eventID's can happen on the same dates.

it is clearer that Options 1, 2 and 3 will not work at all, even if the userID, eventID combination is unique, since it is only userID that is truly being checked here for overlapping dates. That leaves Option #4, which will work if the "resource" is simply the userID. But another issue was brought up:

The INSERT is generated in C# code by Entity Framework, so it's on it's own.
...
could a solution be to use a INSTEAD OF INSERT trigger rather than a CHECK CONSTRAINT? This would ensure even manual changes to the database table have to abide by the validation.

True, moving the logic to a Stored Procedure such that it can encapsulate this logic (either Option 3 or 4) would only work if all codes paths (and ad hoc updates) used the Stored Procedure. An INSTEAD OF INSERT Trigger would handle events initiated by unscrupulous support staff and/or developers who refuse to use the Stored Procedure due to philosophical and/or moral disagreements with following the rules, but it can't use the existing "check" logic. Such a Trigger would get around the "row already exists so it blocks the other Session" issue, but then it ends up with the same issue as Option 3 where both Sessions can check, find no matching rows, and proceed to insert overlapping dates.

But, using an "app lock" as the logic in the INSTEAD OF INSERT Trigger should work. And, since the Trigger already exists within a Transaction, that part does not need to be handled manually. The process / Trigger would look something like:
```
-- Either prevent multi-row inserts OR remove this "IF" block and wrap the rest
-- of the logic in a cursor that will process 1 row at a time from "inserted".
IF ((SELECT COUNT(*) FROM inserted) > 1)
BEGIN
  ROLLBACK;
  RAISERROR(N'Slow your roll, yo! One event at a time, ya dig?', 16, 1);
END;

DECLARE @Resource NVARCHAR(150);
SET @Resource = N'New Booking for: ' + CONVERT(NVARCHAR(20), @userID);

EXEC sys.sp_getapplock @Resource, other options;

IF (NOT EXISTS (SELECT *
  FROM dbo.bookings a WITH (READCOMMITTEDLOCK)
  WHERE a.id <> @id
  AND a.userID = @userID
  AND @bookingStart < a.bookingEnd
  AND a.bookingStart < @bookingEnd
  AND a.eventID = @eventID))
BEGIN
  INSERT INTO dbo.bookings (columns) VALUES (@values);
END;

EXEC sys.sp_releaseapplock @Resource;
```
Doing the "app lock" this way will not cause contention on INSERTs for other userIDs.

Michael Kutz · Answer 2 · 2018-03-14T18:00:39.883

In Oracle, your logic would cause a Mutating Table error. The solution there is probably similar.

Solution: Serialize all DML activities to the target table.

The details of the recommendation would probably change between RDBMS but the pseudo code should not change.

Pseudo code:

Acquire a serialization lock
Assert that the changed data is still valid
Do the DML

Enforcing Serialization

Applications should not be allowed to do direct DML activities against the table. Instead, they must use a set of Procedures.

By usage Procedures, you gain the ability for code and logic reuse. This becomes important as more and more applications, written in different languages, are created.

Implementing "Acquire Lock"

Most of the solutions to Mutating Table within Oracle involve row locks via select ... for update. Your comment to @Solomon's answer seems to indicate that MS-SQL uses UPDLOCK to achieve the same thing.

If possible, you want to minimize the number of row-locks you obtain. In some situations, you can achieve this by row-lock the Parent row referenced by a Foreign Key. For your situation, you would probably want to grab a row-lock, on a different table, for each day/month, of an event.

Other methods of serialization involve using a named mutex, DBMS_LOCK for Oracle, or locking the entire table.

You'll have to adjust the actual code for MS-SQL.

SQL deadlock on nonclustered key caused by two INSERTs and a CHECK CONSTRAINT on the same table

2 Answers2

Enforcing Serialization

Implementing "Acquire Lock"