5

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

Dedlock graph

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;
1
  • Added the SQL of the scalar-valued function which is called by the CHECK CONSTRAINT which checks the function == 1.
    – Marcus
    Mar 14, 2018 at 15:49

2 Answers 2

5
+50

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:

  1. Insert a row
  2. CHECK CONSTRAINT fires: Within the explicit / implicit or auto-commit transaction that is the INSERT statement, verify the condition via the scalar UDF.
  3. If the UDF returns a 1, implicitly issue a ROLLBACK and exception.
  4. 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:

  1. 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.

  2. 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.

  3. 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;
    
  4. 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):

  5. 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.

1
1

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:

  1. Acquire a serialization lock

  2. Assert that the changed data is still valid

  3. 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.

5
  • @Marcus and Michael, if I am understanding this answer correctly, it is the same thing as my Option #4 (and the guts of Option #5). The "app lock" is the "named mutex" / serialization lock that does not take any row locks, etc. UPDLOCK does take row locks, but this is not an UPSERT pattern so most of the time no row will exist. In the chat room, Marcus noted that HOLDLOCK on the SELECT takes range locks on the table for the same filter conditions (to reserve for future insert). This is why my preference is the "app lock" as it does not cause contention on the table. Mar 14, 2018 at 21:12
  • @MichaelKutz & @Solomon, can we guarantee that taking the serialization lock will ensure SQL server queues up requests one by one? There are many other questions on here which suggest a pattern like IF NOT EXISTS(SELECT ...) BEGIN INSERT INTO ... however we've seen above this is not thread-safe.
    – Marcus
    Mar 14, 2018 at 23:51
  • Reading how you'd solve this in Oracle has led to another possible solution as identified here: jeffkemponoracle.com/2012/08/non-overlapping-dates-constraint Using a Materialised View with a CHECK CONSTRAINT. Equivalent for SQL Server would be an Indexed View with a Unique Constraint: spaghettidba.com/2011/08/03/…
    – Marcus
    Mar 15, 2018 at 0:05
  • @Marcus and Michael: yes, the app lock (i.e. serialization lock in SQL Server) guarantees a queued, single-threaded approach. That is what the app lock does, based on the arbitrary resource (i.e. the string that you construct). If you concatenate a literal and the userID into the resource, then that string value is what gets locked, not the Table. Another process attempting to lock that same string will wait. Another process attempting to lock another string (that no other process has locked) will get its lock and proceed. I've updated my answer with links to answers of mine with more info. Mar 15, 2018 at 2:16
  • @Marcus - The Materialized View trick is almost always mentioned within Mutating Table threads. Jeff's MV implementation inherently serialize a full rebuild of the MV at COMMIT. This implies serialization at the same level of granularity as hard-coding a static string for app_lock. By using a finer grain lock (eg userID for app_lock), you are allowing the database to modify events by different users simultaneously. I happen to like the Procedure method because most of the code can be generated using something like Jeff's jk64. INSTEAD OF trigger would be a wrapper for them. Mar 15, 2018 at 21:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.