September 17, 2010 at 11:21 pm
We have a table containing a single row, single column (bigint) “counter” value (1,2,3,4…). We created a stored proc, uspGetSetCounter, that returns the current value of the counter and increments the value of the counter in the table by 1 (ready for the next proc call.)
I setup two Query editor sessions both pointing to the database containing our single row, single column table. Each query window contained a loop that called “uspGetSetCounter” 50,000 times. I began running the loop in the first query editor then immediately started the loop in the second query window. I found that repeated values of “counter” were sometimes returned in the output window using this approach. I also found that by surrounding the “GetSetCounter” logic in uspGetSetCounter with “begin transaction….commit” that the problem disappeared. (In other words, each call to the proc returned an incremented single value. This was the behavior we wanted.)
Why does adding the begin transaction… logic fix the issue? (Does the first call, in a transaction, block all other calls until the first call is finished?)
TIA,
Barkingdog
September 18, 2010 at 12:04 am
For starters, you have to understand that everything is a transaction, just you usually don't see it.
SELECT * from Foo... is a transaction, as is UPDATE Foo SET Bar = 'Rhesus Monkey'.
These are implicit transactions. The server does it for you on a per statement level. It makes sure that while you're reading (unless you're in dirty read mode), your data won't change mid-read. Think of having a large table that you're accessing 2-3 times in a query, and something changes at the end between read 2 on index1 and read 3 on index8. Your numbers go screwy.
So, you end up with locks (or latches, let's not go that deep). These locks tell the server if you're sharing the data, or if you have exclusive access for now. Usually, when you read, you share the lock with others. When you write/change/remove, you get exclusivity. Now, for exclusivity, it has to wait until everyone else is done.
So far so good? Excellent. Next part:
By declaring the transaction directly, you're carrying all your locks until you commit (or rollback, that's how you release a transaction if you don't want to carry the changes) the transaction. At the committal, you release the locks, the log file says it's a good transaction (is now repeatable and saved in case of crash, log backups full/bulk recovery), and something else can use the data.
This is usually done when you are changing data in multiple places but it's all related. Think entering in a row in a sales record, then updating the invoice total in another table. Without both correllating, you've got a mess. So, you wrap it entirely in a transaction. Either it all goes in, or none of it.
That's transactions, explicit and implicit. Hopefully that helps.
Now, to more technical matters. I'm very surprised you didn't end up in a deadlock scenario with your endless looping. What happens in a deadlock (usually, there are other cases) is two or more connections have a transaction that will get a shared read lock on some data. Now, one of them goes to update, and has to wait for the first one to release its share. Since the second one is now also waiting to update, it's waiting on the first one.
That's a deadlock. How you managed to avoid it is surprising. I would recommend, in your proc on the first select, to use the hint TABLOCKX, since the only data stored here is the one counter.
What this will do is lock the data out from ANY other process until that one's done with it. So be aware, this isn't something you want to do without being sure of why you're doing it. You use it for very specific cases (mass table updates that need simultaneous switchover, etc) where you want one, and only one process, to have access to the data while it runs. In the case of your counter table, this is appropriate as long as it's the only counter existing. There are tighter levels of exclusive declaration available, depending on requirements, and they're listed in BooksOnline if you want more detail.
So, finally, one question... why the counter table? CREATE TABLE FooBar (FooBarID INT IDENTITY( 1, 1), <tabledef>) should work just as well. If it's a matter of needing to know the ID for multiple inserts, look up the SCOPE_IDENTITY() function. It's much cleaner that way.
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 18, 2010 at 5:02 am
nice explanation, Craig;
Lowell
September 19, 2010 at 5:07 pm
Craig,
You wrote
>>> CREATE TABLE FooBar (FooBarID INT IDENTITY( 1, 1), <tabledef>) should work just as well. If it's a matter of needing to know the ID for multiple inserts, look up the SCOPE_IDENTITY() function. It's much cleaner that way.
If I understand your suggestion, won't the FOOBAR table grow in record count (1,2,3,4,....) endlessly? By updating a single row table, no new records are introduced. It will always be a single row.
Barkingdog
September 19, 2010 at 5:12 pm
You're absolutely correct, and it may be that I've made a poor assumption. Let's take one extra step back. What are you using this counter for? I had assumed it was for record identities.
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 19, 2010 at 6:39 pm
Barkingdog (9/19/2010)
Craig,You wrote
>>> CREATE TABLE FooBar (FooBarID INT IDENTITY( 1, 1), <tabledef>) should work just as well. If it's a matter of needing to know the ID for multiple inserts, look up the SCOPE_IDENTITY() function. It's much cleaner that way.
If I understand your suggestion, won't the FOOBAR table grow in record count (1,2,3,4,....) endlessly? By updating a single row table, no new records are introduced. It will always be a single row.
Barkingdog
That's called a "sequence" table and it's a huge source of deadlocks for most folks. One of the companies that did such a thing hired me because they were getting an average of 640 deadlocks per day with spikes to 4000. Those deadlocks each day were caused by bad code on the sequence table (they called it NextID). This is definitely not something to handle casually. You really shouldn't be using anything other than an IDENTITY column to do this type of numbering in SQL Server but, if the company gods will it, you have to do it absolutely the correct way.
Please post the code you're using for this so far and please post the CREATE TABLE statement including all indexes and the data included (see the first link in my signature below for how to properly post the data... don't assume you know how, please). From that, I'll be able to show you the right way to use a sequence table for 1 or many rows of insertion and we'll do it without building in a deadlock hotspot.
--Jeff Moden
Change is inevitable... Change for the better is not.
September 19, 2010 at 6:42 pm
I'm not sure (CRS), but I think Paul White may have written an article on this subject. I'll see if I can find it.
--Jeff Moden
Change is inevitable... Change for the better is not.
September 19, 2010 at 6:52 pm
Dang... I reviewed an article for someone on Sequence tables and now I can't find it.
At any rate, the WORST thing you could possibly do is use a BEGIN TRANSACTION on this problem. Post the stuff I asked for and I'll show you how to do it the right way.
And, no... I'm not just guessing here. I've gone through this type of thing with a bunch of folks and I know how to fix this the right way. 😉
--Jeff Moden
Change is inevitable... Change for the better is not.
September 19, 2010 at 7:18 pm
Jeff Moden (9/19/2010)
Dang... I reviewed an article for someone on Sequence tables and now I can't find it.At any rate, the WORST thing you could possibly do is use a BEGIN TRANSACTION on this problem. Post the stuff I asked for and I'll show you how to do it the right way.
And, no... I'm not just guessing here. I've gone through this type of thing with a bunch of folks and I know how to fix this the right way. 😉
Alright, you've got ME curious. Besides that, the sequence table, afaik, is basically this (no indexing)
CREATE TABLE seqNum ( NextID BIGINT) ON [PRIMARY]
GO
INSERT INTO seqNum (NextID) VALUES ( 23912)
GO
CREATE PROC RetreiveSeqnum AS
BEGIN TRAN
Declare @return BIGINT
SELECT @return= NextID FROM seqNum (TABLOCKX)
UPDATE seqNum SET NextID = NextID + 1
COMMIT TRAN
return @return
GO
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 19, 2010 at 8:05 pm
It's the combination of SELECT/UPDATE inside of an explicit transaction that produces the deadlocks just like at the company I was telling you about. If the following is kept outside of any transaction, I guarantee no deadlocks on this piece of code...
CREATE PROCEDURE dbo.GetNextID
@NextID INT OUTPUT
AS
IF @Increment > 0
UPDATE nid
SET @NextID = NextID = NextID + 1
FROM dbo.NextID nid
SELECT @NextID = @NextID - 1
RETURN
Of course, that doesn't show how to return a programmable increment of ID's. That's partially done in the proc and partially outside the proc. Inside the proc would look like the following (which will also handle more than one table's worth of ID's)...
CREATE PROCEDURE dbo.GetNextID
@TableName SYSNAME,
@Increment INT = 1,
@NextKey INT OUTPUT
AS
IF @Increment > 0
BEGIN
UPDATE dbo.NextID
SET @NextID = NextID = @NextID + @Increment
WHERE TableName = @TableName
SELECT @NextID = @NextID - @Increment
RETURN
END
SELECT @NextID = NULL
RETURN
Of course, we can get very sophisticated with all this and still not cause any deadlocks. Here's what I wrote for that company a while back... again, this updates a sequence table known as NextID and it carries the "next ID" for almost every table in the database... and, no... they didn't want it to automatically make a new entry for new tables which could easily be done without any additional risk of deadlocks... 😉
CREATE PROCEDURE dbo.GetNextID
/****************************************************************************************
Purpose:
This stored procedure is used to get a NextID for the table identified by the @KeyID
parameter. It will "reserve" a block of ID's according to the @Increment parameter.
The @NextID returned is always the first ID of a reserved block of numbers. The reserved
block size defaults to 1.
Usage:
EXEC @return = dbo.GetNextID @KeyID, @Increment, @NextID=@NextID OUTPUT
Outputs:
1. Returns a -1 if error and 0 if success.
2. @NextID will be a -1 if an error occured. Otherwise, it will contain the first
NextID of the requested block of NextID's.
Notes:
1. This stored procedure replaces the original stored procedure provided a 3rd party.
2. This procedure has been enhanced compared to the original...
a. The UPDATE statement sets both the @NextID variable and the NextID column in the
NextID table eliminating the need for a separate SELECT from NextID after the
UPDATE. This virtually eliminates deadlocks associated with this "hot spot".
b. Because of (2.b) above, there is no longer a need for a transaction. If the
UPDATE didn't work, there is no need for a ROLLBACK because nothing was updated.
c. Previous error handling did not correctly return the invalid KeyID if present.
d. A test has been added to ensure a negative value for @Increment was not
passed in.
e. A test to ensure that @NextID was correctly updated has been added.
f. Repairs to the previous error routines have been made so that the values returned
to @@ERROR and @@ROWCOUNT are correctly used by more than one statement.
Revisions:
Rev 01 - 03/01/2005 - Jeff Moden - Initial creation and unit test.
Note: Method first suggested to me by Kalpa Shah.
****************************************************************************************/
--=======================================================================================
-- Define the I/O parameters used by this procedure
--=======================================================================================
--===== Declare the passed parameters
@KeyID INT, --Identifies table to get the NextID for
@Increment INT = 1, --Number of NextIDs to "reserve"
@NextID INT OUTPUT --Returns start # of block of IDs
AS
--=======================================================================================
-- Main body of procedure
--=======================================================================================
--===== Suppress auto-display of row counts for appearance and speed
SET NOCOUNT ON
--===== Declare local variables
DECLARE@MyError INTEGER --Holds @@ERROR for additional processing
DECLARE @ErrMessage VARCHAR(100) --Holds calculated error messages because RaisError
--cannot calulate messages on the fly.
DECLARE @MyRowCount INTEGER --Hold @@ROWCOUNT for additional processing
--===== Preset @NextID to an error condition
SET @NextID = -1 --Defaults don't work consistently on OUTPUT parameters
--===== If the increment is not greater than zero, raise and error and exit immediately
IF @Increment <= 0
BEGIN --Start of error processing
--===== Process errors (RaisError cannot do calcs for error message)
SET @ErrMessage = 'The NextID row could not be updated. '
+ 'Increment was set to '
+ CONVERT(VARCHAR(11),@Increment) + '.'
RAISERROR (@ErrMessage,1,1)
RETURN -1 --Returns an error indication to calling procedure
END --End of error processing
--===== Update the correct NextID row according to the KeyID passed in.
-- Sets @NextID and the column to the previous value + the increment
-- simultaneously so we don't need to read from the NextID table to
-- get the value of @NextID in the following steps.
UPDATE NextID WITH (UPDLOCK)
SET @NextID = NextID = NextID + @Increment
WHERE KeyID = @KeyID
-- Get the error value and rowcount
SELECT @MyError = @@ERROR, @MyRowCount = @@ROWCOUNT
--===== Check for errors, a rowcount of 1, and a non-default value for @NextID
IF @MyError <> 0 --An error did occur
OR @MyRowCount <> 1 --The row was not updated
OR @NextID = -1 --A new value for @NextID was not returned
BEGIN
--===== Process errors (RaisError cannot do calcs for error message)
IF @MyError <> 0 --Error occured
SET @ErrMessage = 'The NextID row could not be updated.'
ELSE --either 1 row or @NextID was not updated
SET @ErrMessage = 'The NextID row could not be updated. KeyID '
+ CONVERT(VARCHAR(11),@KeyID)
+ ' may not exist.'
RAISERROR (@ErrMessage,1,1)
RETURN -1 --Returns an error indication to calling procedure
END --End of error processing
--===== Calculate and return the first number in the block of reserved NextID's
-- to the @NextID output parameter
SELECT @NextID = @NextID - @Increment
--===== Return a "success" indication to the calling procedure
RETURN 0
GO
--Jeff Moden
Change is inevitable... Change for the better is not.
September 19, 2010 at 8:12 pm
Jeff,
My solution is the same as yours except I used:
SELECT @return= NextID FROM seqNum with (UPDLOCK)
Given the table has only a single record I think the solutions are equivalent. But in a way the itnent seems clearer using TABLOCK instead of UPDLOCK.
Barkingdog
September 19, 2010 at 8:14 pm
Ah, that makes sense, you're avoiding the necessity of the double call by using the running total method that's undocumented.
Btw, if you force the tablockx, you won't get deadlocks, either. You might get a heck of a wait cycle for the proper locks if this gets called at a rediculous volume, but you'll avoid deadlocking.
That's a neat piece of code ya got there though... Look, butterflies! (Swipes it, and makes off like a thief)
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 19, 2010 at 8:15 pm
Ah... to be sure... the "Kalpa Shah" that I refer to in the revision history was one of the two Systems DBA's I was working with on this problem. Kalpa remembered that there was a 3 part update (not to be confused with a "quirky update") documented in BOL. She was right so I always give her credit for it especially since I've used it a couple of billion times now. 😀
--Jeff Moden
Change is inevitable... Change for the better is not.
September 19, 2010 at 8:15 pm
Barkingdog (9/19/2010)
Jeff,My solution is the same as yours except I used:
SELECT @return= NextID FROM seqNum with (UPDLOCK)
Actually, that was me. Jeff's solution is much more elegant.
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
September 19, 2010 at 8:17 pm
Jeff Moden (9/19/2010)
Ah... to be sure... the "Kalpa Shah" that I refer to in the revision history was one of the two Systems DBA's I was working with on this problem. Kalpa remembered that there was a 3 part update (not to be confused with a "quirky update") documented in BOL. She was right so I always give her credit for it especially since I've used it a couple of billion times now. 😀
Actually, the 3 part update that you mention actually probably is documented. I always think of it as undocumented because MS won't support carrying the value between rows, which is primarily where I used that method.
Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.
For better assistance in answering your questions[/url] | Forum Netiquette
For index/tuning help, follow these directions.[/url] |Tally Tables[/url]
Twitter: @AnyWayDBA
Viewing 15 posts - 1 through 15 (of 24 total)
You must be logged in to reply to this topic. Login to reply