Stored procedure dirty read?

Question

I have a stored procedure that selects an integer, which is used as a sequence number.  The stored procedure then updates the table with an incremented value.  I'm noticing, however, that the stored procedure occasionally returns the same value twice, which corrupts the sequence of numbers.  My theory is that 1 instance of the sp is selecting before a previous one has finished updating.  Any insight on how to prevent this?

USE [BizTalk_ICS_EDI]
GO
/****** Object:  StoredProcedure [dbo].[usp_GetEDI844HDRSeqNum]    Script Date: 03/06/2012 13:07:58 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

ALTER PROCEDURE [dbo].[usp_GetEDI844HDRSeqNum]

AS

SET NOCOUNT ON

BEGIN TRANSACTION EDI844HDRSeqNum;

SELECT EDI844HDRSeqNum FROM dbo.EDIKey;

UPDATE dbo.EDIKey SET EDI844HDRSeqNum = EDI844HDRSeqNum + 1;

COMMIT TRANSACTION EDI844HDRSeqNum;

Answer 1

Try reversing the statement

USE [BizTalk_ICS_EDI]
GO
/****** Object:  StoredProcedure [dbo].[usp_GetEDI844HDRSeqNum]    Script Date: 03/06/2012 13:07:58 ******/
SET ANSI_NULLS ON
GO
SET QUOTED_IDENTIFIER ON
GO

ALTER PROCEDURE [dbo].[usp_GetEDI844HDRSeqNum]

AS

SET NOCOUNT ON

BEGIN TRANSACTION EDI844HDRSeqNum;

UPDATE dbo.EDIKey SET EDI844HDRSeqNum = EDI844HDRSeqNum + 1;

SELECT EDI844HDRSeqNum - 1  FROM dbo.EDIKey;

COMMIT TRANSACTION EDI844HDRSeqNum;

Answer 2

I need to return the value from the table so that the application that calls the sp can use it, then increment the value in the table.  There are several instances of this sp running at one time, so I need to make sure that instance B is not doing a select before instance A has updated the table.

Answer 3

This sounds like exactly what you think

I think the automatic isolation level might be to 'low':
http://msdn.microsoft.com/en-us/library/ms189122.aspx

Maybe you need to manually lock it, using sp_getapplock:
http://msdn.microsoft.com/en-us/library/ms189823.aspx

However, you should add error handling that rolls back the transactrion if anythign goes wrong. otherwise you end up with a still open transaction in an error chase. Even, you might get the still open transaction again on the next connection (without knowing), wich can cause real trouble for data integrity.
SET XACT_ABORT ON might be enough for this chase. It rolls back all transactions and kills the batch if any error happens.

Could you use IDENTITY() for thsi colunm instead? It's way easier than enterting those values manually.

Answer 4

You need the proper locking to prevent races.

For example:

BEGIN TRANSACTION

SELECT EDI844HDRSeqNum
FROM dbo.EDIKey WITH (UPDLOCK, HOLDLOCK)

UPDATE dbo.EDIKey
SET EDI844HDRSeqNum = EDI844HDRSeqNum + 1

COMMIT TRANSACTION EDI844HDRSeqNum

The big disadvantage of this, is that it will serialize all calls to this stored procedure, so if several connections call this procedure at the same time, they will each have to wait for each other.

If this were to be used on a high volume database, such procedure could seriously limit the server throughput.

---

Gert-Jan

Answer 5

You might want to generally read on Concurrency problems:
http://msdn.microsoft.com/en-us/library/ms190805.aspx

You could also make the row unique, thus enforcing those on table level. But that could mean some of these calls will end in an error.

Answer 6

Frank, my solution is returning a value  "EDI844HDRSeqNum - 1" in the second statement.

Answer 7

Christopher--No, I don't think an identity column would help since I'm not doing any inserts.  I'd still have to do a select and then an update on the identity column.  I'm basically just trying to use this as a sequence number that I can assign to transactions in my application.  I don't know what the number of transactions will be so I was looking for a way to grab them on the fly.  I have tried different isolation levels with the same results.

