Multiple deadlocks occurring with contentious code

  • We are seeing multiple frequent deadlocks occurring with the following extract of code, which forms part of a stored procedure called sp_WS_Save_Record_Details. We have a web service that calls multiple instances of this stored procedure simultaneously.  Records in the Record_Stub table are identified by an external reference.

    The code is trying to ensure that if multiple calls are made with the same external reference, only one call can evaluate and process a given record at a time. However, this is causing concurrency issues when the volumes reach a given threshold, and we are seeing pileups of deadlocks occurring at peak times.

    SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
    BEGIN TRANSACTION


    SET @OriginallyCreated = 
    (
    SELECT MAX(CreatedOn)
    FROM dbo.Record_Stub
    WHERE ExternalReference = LTRIM(RTRIM(@ExternalReference))
    AND CreatedBy = @UserID
    AND RegionID = @RegionID
    );


    IF @OriginallyCreated IS NOT NULL
    BEGIN
    IF DATEDIFF(s, @OriginallyCreated, GETDATE()) < 90
    SELECT @FailureMessage = 'The record for external reference ' + RTRIM(@ExternalReference) + ' is still being processed.  Please try again later.';
    ELSE
    DELETE dbo.Record_Stub
    WHERE ExternalReference = LTRIM(RTRIM(@ExternalReference))
    AND CreatedBy = @UserID
    AND RegionID = @RegionID;
    END


    IF (ISNULL(@FailureMessage, '') <> '')
    BEGIN
    ROLLBACK TRANSACTION;
    RETURN 100;
    END


    -- It's a new submission so let's create a stub for it.
    INSERT dbo.Record_Stub (ExternalReference, RegionID, CreatedBy, CreatedOn)
    VALUES (@ExternalReference, @RegionID, @UserID, GETDATE());


    SELECT @RecordID = SCOPE_IDENTITY();
    SELECT @RecordStatusIndicator = 'I'


    COMMIT TRANSACTION;
    SET TRANSACTION ISOLATION LEVEL READ COMMITTED

     

    The Record_Stub table is kept to a manageable size of around 400,000 records, by a nightly job which deletes records older than 7 days. The table structure and indexing is given below. There are no triggers on this table.

    CREATE TABLE dbo.Record_Stub (
    RecordID int IDENTITY(1,1) NOT NULL,
    ExternalReference nchar(25) NULL,
    RegionID int NULL,
    CreatedBy int NOT NULL,
    CreatedOn datetime NOT NULL,
    CONSTRAINT PK_Record_Stub PRIMARY KEY CLUSTERED (RecordID) WITH (PAD_INDEX = OFF, FILLFACTOR = 90)
    );
    GO
    CREATE NONCLUSTERED INDEX idx_ExternalReference ON dbo.Record_Stub (ExternalReference) WITH (PAD_INDEX = OFF, FILLFACTOR = 70);
    GO

     

    The following is a typical example of the deadlock report XML we are seeing.

    xml_report <deadlock>
    <victim-list>
    <victimProcess id="process35a951b84e8"/>
    </victim-list>
    <process-list>
    <process id="process35a951b84e8" taskpriority="0" logused="336" waitresource="KEY: 30:72057596372844544 (1c3c61e1f34f)" waittime="2903" ownerId="133046337637" transactionname="user_transaction" lasttranstarted="2024-05-17T11:09:15.973" XDES="0x2e15b084bd0" lockMode="RangeI-N" schedulerid="5" kpid="11172" status="suspended" spid="213" sbid="0" ecid="0" priority="0" trancount="2" lastbatchstarted="2024-05-17T11:09:15.830" lastbatchcompleted="2024-05-17T11:09:15.827" lastattention="1900-01-01T00:00:00.827" clientapp=".Net SqlClient Data Provider" hostname="********" hostpid="7604" loginname="********" isolationlevel="serializable (4)" xactid="133046337637" currentdb="30" currentdbname="MyDatabase" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
    <executionStack>
    <frame procname="MyDatabase.dbo.sp_WS_Save_Record_Details" line="634" stmtstart="70856" stmtend="71150" sqlhandle="0x03001e00e7ee425b28e85501d4b0000001000000000000000000000000000000000000000000000000000000">  INSERT dbo.Record_Stub (ExternalReference, RegionID, CreatedBy, CreatedOn)         VALUES (@ExternalReference, @RegionID, @UserID, GETDATE()    </frame>
    </executionStack>
    <inputbuf>  Proc [Database Id = 30 Object Id = 1531113191]   </inputbuf>
    </process>
    <process id="process356e015bc28" taskpriority="0" logused="336" waitresource="KEY: 30:72057596372844544 (1c3c61e1f34f)" waittime="2914" ownerId="133046337650" transactionname="user_transaction" lasttranstarted="2024-05-17T11:09:15.973" XDES="0x30777353850" lockMode="RangeI-N" schedulerid="4" kpid="5616" status="suspended" spid="208" sbid="0" ecid="0" priority="0" trancount="2" lastbatchstarted="2024-05-17T11:09:15.830" lastbatchcompleted="2024-05-17T11:09:15.827" lastattention="1900-01-01T00:00:00.827" clientapp=".Net SqlClient Data Provider" hostname="********" hostpid="7604" loginname="********" isolationlevel="serializable (4)" xactid="133046337650" currentdb="30" currentdbname="MyDatabase" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
    <executionStack>
    <frame procname="MyDatabase.dbo.sp_WS_Save_Record_Details" line="634" stmtstart="70856" stmtend="71150" sqlhandle="0x03001e00e7ee425b28e85501d4b0000001000000000000000000000000000000000000000000000000000000">  INSERT dbo.Record_Stub (ExternalReference, RegionID, CreatedBy, CreatedOn)         VALUES (@ExternalReference, @RegionID, @UserID, GETDATE()    </frame>
    </executionStack>
    <inputbuf>  Proc [Database Id = 30 Object Id = 1531113191]   </inputbuf>
    </process>
    </process-list>
    <resource-list>
    <keylock hobtid="72057596372844544" dbid="30" objectname="MyDatabase.dbo.Record_Stub" indexname="idx_ExternalReference" id="lock1ee9ae60100" mode="RangeS-S" associatedObjectId="72057596372844544">
    <owner-list>
    <owner id="process356e015bc28" mode="RangeS-S"/>
    <owner id="process356e015bc28" mode="RangeI-N" requestType="convert"/>
    </owner-list>
    <waiter-list>
    <waiter id="process35a951b84e8" mode="RangeI-N" requestType="convert"/>
    </waiter-list>
    </keylock>
    <keylock hobtid="72057596372844544" dbid="30" objectname="MyDatabase.dbo.Record_Stub" indexname="idx_ExternalReference" id="lock1ee9ae60100" mode="RangeS-S" associatedObjectId="72057596372844544">
    <owner-list>
    <owner id="process35a951b84e8" mode="RangeS-S"/>
    <owner id="process35a951b84e8" mode="RangeI-N" requestType="convert"/>
    </owner-list>
    <waiter-list>
    <waiter id="process356e015bc28" mode="RangeI-N" requestType="convert"/>
    </waiter-list>
    </keylock>
    </resource-list>
    </deadlock>

    We understand that the SERIALIZABLE transaction isolation level is the least concurrent of the isolation levels, but we need some way of ensuring that only one instance of the stored procedure can evaluate and process a given external reference at a time.

    Can anyone please suggest a better way of achieving this, whilst improving concurrency?

    • This topic was modified 7 months ago by  zoggling.
  • Imo the issue with assigning value(s) to variable(s) and then using variable(s) to resolve flow of control is it can create race conditions.  Then database serialization is attempted to hold values constant between statement execution within procedures.  Altho a lot could be written and code patterns could be considered from many different angles, imo, it comes down to "JFDI" or "jiff-dee".  Just Freakin' Do It.  What does "It" refer to?  Basically, re-write your code to speculatively execute the the most likely successful outcome.  If the most likely outcome doesn't happen, then speculatively execute the next statement...

    For the code posted we don't have sample data so I'm not sure if the code below will run as intended.  The intention is to replicate the functionality of the posted code with non-blocking (or less blocking).  Also, instead of testing for the error message existence it could be replaced with THROW.  As with everything it needs testing and adjustment as needed

    INSERT dbo.Record_Stub (ExternalReference, RegionID, CreatedBy, CreatedOn)
    select @ExternalReference, @RegionID, @UserID, GETDATE()
    where not exists (select 1
    from dbo.Record_Stub
    WHERE ExternalReference = LTRIM(RTRIM(@ExternalReference))
    AND CreatedBy = @UserID
    AND RegionID = @RegionID);
    if @@rowcount=0; /* row exists */
    delete rs
    from dbo.Record_Stub rs
    where ExternalReference = LTRIM(RTRIM(@ExternalReference))
    AND CreatedBy = @UserID
    AND RegionID = @RegionID
    and not exists (select 1
    from dbo.Record_Stub in_rs
    WHERE in_rs.ExternalReference=rs.ExternalReference
    AND in_rs.CreatedBy = rs.CreatedBy
    AND in_rs.RegionID = rs.RegionID
    and datediff(second, in_rs.CreatedOn, getdate()) < 90);
    if @@rowcount=0; /* row exists and date is less then 90 */
    begin
    declare @error_msg nvarchar(2044)=formatmessage(50001, concat(N'The record for external reference ', rtrim(@ExternalReference), N' is still being processed. Please try again later.'));
    throw 50001, @error_msg, 1;
    end

    Aus dem Paradies, das Cantor uns geschaffen, soll uns niemand vertreiben können

  • I don't see a unique index on the columns ExternalReference, RegionID and CreatedBy.  That means you can have multiple rows with those 3 columns - including multiple rows with the same CreatedOn as well as different dates.

    If you created a unique constraint on those 3 columns - then you can only have a single row per combination.  Any attempt to insert a new row will fail due to the constraint.

    If you had that - then using a TRY/CATCH would simplify the process.  Delete any rows older than 90 seconds - insert new row - if insert fails (row exists) then catch the error, rollback transaction and exit with defined failure message.

    For this - you don't need an explicit transaction nor do you need to serialize the process (unless you have other code not shown that is dependent on a serialized process).  If the delete finds a row to be deleted - then the INSERT will create a new row - if the delete does not find a row to be deleted - then if a row exists the insert will fail, if a row does not exist then the insert will succeed.

    DECLARE @error_msg nvarchar(2044) = formatmessage(50001, concat(N'The record for external reference ', rtrim(@ExternalReference), N' is still being processed.  Please try again later.'));

    SET @ExternalReference = TRIM(@ExternalReference);

    BEGIN TRY

    --==== Delete the row if more than 90 seconds old
    DELETE rs
    FROM dbo.Record_Stub rs
    WHERE rs.ExternalReference = @ExternalReference
    AND rs.CreatedBy = @UserID
    AND rs.RegionID = @RegionID
    AND rs.CreatedOn < dateadd(second, -90, getdate());

    --==== Attempt to insert a new row
    INSERT dbo.Record_Stub (ExternalReference, RegionID, CreatedBy, CreatedOn)
    VALUES (@ExternalReference, @RegionID, @UserID, GETDATE());

    END TRY
    BEGIN CATCH

    --==== Throw the error back to the client
    THROW 50001, @error_msg, 1;

    END CATCH

    Jeffrey Williams
    “We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”

    ― Charles R. Swindoll

    How to post questions to get better answers faster
    Managing Transaction Logs

  • IF the most critical lookup on the table is the one you've posted, and since you're deleting rows frequently, I'd suggest you consider re-clustering the table to allow clus index seek (and scan from there if needed) rather than SQL having to scan the entire table, which can lead to deadlocks.

    ALTER TABLE dbo.Record_Stub DROP CONSTRAINT PK_Record_Stub;

    CREATE UNIQUE CLUSTERED INDEX CL_Record_Stub ON dbo.Record_Stub ( ExternalReference, CreatedOn, RecordID ) WITH ( FILLFACTOR = 85, SORT_IN_TEMPDB = ON );

    ALTER TABLE dbo.Record_Stub ADD CONSTRAINT PK_Record_Stub PRIMARY KEY NONCLUSTERED ( RecordID ) WITH (PAD_INDEX = OFF, FILLFACTOR = 90);

    SQL DBA,SQL Server MVP(07, 08, 09) "It's a dog-eat-dog world, and I'm wearing Milk-Bone underwear." "Norm", on "Cheers". Also from "Cheers", from "Carla": "You need to know 3 things about Tortelli men: Tortelli men draw women like flies; Tortelli men treat women like flies; Tortelli men's brains are in their flies".

  • Thank you all very much for your suggestions. We are reviewing this to see how best to proceed. Will provide feedback on how this progresses.

  • I would make sure your process design is efficient.   Deleting/updating/selecting database records should use proper primary key.   I would re-design your processes so that there is a single function in separate stored procedures.

     

    DBASupport

  • I am pleased to confirm that the solution provided by Jeffrey Williams has solved the issue for us. The modified code has been live for a few days now, and no further deadlocks have occurred.

    Many thanks all for your help.

  • Thank you for the update - glad that worked out for you.

    Jeffrey Williams
    “We are all faced with a series of great opportunities brilliantly disguised as impossible situations.”

    ― Charles R. Swindoll

    How to post questions to get better answers faster
    Managing Transaction Logs

Viewing 8 posts - 1 through 7 (of 7 total)

You must be logged in to reply to this topic. Login to reply