Managing concurrency when using SELECT-UPDATE pattern

Question

Let's say you have the following code (please ignore that it's awful):

BEGIN TRAN;
DECLARE @id int
SELECT @id = id + 1 FROM TableA;
UPDATE TableA SET id = @id; --TableA must have only one row, apparently!
COMMIT TRAN;
-- @id is returned to the client or used somewhere else

To my eye, this is NOT managing concurrency properly. Just because you have a transaction doesn't mean someone else won't read the same value that you did before you get to your update statement.

Now, leaving the code as is (I realize this is better handled as a single statement or even better using an autoincrement/identity column) what are sure ways to make it handle concurrency properly and prevent race conditions that allow two clients to get the same id value?

I'm pretty sure that adding a WITH (UPDLOCK, HOLDLOCK) to the SELECT will do the trick. The SERIALIZABLE transaction isolation level would seems to work as well since it denies anyone else to read what you did until the tran is over (UPDATE: this is false. See Martin's answer). Is that true? Will they both work equally well? Is one preferred over the other?

Imagine doing something more legitimate than an ID update--some calculation based on a read that you need to update. There could be many tables involved, some of which you'll write to and others you won't. What is the best practice here?

Having written this question, I think the lock hints are better because then you are only locking the tables you need, but I'd appreciate anyone's input.

P.S. And no, I don't know the best answer and really do want to get a better understanding! :)

Answer 1

Just addressing the SERIALIZABLE isolation level aspect. Yes this will work but with deadlock risk.

Two transactions will both be able to read the row concurrently. They will not block each other as they will either take an object S lock or index RangeS-S locks dependant on table structure and these locks are compatible. But they will block each other when attempting to acquire the locks needed for the update (object IX lock or index RangeS-U respectively) which will lead to deadlock.

The use of an explicit UPDLOCK hint instead will serialize the reads thus avoiding the deadlock risk.

Answer 2

I think the best approach for you would be to actually expose your module to high concurrency and see for yourself. Sometimes UPDLOCK alone is enough, and there is no need for HOLDLOCK. Sometimes sp_getapplock works out very well. I would not make any blanket statement here - sometimes adding one more index, trigger, or indexed view changes the outcome. We need to stress test code and see for ourselves on case by case basis.

I have written several examples of stress testing here

Edit: for better knowledge of the internals, you can read Kalen Delaney's books. However, books may get out of sync just like any other documentation. Besides, there are too many combinations to consider: six isolation levels, many types of locks, clustered/nonclustered indexes and who knows what else. That is a lot of combinations. On top of that, SQL Server is closed source, so we cannot download source code, debug it and such - that would be the ultimate source of knowledge. Anything else may be incomplete or outdated after the next release or service pack.

So, you should not decide what works for your system without your own stress testing. Whatever you have read, it can help you in understanding what is going on, but you have to prove that the advice you have read works for you. I don't think anybody can do it for you.

Answer 3

In this particular case the addition of a UPDLOCK lock to the SELECT would indeed prevent anomalies. The addition of HOLDLOCK isn't necessary as an update lock is held for the duration of the transaction, but I confess to including it myself as a (possibly bad) habit in the past.

Imagine doing something more legitimate than an ID update, some calculation based on a read that you need to update. There could be many tables involved, some of which you'll write to and others you won't. What is the best practice here?

There is no best practice. Your choice of concurrency control must be based on the application's requirements. Some applications/transactions need to execute as if they had exclusive ownership of the database, avoiding anomalies and inaccuracy at all costs. Other applications/transactions can tolerate some degree of interference from each other.

• Retrieving a banded stock level (<5, 10+, 50+, 100+) for a product in a web store = dirty read (inaccurate doesn't matter).
• Checking and reducing stock level on that web store checkout = repeatable read (we MUST have the stock before we sell, we MUST not end up with a negative stock level).
• Moving cash between my current and savings account at the bank = serializable (don't miscalculate or misplace my cash!).

Edit: @AlexKuznetsov's comment prompted my re-reading the question and removing the very obvious error in my answer. Note to self on late night posting.