How do I get the right locks for this SQL?

Question

My database is SQL Server 2005/8. In a booking system we have a limit of 24 bookings on an event. This code in a stored procedure checks: - that the current user (@UserId) is not already booked on the event (@EventsID) - that the current event has a current booking list of under 24 - inserts a new booking.

BEGIN TRANSACTION 
IF (((select count (*) from dbo.aspnet_UsersEvents with (updlock) 
      where UserId = @UserId and EventsId = @EventsId) = 0) 
AND  ((SELECT Count(*)  FROM dbo.aspnet_UsersEvents with (updlock) 
      WHERE EventsId = @EventsId) < 24))
BEGIN
  insert into dbo.aspnet_UsersEvents (UserId, EventsId) 
      Values (@UserId, @EventsId)
END
COMMIT

The problem is that it is not safe. Two users might perform the test simultaneously and conclude they can both book. Both insert a row and we end up with 25 bookings.

Simply enclosing it in a transaction does not work. I tried adding WITH (UPDLOCK) to the selects in the hope that one would take update locks and keep the other out. That does not work.

Mark Storey-Smith · Accepted Answer · 2011-12-01T21:33:29.887

3

Three options:

SET TRANSACTION ISOLATION LEVEL REPEATABLE READ
Change the lock hint to WITH (UPDLOCK, HOLDLOCK)
Add a unique constraint to dbo.aspnet_UsersEvents and a TRY/CATCH around the insert.

You can use the following script to confirm that the lock is taken and immediately released if you omit HOLDLOCK. You will also see that the lock is not released (no 'releasing lock reference on KEY' output) when HOLDLOCK is used.

(Gist script)

edited Dec 01 '11 at 21:33

answered Dec 01 '11 at 21:22

Mark Storey-Smith

997
10
15

Why so complicated? See my answer. – ta.speot.is Dec 01 '11 at 21:25
Because your answer is not transactionally safe. That a single statement is always consistent is a common fallacy. – Mark Storey-Smith Dec 01 '11 at 21:29
1

Can you please point out some documentation for SQL Server's non-guarantee that a single statement is not consistent. I'm assuming he's not running at `READ UNCOMMITTED`. – ta.speot.is Dec 01 '11 at 21:33
Adding the HOLDLOCK hint seems to be the major part I was missing - thank you. – RichardHowells Dec 01 '11 at 22:07
I tried REPEATABLE READ, but it's not actually enough. Even though the reads get the same rows as before (repeatable) they miss a new one. Using SERIALIZABLE does work, but I'm just a little scared of SERIALIZABLE. – RichardHowells Dec 01 '11 at 22:08
In fact there is a unique constraint on the composite key EventsID and UserID. That only prevents one user booking twice on the same event. It does not help with the 24 limit test. – RichardHowells Dec 01 '11 at 22:12
@todda.speot.is I have a demo script if you want to pop into the [dba.se chat](http://chat.stackexchange.com/rooms/179/the-heap) – Mark Storey-Smith Dec 01 '11 at 22:35
@RichardHowells Apologies, I misunderstood the part about the event count. Glad to hear the HOLDLOCK sorts it. – Mark Storey-Smith Dec 01 '11 at 22:53

ta.speot.is · Answer 2 · 2011-12-01T21:36:23.423

Just do it in one statement, at READ COMMITTED or higher.

INSERT dbo.aspnet_UsersEvents
       (UserId,EventsId)
OUTPUT inserted.UserEventsId -- Or whatever, just getting back one row identifies the insert was successful
SELECT @UserId
       , @EventsId
 WHERE ( SELECT COUNT (*)
           FROM dbo.aspnet_UsersEvents
          WHERE UserId = @UserId
                AND EventsId = @EventsId ) = 0
       AND ( SELECT COUNT(*)
               FROM dbo.aspnet_UsersEvents
              WHERE EventsId = @EventsId ) < 24

_{Side note: your SELECT COUNT(*) for duplicate checking seems excessive, personally I'd use NOT EXISTS(SELECT NULL FROM ... WHERE UserID = ..., EventsID = ....}

You make a useful point that an Exists test is clearer than the count. Thank you. — RichardHowells, Dec 01 '11 at 22:06

How do I get the right locks for this SQL?

2 Answers2