0

I am getting a deadlock error with below stored procedure - UpdateTestEvents.

Below is the xml deadlock report:

<deadlock>
 <victim-list>
  <victimProcess id="process1128b529468" />
 </victim-list>
 <process-list>
  <process id="process1128b529468" taskpriority="0" logused="0" waitresource="KEY: 7:72057594042777600 (fec90e3a2350)" waittime="2364" ownerId="158290173" transactionname="user_transaction" lasttranstarted="2017-12-17T01:20:45.553" XDES="0x1064ff98408" lockMode="U" schedulerid="9" kpid="6664" status="suspended" spid="57" sbid="2" ecid="0" priority="0" trancount="2" lastbatchstarted="2017-12-17T01:20:45.547" lastbatchcompleted="2017-12-17T01:20:45.543" lastattention="1900-01-01T00:00:00.543" clientapp="EntityFramework" hostname="STAAP8895" hostpid="3616" loginname="XLAPSDBScoring" isolationlevel="read committed (2)" xactid="158290173" currentdb="7" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
   <executionStack>
    <frame procname="analytics.dbo.UpdateTestEvents" line="25" stmtstart="1836" stmtend="2132" sqlhandle="0x030007005e4b4b2a97304c0155a5000001000000000000000000000000000000000000000000000000000000">
UPDATE dbo.History
SET Ignore = 0
WHERE Number = @Number
    AND dbo.StringsMatch(@candidate, ACType, DEFAULT) =    </frame>
   </executionStack>
   <inputbuf>
Proc [Database Id = 7 Object Id = 709577566]   </inputbuf>
  </process>
  <process id="process1127e522ca8" taskpriority="0" logused="301092" waitresource="KEY: 7:72057594043039744 (c41e1b4226b6)" waittime="2364" ownerId="158290165" transactionname="user_transaction" lasttranstarted="2017-12-17T01:20:45.447" XDES="0xf8dc5ff8a8" lockMode="U" schedulerid="2" kpid="4888" status="suspended" spid="60" sbid="2" ecid="0" priority="0" trancount="2" lastbatchstarted="2017-12-17T01:20:45.440" lastbatchcompleted="2017-12-17T01:20:45.437" lastattention="1900-01-01T00:00:00.437" clientapp="EntityFramework" hostname="STAAP1493" hostpid="3304" loginname="XLAPSDBScoring" isolationlevel="read committed (2)" xactid="158290165" currentdb="7" lockTimeout="4294967295" clientoption1="671088672" clientoption2="128056">
   <executionStack>
    <frame procname="analytics.dbo.UpdateTestEvents" line="32" stmtstart="2370" stmtend="3926" sqlhandle="0x030007005e4b4b2a97304c0155a5000001000000000000000000000000000000000000000000000000000000">
