0

I have a table tblImportData that looks like this

CREATE TABLE [dbo].[tblImportData] ( 
[ImportDataID]      INT              IDENTITY(1,1)   NOT NULL,
[ImportFileNumber]  VARCHAR(50)                      NOT NULL,
[ImportData]        VARCHAR(4000)                    NOT NULL,
[FileName]          VARCHAR(1000)                        NULL,
[DateImported]      DATETIME                             NULL,
CONSTRAINT   [PK_ImportDataID] PRIMARY KEY CLUSTERED    (ImportDataID asc))

GO

CREATE NONCLUSTERED INDEX [IX_ImportData_ImportFileNumber] 
  ON tblImportData (ImportFileNumber asc)

Now there are several threads in our services that insert, read and delete data from this table.
And sometimes I see deadlocks in my logs.

So I was thinking about using WITH (UPDLOCK, ROWLOCK) on my select statements on this table but I am not sure if this will help me.

The situation is often like this

  • a thread reads an import file and inserts each line of the file into tblImportData and gives all these lines the same ImportFileNumber
  • the same thread then calls a stored procedure that has a parameter ImportFileNumber
  • That stored procedure will then read all rows from tblImportTable where ImportFileNumber = @ImportFileNumber and does its magic
  • after that the thread will try do delete all rows in tblImportData again delete from tblImportData where ImportFileNumber = @ImportFileNumber

And there the deadlocks appear, propably because there are about 7 threads that to the exact same thing (just with another stored proc)

So would it help me if I put WITH (UPDLOCK, ROWLOCK) in every select on tblImportData in the stored procedures ?
Will it not make some of the selects hang ?
Will it not make my delete hang ?

some example of the select statements in these procedures (just samples, not all selects)

select top 1 
       @Data = Importdata
from   dbo.tblImportData
where  ImportFileNumber = @ImportFileNumber
and    substring(Importdata, 1, 1) = '6'
insert into @table (ImportData, LaadDate, Reference, Chassis, Price)
 select Importdata,
       convert(datetime, substring(Importdata, 22, 10), 126) as LaadDate,
       substring(Importdata, 32, 10) as Reference,
       substring(Importdata, 156, 17) as Chassis,
       convert(decimal(16,2), substring(Importdata, 208, 6)) as Price
from   tblImportData
where  ImportFileNumber = @ImportFileNumber
declare crImportData cursor local FAST_FORWARD READ_ONLY for
select Importdata,
       FileName,
       DateImported
from   tblImportData
where  ImportFileNumber = @ImportFileNumber

Deadlock graph

enter image description here

3 Answers 3

4

So would it help me if I put WITH (UPDLOCK, ROWLOCK) in every select on tblImportData in the stored procedures ?

Hard to say without knowing exactly what is causing the deadlocks and more about how your overall process is intended to run. It might make your problems worse.

It's tedious work, but the best way to resolve a deadlock is to capture the deadlock graph and ideally the query plans for the transactions involved. You can then usually work out exactly what is causing the deadlock, which will lead you to one or more potential solutions. It's always better to understand why you're making a change like this, rather than just hoping it will work.

Will it not make some of the selects hang?

If by 'hang' you mean 'block', then potentially, yes. Update locks are not compatible with each other like Shared locks are.

Will it not make my delete hang?

Maybe. Shared locks are just as incompatible with the Exclusive locks needed by a deletion as Update locks are.

The main difference is that Update locks acquired via an UPDLOCK hint on qualifying rows are held to the end of the transaction. Shared locks are normally released when moving on to test the next row, but this can be extended to the end of the statement if the query plan has certain features like prefetching on a key lookup.

Design

Aside from all that, much depends on your design like whether you intend multiple threads work on the same import file number or they should be exclusive.

If they can work on the same file, it may be difficult to avoid common deadlock scenarios. For example, one thread may be reading using a nonclustered index seek followed by a clustered index lookup while another thread is deleting rows from the clustered index, followed by deleting the corresponding rows from the nonclustered index. Accessing the same resources in a different order is the classic recipe for a deadlock.

You might find the application design has a fault, or you're not using SQL Server in the best way if you intend this table to be a sort of queue of work (see Erik's answer).

Deadlocks

Deadlocks can also arise when SQL Server chooses a different execution plan because the number of rows in the table or value distribution statistics have changed. Your nonclustered index isn't covering for your queries (understandable, given the size of the other attributes) so SQL Server will often be making a choice between scanning the table (clustered index) or seeking the nonclustered index and performing some number of lookups.

If you aren't seeing many deadlocks and they're not causing problems other than being logged, you might decide it's not worth 'fixing'. Presumably, your process is engineered to retry failures like this.

The other usual fixes for this broad type of problem are to switch to a row-versioning isolation level like Read Committed Snapshot Isolation (RCSI) or Snapshot Isolation (SI) so readers don't acquire locks, or to make the nonclustered index covering (including by changing the clustered index) so lookups aren't needed.

