Avoiding deadlock

I have a stored procedure spUpdateClient, which takes as params a number of properties of a client application that wants to register its existence with the database. The sp just needs to add a new row or update an existing row with this data.

I tried to accomplish this with code somethign like this. (The table I'm updating is called Client, and its primary key is ClientId, which is a value passed into the sp from the client.)

IF (SELECT COUNT(ClientId) FROM Clients WHERE ClientId=@.ClientId) = 0
BEGIN
-- client not found, create it
INSERT INTO Clients (ClientId, Hostname, Etc)
VALUES (@.ClientId, @.Hostname, @.Etc)
END

ELSE

BEGIN
-- client was found, update it
UPDATE Clients
SET Hostname=@.Hostname, Etc=@.Etc
WHERE ClientId=@.ClientId
END

But the client apps call this every second or so, so soon enough I started getting primary key violations. It looks like one client would make two calls nearly at the same time, both would get a 0 value on the SELECT line, so both would try to insert a new row with the same ClientId. No good.

So then I added

SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
BEGIN TRANSACTION

at the top, and a COMMIT at the bottom. I thought the first one in would get to run the whole sp, and the next one in would have to wait for the first to be done.

Instead I'm now getting deadlock errors.

If I understand the docs right, that's because the exclusive lock is not placed on the Clients table until the INSERT happens, not at the SELECT. So when two calls to the sp happen at nearly the same time (call them A and B), A does the SELECT and that locks Clients so nobody else can update it. Then B does the SELECT, locking Clients so nobody else (including A) can update it. Now A needs to exclusively lock Clients to do its INSERT, but B still has that read lock on it, and they're deadlocked.

I could catch the deadlock in my client app after SQL Server kills one of the transactions, but it seems to me there should be some way to set a lock at the top of the sp that says "nobody else can enter this sp until I exit it". Any such thing?

Thanks.

Nate Hekman

You can change your code to the following:

BEGIN TRANSACTION
IF NOT EXISTS(SELECT * FROM Clients WITH(SERIALIZABLE, XLOCK)

WHERE ClientId=@.ClientId)
BEGIN
-- client not found, create it
INSERT INTO Clients (ClientId, Hostname, Etc)
VALUES (@.ClientId, @.Hostname, @.Etc)
END

ELSE

BEGIN
-- client was found, update it
UPDATE Clients
SET Hostname=@.Hostname, Etc=@.Etc
WHERE ClientId=@.ClientId
END

COMMIT

The XLOCK hint in the SELECT statement will ensure that if the rows exists you lock it exclusively so the update will work fine. And the SERIALIZABLE hint will ensure that if the row doesn't exist you lock the key range for the new row. This will prevent the deadlock from happening.

Another approach is to do the following:

BEGIN TRANSACTION
UPDATE Clients WITH(SERIALIZABLE)
SET Hostname=@.Hostname, Etc=@.Etc
WHERE ClientId=@.ClientId
IF @.@.ROWCOUNT = 0

BEGIN
-- client not found, create it
INSERT INTO Clients (ClientId, Hostname, Etc)
VALUES (@.ClientId, @.Hostname, @.Etc)
END

COMMIT

|||

This is a very good understanding of why you are getting a deadlock :) You can cause it to single thread by adding an LOCK hint to the existence check. Also, change from using a count to exists, it will be better as it just needs to see a single row, rather than counting them all. No need for serializable here now, but make sure ClientId is indexed right (and if it is the declare primary key that should be fine)

BEGIN TRANSACTION

IF EXISTS (SELECT 1
FROM Clients WITH (XLOCK)
WHERE ClientId=@.ClientId)
BEGIN
-- client not found, create it
INSERT INTO Clients (ClientId, Hostname, Etc)
VALUES (@.ClientId, @.Hostname, @.Etc)
END
ELSE
BEGIN
-- client was found, update it
UPDATE Clients
SET Hostname=@.Hostname, Etc=@.Etc
WHERE ClientId=@.ClientId
END

COMMIT TRANSACTION

An alternative is to just ignore the deadlock as you know why it occurs, and since you are only keeping a single copy, it is just as good and will be fast enough...

