Please start any new threads on our new site at https://forums.sqlteam.com. We've got lots of great SQL Server experts to answer whatever question you can come up with.

 All Forums
 SQL Server 2008 Forums
 Transact-SQL (2008)
 My transactions need isolating!

Author  Topic 

EliW
Starting Member

5 Posts

Posted - 2010-05-10 : 12:38:59
TIA for your help! I am working with a table that will lock FoobarID for a given user for five minutes. (I want to keep a lock history, so it'll be impractical to use a primary key, or to delete old records, and so on.)

I need to prevent multiple users from creating a lock on the same FoobarID at the same time. So far, I've just resorted to making the INSERT statement "atomic", which I think will help, but I probably need to do more. Here is my code so far; it's just a mock-up, that will become a stored proc with parameters when it grows up :
DECLARE @LockID INT
DECLARE @FoobarID INT
DECLARE @UserID INT

SET @FoobarID = 1
SET @UserID = 1

INSERT tblLock
(LockTypeID
,LockObjectID
,StartTime
,EndTime
,UserID)
SELECT LockTypeID = 1
,LockObjectID = @FoobarID
,StartTime = GETDATE()
,EndTime = DATEADD(MINUTE, 5, GETDATE())
,UserID = @UserID
WHERE NOT EXISTS (SELECT 1
FROM tblLock
WHERE LockTypeID = 1
AND LockObjectID = @FoobarID
AND GETDATE() BETWEEN StartTime AND EndTime)

IF @@ROWCOUNT = 1
SET @LockID = SCOPE_IDENTITY()
ELSE
SET @LockID = -1

SELECT @LockID
SELECT * FROM tblLock

There will also be a separate stored proc for extending the lock for five additional minutes, thus:
UPDATE tblLock
SET EndTime = DATEADD(MINUTE, 5, GETDATE())
WHERE LockID = @LockID

Here is my code again, with several suggestions added (marked A, B, C, D and E). Which of these should I use, and which ones would be superfluous and/or will just degrade performance?

Also, should any of these items (A,B,C,D,E) be added to the second stored proc (the one that extends the lock for five minutes)?
SET TRANSACTION ISOLATION LEVEL REPEATABLE READ --(A) Is this necessary?
SET TRANSACTION ISOLATION LEVEL SERIALIZABLE --(B) This is probably too much.


DECLARE @LockID INT
DECLARE @FoobarID INT
DECLARE @UserID INT

SET @FoobarID = 1
SET @UserID = 1

BEGIN TRANSACTION --(C) Isn't the INSERT statement atomic, making this unnecessary?

INSERT tblLock
(LockTypeID
,LockObjectID
,StartTime
,EndTime
,UserID)
SELECT LockTypeID = 1
,LockObjectID = @FoobarID
,StartTime = GETDATE()
,EndTime = DATEADD(MINUTE, 5, GETDATE())
,UserID = @UserID
WHERE NOT EXISTS (SELECT 1
FROM tblLock (UPDLOCK, ROWLOCK) --(D) and (E), is (E) necessary?
WHERE LockTypeID = 1
AND LockObjectID = @FoobarID
AND GETDATE() BETWEEN StartTime AND EndTime)

COMMIT TRANSACTION --(C) again

IF @@ROWCOUNT = 1
SET @LockID = SCOPE_IDENTITY()
ELSE
SET @LockID = -1

SELECT @LockID
SELECT * FROM tblLock

Kristen
Test

22859 Posts

Posted - 2010-05-10 : 12:59:29
The INSERT statement is atomic, and thus I don't think you need the transaction isolation settings, not the UPDLOCK / ROWLOCK hints, nor the transaction block (although that is benign)

I've done this type of ID-lock before, although not for a given time-limit. We had routines to remove locks (after a PC crash, for example), a routine that will tell a user [trying to lock record] who currently held the lock, and when we updated a record we double-checked that the lock was still in place at that time:

UPDATE U
SET Col1 = 'XXX'
FROM MyTable AS U
WHERE U.ColID = @FoobarID
AND EXISTS (SELECT * FROM tblLock WHERE LockTypeID = 1 AND LockObjectID = @FoobarID AND UserID = @UserID)

this makes sure that the lock has not been released (some how) whilst the application thought iut was "Owned" by the user. (You would need to include your expiry date test)
Go to Top of Page

Lamprey
Master Smack Fu Yak Hacker

4614 Posts

Posted - 2010-05-10 : 14:14:05
The INSERT is atomic, but the EXISTS can still cause a deadlock without UPDLOCK, HOLDLOCK:
http://weblogs.sqlteam.com/dang/archive/2007/10/28/Conditional-INSERTUPDATE-Race-Condition.aspx
Go to Top of Page

EliW
Starting Member

5 Posts

Posted - 2010-05-10 : 17:00:34
quote:
Originally posted by Lamprey

The INSERT is atomic, but the EXISTS can still cause a deadlock without UPDLOCK, HOLDLOCK:
http://weblogs.sqlteam.com/dang/archive/2007/10/28/Conditional-INSERTUPDATE-Race-Condition.aspx



1) So even though the EXISTS query is embedded in the INSERT query, deadlocking can still occur and this is not quite as "atomic" as I thought it was?

