4

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;
  • 2
    Post the execute plan to brentozar.com/pastetheplan and the DDL for idx_report_foobar. – Dan Guzman Mar 9 '18 at 16:20
  • Which execution plan are you looking for? – Marcus Mar 9 '18 at 16:42
  • The execution plan of the INSERT statement. – Dan Guzman Mar 9 '18 at 17:38
  • I hazard a guess that there's a problem with your scalar valued function. – Randolph West Mar 10 '18 at 6:55
  • @RandolphWest The scalar valued function is visible in the deadlock XML - it's the mydb.dbo.CheckForDoubleBookings – Marcus Mar 12 '18 at 11:18
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.

  • Thank you. Hope to come back shortly with more information, though this is very helpful. Under option 3, wouldn't a UPDLOCK hint be required on the IF NOT EXISTS SELECT statement? Otherwise deadlocks are just as likely to happen. – Marcus Mar 14 '18 at 12:33
  • Option 2 wouldn't work for us as the check constraint is to ensure no overlapping date ranges. This option would only be successful in stopping date range overlaps where the start and end dates were the same. – Marcus Mar 14 '18 at 14:08
  • I'm not sure I understand the last sentence of option 4. Could you please clarify? This would prevent more than one lock at a time for that combination, but that combination already existing for past dates (that are not within the current range) would not be an issue.. – Marcus Mar 14 '18 at 14:10
  • @Marcus I do not believe that an UPDLOCK is needed as there is either nothing no row there to lock or the row exists and the INSERT never happens. Of course, it would really help to provide more certainty there if you made it clear whether this INSERT was in an explicit transaction or on its own (i.e an auto-commit transaction). It would also greatly help if you mentioned in the question which combination of the 5 columns determined uniqueness. Is eventID unique by itself, in combination with userID, etc? Option 2 would work IF eventID or userID, eventID were unique. – Solomon Rutzky Mar 14 '18 at 14:16
  • 1
    I think that you might be able to achieve the same effect by having a UserLastBooking table with PK on [User] that has to be updated before the booking is Inserted. Under RCSI, conflicting simultaneous updates should fail at COMMIT with a concurrency error (I think). – RBarryYoung Mar 15 '18 at 14:47
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.

  • Thanks. Should the answer say "...the pseudo code should:" rather than "...the pseudo code should not:"? – Marcus Mar 14 '18 at 17:11
  • @Marcus - Hopefully I clarified that "the pseudo code should not change" when going from one RDBMS to another. – Michael Kutz Mar 14 '18 at 18:04
  • @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. – Solomon Rutzky Mar 14 '18 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 '18 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 '18 at 0:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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