You won't really know if either of those is appropriate until you've completed the deadlock root cause analysis.

Naturally, not every deadlock requires an in-depth analysis. Once you've understood the basic mechanisms and investigated a number of them, you'll come to recognise patterns to avoid, as in David's answer.

0
3

Table Improvement

Your table really should look like this, with a computed column to help the non-sargable predicate shown in one of your queries.

CREATE TABLE 
    dbo.tblImportData
(
    ImportDataID integer IDENTITY(1, 1) NOT NULL,
    ImportFileNumber varchar(50) NOT NULL,
    ImportData varchar(4000) NOT NULL,
    ImportData_Sub AS /*This is new*/
        CONVERT
        (
            bit,
            CASE 
                WHEN SUBSTRING(ImportData, 1, 1) = '6'
                THEN 'true'
                ELSE 'false'
            END
        ),
    FileName varchar(1000) NULL,
    DateImported datetime NULL,
    CONSTRAINT PK_ImportDataID
        PRIMARY KEY CLUSTERED (ImportDataID)
);
GO

CREATE INDEX 
    ImportData_ImportFileNumber
ON tblImportData       /*This is new*/
    (ImportFileNumber, ImportData_Sub);

First Query

select top 1 
       @Data = Importdata
from   dbo.tblImportData
where  ImportFileNumber = @ImportFileNumber
and    substring(Importdata, 1, 1) = '6'

TOP without an ORDER by is useless. See:

This is the query that the computed column and updated index will help.

The Rest

Since everything else is primarily searching on ImportFileNumber, your index should generally be sufficient, but to your question about if adding UPDLOCK and ROWLOCK hints will help: No, not without a READPAST hint to skip over currently locked rows.

Additionally, you're probably better off deleting from the table first, and using the results for further processing. I'm not sure what your whole process looks like, but an example would be something like:

DECLARE
    @table table
(
    ImportData varchar(4000) NOT NULL,
    LaadDate date,
    Reference varchar(10),
    Chassis varchar(17),
    Price decimal(6, 2)
);

INSERT INTO
    @table
(
    ImportData,
    LaadDate,
    Reference,
    Chassis,
    Price
)
SELECT
    id.ImportData,
    LaadDate = CONVERT(datetime, SUBSTRING(id.ImportData, 22, 10), 126),
    Reference = SUBSTRING(id.ImportData, 32, 10),
    Chassis = SUBSTRING(id.ImportData, 156, 17),
    Price = CONVERT(decimal(16, 2), SUBSTRING(id.ImportData, 208, 6))
FROM 
(
    DELETE id WITH(ROWLOCK)
    OUTPUT 
        Deleted.ImportData
    FROM dbo.tblImportData AS id
    WHERE id.ImportFileNumber = @ImportFileNumber
) AS id;

Assuming that the contents of the table variable drives the remaining queue processing, of course.

This way, you don't have to worry about making a second trip to delete from the table. Assuming you have proper error handling in place that will roll back to the delete, you shouldn't lose anything in the event of a failure during processing.

0
3

Change the clustered index so the concurrent transactions aren't reading each other's rows.

CREATE TABLE [dbo].[tblImportData] 
( 
[ImportDataID]      INT              IDENTITY(1,1)   NOT NULL,
[ImportFileNumber]  VARCHAR(50)                      NOT NULL,
[ImportData]        VARCHAR(4000)                    NOT NULL,
[FileName]          VARCHAR(1000)                        NULL,
[DateImported]      DATETIME                             NULL,
CONSTRAINT [PK_ImportDataID] PRIMARY KEY CLUSTERED (ImportFileNumber, ImportDataID)
)

or even better, create the "parent" table and reference that with an integer foreign key.

CREATE TABLE ImportFiles
(
  ImportFileId     int identity primary key,
  ImportFileNumber varchar(50),
  ImportDate       datetime2(0)
)


CREATE TABLE ImportData
( 
    ImportFileId      INT                              NOT NULL,
    ImportDataID      INT              IDENTITY(1,1)   NOT NULL,
    ImportData        VARCHAR(4000)                    NOT NULL,
    FileName          VARCHAR(1000)                        NULL,
    DateImported      DATETIME                             NULL,
    CONSTRAINT PK_ImportDataID PRIMARY KEY CLUSTERED (ImportFileId, ImportDataID),
    CONSTRAINT FK_ImportDataID FOREIGN KEY (ImportFileID) references ImportFiles(ImportFileId)
)

This is the correct pattern for most "child tables", and one of the cases where the reflexive "int identity primary key" leads people into trouble.

All the rows for a single ImportFileNumber will be contiguous in in the index, and a query reading rows for one ImportFileNumber will never need to scan rows belonging to another ImportFileNumber.

1
  • 2
    The design should have been like this from the start, It will take some work but I am going to change it like this. Then we will see if the deadlocks still appear or not, thanks
    – GuidoG
    Commented May 6 at 11:49

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.