WITH ValidOriginsAndDestinations AS
(
      SELECT Origin FROM dbo.History
     WHERE Ignore = 0
        AND Number = @Number
     UNION ALL
         SELECT Destination FROM dbo.History
     WHERE Ignore = 0
        AND Number = @Number
)
UPDATE fh
SET Ignore = 0
FROM dbo.History AS fh
WHERE Number = @Number
    AND Ignore = 1
    AND 
    (       
        Origin IN (SELECT * FROM ValidOriginsAndDestinations)
        OR Destination IN (SELECT * FROM ValidOriginsAndDestinations)    </frame>
   </executionStack>
   <inputbuf>
Proc [Database Id = 7 Object Id = 709577566]   </inputbuf>
  </process>
 </process-list>
 <resource-list>
  <keylock hobtid="72057594042777600" dbid="7" objectname="analytics.dbo.History" indexname="PK_History" id="lockf50c41ac80" mode="X" associatedObjectId="72057594042777600">
   <owner-list>
    <owner id="process1127e522ca8" mode="X" />
   </owner-list>
   <waiter-list>
    <waiter id="process1128b529468" mode="U" requestType="wait" />
   </waiter-list>
  </keylock>
  <keylock hobtid="72057594043039744" dbid="7" objectname="xl_analytics_aviation.dbo.History" indexname="IX_History_Number" id="lock1128b5be680" mode="U" associatedObjectId="72057594043039744">
   <owner-list>
    <owner id="process1128b529468" mode="U" />
   </owner-list>
   <waiter-list>
    <waiter id="process1127e522ca8" mode="U" requestType="wait" />
   </waiter-list>
  </keylock>
 </resource-list>
</deadlock>

And the stored procedure looks like below:

CREATE PROCEDURE [dbo].[UpdateTestEvents]
    @Number varchar(50)
AS
DECLARE @tolerance decimal(10,10) = 0.15
DECLARE @totalEvents decimal(10,0) = (SELECT COUNT(*) FROM dbo.History fh WHERE fh.Number = @Number)

IF(@totalEvents = 0) RETURN

DECLARE @candidate VARCHAR(50) = 
    (SELECT TOP 1 ACType
    FROM dbo.History AS fh
    WHERE fh.Number = @Number
    GROUP BY ACType
    HAVING (COUNT(*) / @totalEvents) > @tolerance
    ORDER BY MAX(ActualDepartureTime) DESC)
SELECT @candidate

UPDATE dbo.History
SET Ignore = 0
WHERE Number = @Number
    AND dbo.StringsMatch(@candidate, ACType, DEFAULT) = 1;


WITH ValidOriginsAndDestinations AS
(

    SELECT Origin FROM dbo.History
     WHERE Ignore = 0
        AND Number = @Number
     UNION ALL
     SELECT Destination FROM dbo.History
     WHERE Ignore = 0
        AND Number = @Number
)

UPDATE fh
SET Ignore = 0
FROM dbo.History AS fh
WHERE Number = @Number
    AND Ignore = 1
    AND 
    (
        Origin IN (SELECT * FROM ValidOriginsAndDestinations)
        OR Destination IN (SELECT * FROM ValidOriginsAndDestinations)
    );

WITH Comfirmeddt AS
(
    SELECT a.lat, a.long FROM dbo.places AS a
    JOIN dbo.History AS fh
    ON     a.tidentifier = fh.Origin
        OR a.tidentifier = fh.Destination    
    WHERE fh.Number = @Number
    GROUP BY a.tidentifier, a.lat, a.long
)
UPDATE fh
SET Ignore = 0
FROM dbo.History AS fh
JOIN dbo.places AS a
ON a.tidentifier = fh.Origin
    OR a.tidentifier = fh.Destination
WHERE fh.Ignore = 1
AND fh.Number = @Number
AND EXISTS
(
    SELECT * FROM Comfirmeddt AS confirmed   
    WHERE
    (
        a.lat  < confirmed.lat  + 0.5 AND a.lat  > confirmed.lat  - 0.5 AND
        a.long < confirmed.long + 0.5 AND a.long > confirmed.long - 0.5
    )
)

GO

I am getting the below error: An error occurred while executing the command. See the inner exception for details. Transaction (Process ID 57) was deadlocked on lock resources with another process and has been chosen as the deadlock victim. Rerun the transaction

The index definition for [PK_History] is as below:

ALTER TABLE [dbo].[History] ADD  CONSTRAINT [PK_History] PRIMARY KEY CLUSTERED 
(
    [HashCode] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
GO

The Primary Key is HashCode Can someone suggest what I can do on this query to avoid such deadlocks in future.

Please find the table structure below:

SET ANSI_NULLS ON
GO

SET QUOTED_IDENTIFIER ON
GO

SET ANSI_PADDING ON
GO

CREATE TABLE [dbo].[History](
    [HashCode] [varchar](50) NOT NULL,
    [FaID] [varchar](50) NOT NULL,
    [Number] [varchar](255) NOT NULL,
    [ActualArrivalTime] [datetime] NULL,
    [ActualDepartureTime] [datetime] NULL,
    [ACType] [varchar](10) NULL,
    [Destination] [varchar](40) NULL,
    [DestinationCity] [varchar](100) NULL,  
    [Origin] [varchar](40) NULL,
    [Ignore] [bit] NOT NULL DEFAULT ((1)),
    [FlNumber] [varchar](255) NULL,
    [DateAdded] [datetime] NOT NULL DEFAULT (getdate()),
 CONSTRAINT [History] PRIMARY KEY CLUSTERED 
(
    [HashCode] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
) ON [PRIMARY] TEXTIMAGE_ON [PRIMARY]

GO

SET ANSI_PADDING ON
GO

ALTER TABLE [dbo].[History]  WITH CHECK ADD  CONSTRAINT [FK_Number] FOREIGN KEY([Number])
REFERENCES [dbo].[Number] ([Number])
GO

ALTER TABLE [dbo].[History] CHECK CONSTRAINT [FK_Number]
GO

The call to the stored procedure happends from .NET code and is achieved by entity framework. Below is the skeleton of the call to this stored procedure from application

using (var db = new NumberDbContext())
            {
foreach (var tn in Numbers)
            {
 db.UpdateTestEvents(tailNumber);
}
}

Please find the execution plan of the query below: https://www.brentozar.com/pastetheplan/?id=B1dGzUMQf

Also definition if IX_History_Number index:

CREATE NONCLUSTERED INDEX [IX_History_Number] ON [dbo].[History]
(
    [Number] ASC
)WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, SORT_IN_TEMPDB = OFF, DROP_EXISTING = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY]
GO
user2081126
  • 77
  • 1
  • 11
  • What is your primary key field in PK_Histor and what is IX_History_Number definition? – sepupic Dec 27 '17 at 14:32
  • IX_History_Number is Non clustered non unique index created for one of the column named Number. And PK_History is clustered primary key Index for the table – user2081126 Dec 27 '17 at 14:47
  • >>>And PK_History is clustered primary key Index for the table<<< Any PK has a key, the field(s) that gives unique combination in a row. So what is the KEY of PK_History? And you did not post IX_History_Number definition, I wanted to know if it has Ignore field (it should have it, because your update wants to update this index too) – sepupic Dec 27 '17 at 14:57
  • @user2081126, the different index access paths are contributing the deadlock. A quick-and-dirty way to avoid these deadlocks is to use an exclusive transaction level `sp_getapplock` on the @number value to serialize access. I assume the proc is called from a transaction initiated in the app code. – Dan Guzman Dec 27 '17 at 15:00
  • @sepupic: I will update the question with definition of indexes. The primary key is HashCode and the index looks as below : ALTER TABLE [dbo].[History] ADD CONSTRAINT [PK_History] PRIMARY KEY CLUSTERED ( [HashCode] ASC )WITH (PAD_INDEX = OFF, STATISTICS_NORECOMPUTE = OFF, SORT_IN_TEMPDB = OFF, IGNORE_DUP_KEY = OFF, ONLINE = OFF, ALLOW_ROW_LOCKS = ON, ALLOW_PAGE_LOCKS = ON) ON [PRIMARY] GO – user2081126 Dec 27 '17 at 15:06
  • @DanGuzman: The proc is called froma transaction initiated in the app code – user2081126 Dec 27 '17 at 15:09
  • @sepupic: I modified the question to include the definition of index also – user2081126 Dec 27 '17 at 15:36
  • Can you please update your question with the actual execution plan of your sp? We need it to see your UPDATEs plans – sepupic Dec 28 '17 at 09:20
  • @sepupic: Added the code that calls the stored procedure from the application. – user2081126 Dec 28 '17 at 10:19
  • No, I asked you to attach here the execution plan. https://stackoverflow.com/questions/7359702/how-do-i-obtain-a-query-execution-plan – sepupic Dec 28 '17 at 10:44
  • After you get it, you can paste it here: https://www.brentozar.com/pastetheplan/ and update your question with the link to it – sepupic Dec 28 '17 at 10:46
  • @sepupic; Added execution plan to below link: https://www.brentozar.com/pastetheplan/?id=B1dGzUMQf – user2081126 Dec 28 '17 at 11:18
  • @sepupic; Did you get a chance to look at the execution plan. Thanks – user2081126 Dec 28 '17 at 13:04
  • Yes I did, but there is no IX_History_Number update in it (you still did not show this index definition, does it has only Number as a key and no more other columns, even as included?), so I don't understand why does it hold U-lock on this index key. You can try to add the index suggested in the plan: ON [dbo].[History] ([Number]) INCLUDE ([HashCode],[AType]), doing so you will offer the best index for the first update so that IX_History_Number will not be used in it, the second update will still use IX_History_Number if it contains only Number, so it may help – sepupic Dec 28 '17 at 13:30
  • (continue) because this way two updates will use different indexes. The other strange thing is that on one hand there is only one db in use, dbid = 7, and your plan shows only db **analytics**, but in your deadlock graph there is another db, another proc: xl_analytics_aviation.dbo.UpdateTestEvents, so maybe this OTHER proc from OTHER db has different code with different plan? Please check your sp in this other db: **xl_analytics_aviation** – sepupic Dec 28 '17 at 13:34
  • @sepupic: Sorry about the db name. it was a typo. the deadlock graph db name should be analytics only, it is not xl_analytics_aviation – user2081126 Dec 28 '17 at 13:43
  • @sepupic; Also updated index definition in the question. Thanks – user2081126 Dec 28 '17 at 13:49
  • @sepupic; Also I just updated index and then uploaded execution plan. Now also the first update uses both the indexes – user2081126 Dec 28 '17 at 16:52
  • @sepupic: so the lock is caused due to accessing the clustered index by both the updates? – user2081126 Jan 02 '18 at 15:18

2 Answers2

0

The deadlock is due to queries using different indexes to access the same row(s). One way to avoid this deadlock condition is to use an application lock to serialize access to the same @Number value until the transaction is committed. Note that the proc must be invoked from an explicit transaction with a Transaction lock owner.

CREATE PROCEDURE [dbo].[UpdateTestEvents]
    @Number varchar(50)
AS
DECLARE @return_code int;
EXEC @return_code = sp_getapplock @Resource = @Number, @LockMode = 'Exclusive', @LockOwner = 'Transaction';
IF @return_code NOT IN(0,1)
BEGIN
    RAISERROR('Unexpected sp_getapplock return code is %d',16,1,@return_code);
    RETURN @return_code;
END;
--remainder of proc code here
GO
Dan Guzman
  • 43,250
  • 3
  • 46
  • 71
  • Will this just be checking if the resource is locked and then throw an exception – user2081126 Dec 27 '17 at 16:23
  • @Unless you specify a @LockTimeout, sp_getapplock will block until the lock is acquired (assuming a default -1 `LOCK_TIMEOUT` for no timeout). That's the behavior you'd get with normal row locking behavior. – Dan Guzman Dec 27 '17 at 16:28
  • Thanks Dan. But I am actually looking for something that would continue the execution without lock. Anything I can modify in the stored proc/index. The suggestion you provided still have an exception block that would be hit when it return any value apart from 0 or 1 – user2081126 Dec 27 '17 at 16:50
  • If you look at the return codes in the doc for other than 0 or 1, those should not happen here. The return code check is the best practice, which is why I included it. How long does the proc run and how often is it executed? To avoid the deadlock, you actually need a lock rather than trying to avoid one. It's just a question of whether SQL does it for you with a row lock on the same index, which may be challenging to suggest here without knowing much about the application and other workload. – Dan Guzman Dec 27 '17 at 17:26
  • It is executed everyday and it rubs for around 30min for different numbers. – user2081126 Dec 27 '17 at 22:16
  • @user2081126, what is the duration of a single execution? Are you saying the deadlock occurred with different number values? Add the full CREATE TABLE DDL with all indexes to your question in that case. – Dan Guzman Dec 27 '17 at 22:20
  • This stored procedures gets called in a loop of numbers. The numbers comes around 300-400. Duration of each call is less only. May be around 5-10seconds – user2081126 Dec 27 '17 at 22:27
  • I just modified the question to include table structure also – user2081126 Dec 28 '17 at 08:00
  • Thanks for the plan. The column name in the DDL is `ACType` but the plan shows name `AType`. Try adding included columns to the non-clustered index so that it covers the UPDATE queries, avoiding the key lookups and associated lock involved in the deadlock: `CREATE NONCLUSTERED INDEX IX_History_Number ON dbo.History(Number) INCLUDE(Ignore, AType, Origin, Destination, ActualDepartureTime);` – Dan Guzman Dec 28 '17 at 13:48
  • Sorry about the column name. Its a typo. the column name is ACType – user2081126 Dec 28 '17 at 14:14
  • Also Dan, I am not able to recreate this deadlock when I run locally in server. is there a way that we can recreate this issue in localy so that I can test the changes you suggested. Thanks for your help. – user2081126 Dec 28 '17 at 14:16
  • Concurrency issues are often difficult to reproduce. Try executing the proc with the same @Number parameter value from multiple SSMS windows concurrently. Add a `WAITFOR TIME` statement as the first statement so the executions run at exactly the same time. – Dan Guzman Dec 28 '17 at 14:27
  • Thanks Dan. But since these 2 indexes point to 2 different columns will the change suggested by you will have an impact – user2081126 Dec 28 '17 at 14:44
  • @user2081126, I don't understand what you mean by 2 indexes. I suggested changing only the non-clustered index which I expect will avoid the clustered primary key index from being used in the query via key lookups. There will be impact of additional storage space for the included columns. Not being familiar with your overall application and workload, I can't speak to additional impact. – Dan Guzman Dec 28 '17 at 15:15
  • Thanks Dan. Apart from additional storage will it cause any other impacts. say for eg: will execution of this table get slowed up or something. – user2081126 Dec 28 '17 at 17:00
  • Also I made the change you suggested and now the update uses only non-clustered index. – user2081126 Dec 28 '17 at 17:00
  • SQL Server will need to do more work when the included columns are updated. Were you able to repro the deadlock without the index change, and if so, did the index change resolve it? – Dan Guzman Dec 28 '17 at 17:08
  • I wasn't able to recreate it. – user2081126 Dec 29 '17 at 07:18
-1

I would firstly check the isolation level in the database,if snapshot isolation is on, there is less likely to get shared locks in the database.

select name
        , s.snapshot_isolation_state
        , snapshot_isolation_state_desc
        , is_read_committed_snapshot_on
        , recovery_model
        , recovery_model_desc
        , collation_name
    from sys.databases s

To avoid deadlocks while executing query, it is good to issue nolock in this scenario, although some argue that it causes dirty reads, use begin and end for each update statement or use try block.

CREATE PROCEDURE [dbo].[UpdateTestEvents] @Number VARCHAR(50)
AS
BEGIN
    DECLARE @tolerance DECIMAL(10, 10) = 0.15
    DECLARE @totalEvents DECIMAL(10, 0) = (
            SELECT COUNT(*)
            FROM dbo.History fh(NOLOCK)
            WHERE fh.Number = @Number
            )

    IF (@totalEvents = 0)
    BEGIN
        DECLARE @candidate VARCHAR(50) = (
                SELECT TOP 1 ACType
                FROM dbo.History(NOLOCK) AS fh
                WHERE fh.Number = @Number
                GROUP BY ACType
                HAVING (COUNT(*) / @totalEvents) > @tolerance
                ORDER BY MAX(ActualDepartureTime) DESC
                )

        --SELECT @candidate
        UPDATE dbo.History
        SET Ignore = 0
        WHERE Number = @Number
            AND dbo.StringsMatch(@candidate, ACType, DEFAULT) = 1;

        BEGIN
            WITH ValidOriginsAndDestinations
            AS (
                SELECT Origin
                FROM dbo.History(NOLOCK)
                WHERE Ignore = 0
                    AND Number = @Number
                
                UNION ALL
                
                SELECT Destination
                FROM dbo.History(NOLOCK)
                WHERE Ignore = 0
                    AND Number = @Number
                )
            UPDATE fh
            SET Ignore = 0
            FROM dbo.History AS fh
            WHERE Number = @Number
                AND Ignore = 1
                AND (
                    Origin IN (
                        SELECT *
                        FROM ValidOriginsAndDestinations
                        )
                    OR Destination IN (
                        SELECT *
                        FROM ValidOriginsAndDestinations
                        )
                    )
        END

        BEGIN
            WITH AirportsWithConfirmedFlights
            AS (
                SELECT a.lat
                    ,a.long
                FROM dbo.places AS a
                INNER JOIN dbo.History AS fh ON a.tidentifier = fh.Origin
                    OR a.tidentifier = fh.Destination
                WHERE fh.Number = @Number
                GROUP BY a.tidentifier
                    ,a.lat
                    ,a.long
                )
            UPDATE fh
            SET Ignore = 0
            FROM dbo.History AS fh
            INNER JOIN dbo.places AS a ON a.tidentifier = fh.Origin
                OR a.tidentifier = fh.Destination
        END
    END
END

`

  • Nolock` and Bad practices

    various documents, best practices suggested to stop using nolock hints as this can cause dirty reads and other affects. But if we test by using nolock hints and the performance and affects and well documented they are much useful. The best work around for this is to alter isolation level in the database. There are several considerations for this aswel.

Community
  • 1
  • 1
Ven
  • 2,011
  • 1
  • 13
  • 27
  • Dead lock is happening on the Update query right. So will it help if we place NOLOCK on select query – user2081126 Dec 27 '17 at 14:03
  • It should help if u place nolock in select query for dbo.history – Ven Dec 27 '17 at 14:04
  • if you read after a committed transaction, it wont be a dirty read. Using begin and end for each statement or using try block takes care of it. – Ven Dec 27 '17 at 14:14
  • NOLOCK will not change anything as there is **no S-lock** in the deadlock graph at all, the conflict is between 2 updates that access the same clustered table and its index in inverse order – sepupic Dec 27 '17 at 14:26
  • I agree with you, there is also possibility of acquiring shared locks, anyways i have suggested using try and catch blocks to follow updates in a seqeunce – Ven Dec 27 '17 at 14:30