Another alternative would be to switch to an insert only methodology and just pump rows into a table. It would give you counts of visits, actual times of visits, etc. You could also glean the same information as you have now with no locking problems at all.

--clientId, visitDate would be the likely UNIQUE constraint
--if they are > .003 seconds apart, which I don't know based on your needs

create table clientVisit
(
clientVisitId int identity primary key,
clientId guid,
visitDate datetime default (getdate()),
hostName varchar(?),
etc varchar(?)
)

then just insert... It will take more disk space, but it should be just as fast. You could then pull the data off periodically and get the same information, plus some.

|||

I didn't think that:

BEGIN TRANSACTION
IF NOT EXISTS(SELECT * FROM Clients WITH(SERIALIZABLE, UPDLOCK)
WHERE ClientId=@.ClientId)

Would suffice since a SHARED lock compatible with an UPDATE lock? In this case, the second could still read there to be no rows.

(I did overlook that you need to increase the isolation level just in case READ_COMMITTED_SNAPSHOT is enabled. And I didn't realize you could put lock hints on UPDATE statements :)

Thanks!

|||You are right. This should be XLOCK instead.|||

Wow, thanks for the excellent replies everyone! I'm very new at T-SQL so all this locking stuff is a lot to mull over. But you've given me several good approaches that I think will work just great.

Thanks again.

Nate

|||

IF (SELECT COUNT(ClientId) FROM Clients WHERE ClientId=@.ClientId) = 0
BEGIN
-- client not found, create it
INSERT INTO Clients (ClientId, Hostname, Etc)
VALUES (@.ClientId, @.Hostname, @.Etc)
END

ELSE

BEGIN
-- client was found, update it
UPDATE Clients
SET Hostname=@.Hostname, Etc=@.Etc
WHERE ClientId=@.ClientId
END

|||

This problem's interesting and I'm sure lots of people have encountered before.

There are some things I still don't understand.
If we set the isolation level as Serializable and use XLOCK for the SELECT as follows:

SET TRANSACTION ISOLATION LEVEL SERIALIZABLE
BEGIN TRANSACTION

IF (SELECT COUNT(ClientId) FROM Clients (WITH XLOCK) WHERE ClientId=@.ClientId) = 0
BEGIN
-- client not found, create it
INSERT INTO Clients (ClientId, Hostname, Etc)
VALUES (@.ClientId, @.Hostname, @.Etc)
END

ELSE

BEGIN
-- client was found, update it
UPDATE Clients
SET Hostname=@.Hostname, Etc=@.Etc
WHERE ClientId=@.ClientId
END

COMMIT TRANSACTION

As Nate Hekman first wrote:
If I understand the docs right, that's because the exclusive lock is not placed on the Clients table until the INSERT happens, not at the SELECT. So when two calls to the sp happen at nearly the same time (call them A and B), A does the SELECT and that locks Clients so nobody else can update it. Then B does the SELECT, locking Clients so nobody else (including A) can update it. Now A needs to exclusively lock Clients to do its INSERT, but B still has that read lock on it, and they're deadlocked.

Question 1>
If the row doesn't exist, does the process go like this:
process1 executes SELECT ... WITH XLOCK and holds an exclusive lock on Clients.
Because isolation level Serializable is used, other processes can't insert new rows or update rows within the defined range. Also, because an exclusive lock is held, other processes can't even read until process1 finishes?

Question 2>
How does it solve the deadlock problem?

|||

Question 1:
When a transaction holds Exclusive locks on a range of rows, other transaction cannot read those rows, unless the second transaction's isolation level is not READ UNCOMMITTED. As Nate uses the same sp for inserts, yes, the select command blocks all readers.

Question2:
Deadlocks do not occur because a transaction that executes its select command is guaranteed to be granted all the locks it will needs further, that is, it cannot be blocked, so a deadlock can never occur. This is a sort of pre-declaration of locks, or pessimistic locking. When transaction A executes its select command, the other ones (say trans. B)cannot read. If transaction A takes a long time to commit, B might be eventually aborted. But this will not happen-the insert transaction is a short one, and it will need milliseconds to commit, after that transaction B(or other waiting transaction) will be granted the lock. In this scenario, aborts are very unlikely to occur unless the workload is high.