2) I've got a co-worker insisting I should still have some kind of SET TRANSACTION ISOLATION LEVEL at the top. Wouldn't "WITH (UPDLOCK, HOLDLOCK)" be enough?

3) Throughout most of our app, we start every stored proc with SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED. If I added WITH (UPDLOCK, HOLDLOCK) to the EXISTS query, wouldn't I still be able to get away with that setting?

4) Double-checking just to make sure: Absolutely NONE of this should be applied to the UPDATE statement in which I extend the lock's end time?
Go to Top of Page

Lamprey
Master Smack Fu Yak Hacker

4614 Posts

Posted - 2010-05-10 : 17:49:19
quote:
Originally posted by EliW

quote:
Originally posted by Lamprey

The INSERT is atomic, but the EXISTS can still cause a deadlock without UPDLOCK, HOLDLOCK:
http://weblogs.sqlteam.com/dang/archive/2007/10/28/Conditional-INSERTUPDATE-Race-Condition.aspx



1) So even though the EXISTS query is embedded in the INSERT query, deadlocking can still occur and this is not quite as "atomic" as I thought it was?

2) I've got a co-worker insisting I should still have some kind of SET TRANSACTION ISOLATION LEVEL at the top. Wouldn't "WITH (UPDLOCK, HOLDLOCK)" be enough?

3) Throughout most of our app, we start every stored proc with SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED. If I added WITH (UPDLOCK, HOLDLOCK) to the EXISTS query, wouldn't I still be able to get away with that setting?

4) Double-checking just to make sure: Absolutely NONE of this should be applied to the UPDATE statement in which I extend the lock's end time?


1. Kind of; My understanding is that without the UPDLOCK, HOLDLOCK on the NOT EXISTS clause that you could get TWO inserts because of the race condition on the NOT EXISTS select part. Thus the second insert would fail (assuming proper constraints).

2. I think it's a good idea to put SET TRANSACTION ISOLATION LEVEL at the top of the stored procedure. But, two things. 1) yes, the WITH (UPDLOCK, HOLDLOCK) will over ride the SET statement at the top. and 2) If you are using READ UNCOMMITTED be careful with bad/duplicate data..!

3. See #2.

4. Correct, there shouldn't be a need to do any locking on the update itself. The one caveat to that is if you need to do other updates on that same row. Then you might need SERIALIZEABLE or something.
Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-05-11 : 02:46:43
Interesting link, thanks.

I see the article suggests that MERGE might solve that - but wouldn't an OUTER JOIN be better?

INSERT tblLock
(LockTypeID
,LockObjectID
,StartTime
,EndTime
,UserID)
SELECT LockTypeID = 1
,LockObjectID = @FoobarID
,StartTime = GETDATE()
,EndTime = DATEADD(MINUTE, 5, GETDATE())
,UserID = @UserID
FROM
(SELECT 1 AS X) AS X
LEFT OUTER JOIN tblLock AS L
ON L.LockTypeID = 1
AND L.LockObjectID = @FoobarID
AND GETDATE() BETWEEN L.StartTime AND L.EndTime
WHERE L.LockTypeID IS NULL

Go to Top of Page

Kristen
Test

22859 Posts

Posted - 2010-05-11 : 02:57:21
Re: 1 - it is atomic (I think?) just it might cause a deadlock (so if the EXISTS test succeeds [target NOT found] then the INSERT will proceed, otherwise not). Thus no race condition??
Go to Top of Page

Lamprey
Master Smack Fu Yak Hacker

4614 Posts

Posted - 2010-05-11 : 10:51:34
There shouldn't be a deadlock. Like you describe, the EXISTS check succeeds so both threads/processes perform the INSERT, of which one will fail. The Race Condition is on the EXISTS SELECT statement itself.
Go to Top of Page
   

- Advertisement -