Gert-Jan--I tried WITH(UPDLOCK, HOLDLOCK) but it did not prevent duplicate returns from the stored proc.  Example of the output I'm getting:

100
101
102
102
104
105
106
106
108
...

Answer 8

How did you called them, that they were fast enough to run into a race condition?

Asuming you only have one row, one column in that table (to hold the single int) try this:

DECLARE @EXAMPLE TABLE (ID INT );

INSERT INTO @EXAMPLE (ID) VALUES (0);

UPDATE @EXAMPLE SET ID=ID+1 OUTPUT inserted.ID;
UPDATE @EXAMPLE SET ID=ID+1 OUTPUT inserted.ID;
UPDATE @EXAMPLE SET ID=ID+1 OUTPUT inserted.ID;
UPDATE @EXAMPLE SET ID=ID+1 OUTPUT inserted.ID;

There will be no cuncurrency problem, a all is done in the same update (wich has total isolation during execution).

See output clause for information:
http://technet.microsoft.com/en-us/library/ms177564.aspx

Answer 9

The default isolation level of a transaction is Read Committed; therefore, your code is unable to block another selection while a selection/update is taking place.   Even setting the isolation level to Serializable will also be insufficient because you will block other to make an update but won't block them to read the old value.

You need to add an Update lock:

BEGIN TRANSACTION EDI844HDRSeqNum;

SELECT EDI844HDRSeqNum FROM dbo.EDIKey with (updlock);

UPDATE dbo.EDIKey SET EDI844HDRSeqNum = EDI844HDRSeqNum + 1;

COMMIT TRANSACTION EDI844HDRSeqNum;

This will block other Select statements with the same updlock until you have finished writing (committed) the current update but won't block an ordinary select statement.  As this table is to be used only for generating a sequence, this is probably only what you need but in more complex scenarios, you might have to add other locks as well.

Answer 10

Sajid13--I'm sorry, I missed that part of your statement.  I tried it and it worked.  I guess the way the locks work, the UPDATE needs to happen before the SELECT.

Thanks!

Answer 11

Gert-Jan--I tried WITH(UPDLOCK, HOLDLOCK) but it did not prevent duplicate returns from the stored proc.  Example of the output I'm getting:

Then I am wondering how you tried this!

Because if I create and populate the table like this:

create table dbo.EDIKey(EDI844HDRSeqNum int)
insert into dbo.EDIKey values(1)

And then run the code below in two different windows, then after 10 seconds the first window returns 1 and after almost 20 seconds, the second window returns 2. Are you sure you have wrapped the statements in a transaction?

BEGIN TRANSACTION

SELECT EDI844HDRSeqNum
FROM dbo.EDIKey WITH (UPDLOCK, HOLDLOCK)

waitfor delay '000:00:10'

UPDATE dbo.EDIKey
SET EDI844HDRSeqNum = EDI844HDRSeqNum + 1

COMMIT TRANSACTION

---

Gert-Jan

Answer 12

I think you wanted the value before the increment. If that is the case, use OUTPUT deleted.ID:

DECLARE @EXAMPLE TABLE (ID INT );

INSERT INTO @EXAMPLE (ID) VALUES (0);

UPDATE @EXAMPLE SET ID=ID+1 OUTPUT deleted.ID;
UPDATE @EXAMPLE SET ID=ID+1 OUTPUT deleted.ID;
UPDATE @EXAMPLE SET ID=ID+1 OUTPUT deleted.ID;
UPDATE @EXAMPLE SET ID=ID+1 OUTPUT deleted.ID;

Note that deleted and inserted have nothing to do with the commands of the same name. Those are temporary tables/variable used by DDL operations. They contain all affected rows in the status "before" and "after".
DELETE has no after and the before contains all the rows that where deleted.

If there is more than one row affected, then I have no idead how to do get the highest ID. You can't use UPDATE OUTPUT in the FROM of a select (intelli sense says yes, but execution says no).