Stored Procedure Running More Than Once

Question

Stored Procedure Running More Than Once

Aiden Leech

Mr or Mrs. 500

Points: 576
More actions
March 7, 2007 at 4:14 am

#172297

Hi,
This is my first post. I have a problem that I cannot explain but maybe one of you have come across before.
On occasions a stored procedure I have written is running effectively more than once, usually twice, creating two records for the same uniquely provided hashed id. When the records are created they have the exact same datetime stamp generated from GetDate() which indicates they have run at precisely the same time on the server. There is also a check for an existing basket id before calling an INSERT. If a record already contains the hashed basket id then it sets the output BasketId to the existing one. Otherwise, a new one is inserted and the scope_identity() used to set the output BasketId.
Are there any knowns problems where stored procedures are run more than once concurrently for the same values? There is only one server in this scenerio so can't be a synchronisation issue across servers. This one has really stumped me. (SP Code is at the bottom of this post)
Thanks,
Aiden
CREATE
PROCEDURE [dbo].[ST_BASKET_INSERT]
@BasketId
int output,
@HashedBasketId
varchar(40)
AS
BEGIN
SET NOCOUNT ON;
DECLARE @ExistingBasketId int
SET @ExistingBasketId = (SELECT BasketId
FROM Basket (nolock)
WHERE HashedBasketId = @HashedBasketId)
IF @ExistingBasketId IS NULL
BEGIN
INSERT INTO Basket(HashedBasketId, CreateDate)
VALUES(@HashedBasketId, GetDate())
SET @BasketId = scope_identity()
END
ELSE
BEGIN
SET @BasketId = @ExistingBasketId
END
END

Viewing 15 posts - 1 through 15 (of 18 total)

You must be logged in to reply to this topic. Login to reply

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 1

Since you have your check with(nolock) and there's no transaction, it's quite possible for two calls at the same time to not find the record and then both insert it.

If you need to ensure that doesn't happen, then you're going to need to wrap this in a transaction and change the locking method. Problem is that could drastically reduce concurrency (as you'd need to use repeatable read or serialisable isolation levels)

To see if the sp is been called twice, run profiler and catch the rpc completed (under stored procs) and SQL batch completed (under T-SQL) events. That will give you a good overview of exactly what is happening on the server.

If the hashedbasketid is supposed to be unique, why don't you add a unique constraint on the field. That way, if somehow a second insert does slip through it will cause a duplicate key error.

Is there a trigger on the Basket table?

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass

Aiden Leech Mr or Mrs. 500 Points: 576 More actions · Answer 2

Thanks for your reply GilaMonster. My initial thoughts on a fix were to add a unique constraint on that field but as the table is live and already has duplicate records I've had to dismiss this idea, but in retrospect this is what I would have done.

I would love to be able to catch it happening using profiler but I cannot replicate the double insert. I have tried "spinning out" the web app so it calls the insert stored proc loads of times, and tried locking out the table using an ordered select on the table to simulate a busy scenario, all to no avail.

Your idea of changing the locking method and wrapping up the entire sp in a transaction seems like the best thing to do now. When you say "drastically reduce concurrency" do you mean that any usage on that table will need to wait and be effectively locked until that transaction has either commited or rolled back? I hope as the sp is short it shouldn't affect performance too much.

Aiden

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 3

Can't you remove the duplicates?

Regarding profiler, can't you leave it runing for some time against the prod database? It would be best to catch the cause of the duplicate calls, rather to put a work around in place.

Read up on serialisable isolation and do some careful perfromance testing before you put the locking more suggestion into prod. It could really degrade performance, depending how often that stored proc exists.

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass

Aiden Leech Mr or Mrs. 500 Points: 576 More actions · Answer 4

Although the records are duplicates, legitimate items can still be related to the same hashed basket id via a basket id reference so I can't remove them immediately. The only way would be to update the item references so they are all under a single basket id then delete the extra rows. I didn't really want to go down this route due to the risk of updating live data, but I will consider it.

The last occurrence of duplication was at 2007-03-08 00:26:29.127 and it's now 9:40 am here; I didn't really want to leave profiler running for hours upon hours. What are the effects of doing this? I would have thought profiler would use up resources on the server. Where does it store all the results whilst it is running, local machine?

I am going to read up on serialisable isolation now

Aiden

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 5

I've before now left profiler running for a full day without issue. Just make sure you apply enough filters to get just what you're interested in (in this case, executions of this stored proc)

You can have profiler save to disk on the machine you're running it on, otherwise it stores in memory. Another route you could go it to script the trace out once you're happy with it, and run it as a server-side trace on the server. If you do that, have it write to a drive other than the data or log drives.

Profiler does have overhead, but it's not much. The least overhead is from a server-side trace saving to a local disk.

Is there any pattern in time as to when the duplications occur?

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass

Aiden Leech Mr or Mrs. 500 Points: 576 More actions · Answer 6

I have made bold an interesting discovery. The pairs labelled (n) have the same hashed basket id but not the exact same datetime stamp, but they are either almost exactly the same or extremely close to exactly 1 minute and 15 secs apart. That's 75 secs; if it was 90 secs I guess it could be some sort of timeout, do you have any ideas what 75 secs could be?

Can this still be attributed to a lack of transaction / locking? Reading about isolation levels I don't think I need to add serializable isolation as I will want my select statement to see the commited transaction so it doesn't insert a duplicate record.

Here are the times:

2007-03-08 00:26:29.127

2007-03-07 21:26:35.463

2007-03-07 20:33:27.200

2007-03-07 19:17:10.610

2007-03-07 18:16:48.683

2007-03-07 16:56:33.013

2007-03-07 12:11:56.263

2007-03-07 11:27:25.467

2007-03-07 11:16:58.203

2007-03-07 09:36:38.010

2007-03-06 20:52:22.057

2007-03-06 15:44:05.293

2007-03-06 14:26:52.510 (5)

2007-03-06 14:26:52.493 (5)

2007-03-06 14:25:57.180

2007-03-06 13:56:02.253

2007-03-06 13:07:03.080

2007-03-06 12:42:08.110

2007-03-06 11:56:20.807

2007-03-06 11:44:19.700

2007-03-06 11:37:50.543 (4) -- 1 minute and 15 secs later

2007-03-06 11:36:44.800 (3) -- 1 minute and 15 secs later

2007-03-06 11:36:35.817 (4)

2007-03-06 11:35:30.050 (3)

2007-03-06 11:26:43.613

2007-03-06 11:26:41.613

2007-03-06 10:39:42.343 (2) -- 1 minute and 15 secs later

2007-03-06 10:38:27.610 (2)

2007-03-06 10:07:12.040 (1) -- 1 minute and 15 secs later

2007-03-06 10:05:57.290 (1)

Gail Shaw SSC Guru Points: 1004504 More actions · Answer 7

I don't think I need to add serializable isolation as I will want my select statement to see the commited transaction so it doesn't insert a duplicate record.

I'm not sure what you mean here. With any isolation above read uncommited, a select will only see committed records, and will wait for locksd to be released before completing.

The serialisable is to prevent phantoms records. though, honestly, thnking about it, I'm not even sure serialisable will be enough. Might be required to do the select with xlock, as well.

This isn't somethng I've ever tried, so I'm going on theoretical knowledge, not practical.

Gail Shaw
Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

We walk in the dark places no others will enter
We stand on the bridge and no one may pass

Aiden Leech Mr or Mrs. 500 Points: 576 More actions · Answer 8

I've decided against using xlock and I'm now going to use something based on http://sqlblogcasts.com/blogs/tonyrogerson/archive/2006/6/30.aspx custom locking and http://www.4guysfromrolla.com/webtech/041906-1.shtml try and catch statements for SQL Server 2005.

I have tested using a WAITFOR of 30 secs and successfully locks this resource so a duplicate is not inserted, without putting a lock on the table or stored proc. Shame there's no BEGIN FINALLY...END FINALLY for releasing the custom lock

Thanks for your help, here's the code:

DECLARE

@Result int

BEGIN TRY

BEGIN TRAN

EXEC @Result = sp_getapplock @Resource = 'BasketInsert', @LockMode = 'Exclusive'

IF @Result NOT IN (0, 1) -- Only successful return codes

BEGIN

PRINT @Result

RAISERROR('Lock failed to acquire.', 16, 1)

END

ELSE

BEGIN

DECLARE @ExistingBasketId int

SET @ExistingBasketId = (SELECT BasketId

FROM Basket

WHERE HashedBasketId = @HashedBasketId)

--WAITFOR DELAY '00:00:30'

IF @ExistingBasketId IS NULL

BEGIN

INSERT INTO Basket(HashedBasketId, CreateDate)

VALUES(@HashedBasketId, GetDate())

SET @BasketId = scope_identity()

END

ELSE

BEGIN

SET @BasketId = @ExistingBasketId

END

EXEC @Result = sp_releaseapplock @Resource = 'BasketInsert'

END

COMMIT TRAN

END TRY

BEGIN CATCH

EXEC @Result = sp_releaseapplock @Resource = 'BasketInsert'

IF @@TRANCOUNT > 0

ROLLBACK TRAN

-- Raise an error with the details of the exception

DECLARE @ErrMsg nvarchar(4000), @ErrSeverity int

SELECT @ErrMsg = ERROR_MESSAGE(),

@ErrSeverity

= ERROR_SEVERITY()

RAISERROR(@ErrMsg, @ErrSeverity, 1)

END CATCH

Ken McKelvey SSCoach Points: 18910 More actions · Answer 9

Consistent use of application locks helps in this sort of situation.

Try something like:

SET QUOTED_IDENTIFIER ON

GO

SET ANSI_NULLS ON

GO

CREATE PROCEDURE [dbo].[ST_BASKET_INSERT]

@BasketId int OUTPUT

,@HashedBasketId varchar(40)

AS

SET NOCOUNT ON

DECLARE @RetVal int

,@ExistingBasketId int

EXEC @RetVal = sp_getapplock

@Resource = @HashedBasketId

,@LockMode = N'Exclusive'

,@LockOwner = N'Session'

,@LockTimeout = 3000

IF @RetVal < 0

BEGIN

SET @BasketId = -1

RETURN -1

END

SELECT TOP 1 -- top used just in case there are duplicates.

@ExistingBasketId = BasketId

FROM dbo.Basket

WHERE HashedBasketId = @HashedBasketId

IF @ExistingBasketId IS NULL

BEGIN

INSERT INTO Basket(HashedBasketId, CreateDate)

VALUES(@HashedBasketId, GETDATE())

SET @BasketId = SCOPE_IDENTITY()

END

ELSE

SET @BasketId = @ExistingBasketId

EXEC sp_releaseapplock

@Resource = @HashedBasketId

,@LockOwner = N'Session'

GO

Ken McKelvey SSCoach Points: 18910 More actions · Answer 10

Ken McKelvey

SSCoach

Points: 18910

March 8, 2007 at 5:17 am

#693772

Opps,

See you have already done that.

Kris Sledge SSC Rookie Points: 41 More actions · Answer 11

I've seen something similar to this with some of our code with the exception of it was just a normal SQL insert that was executing twice and it took us forever to find the cause. I may be way off in left field with this but I'm assuming based on the comment about this being a function for a basket that it's a web application. If not sorry, nothing more to see here, move along. If so, are you using an input tag type image with an onclick event. That was the the cause of our problem. The input type image is identical to the input type submit, in that it does the submit for you upon clicking the image. If you had an onclick event that submitted the form, you'd get a double submit that, in our case, appears to be running twice.

Ninja's_RGR'us SSC Guru Points: 294069 More actions · Answer 12

Wow, thanx for this one. This is really not an obvious place to look for the problem .

Aiden Leech Mr or Mrs. 500 Points: 576 More actions · Answer 13

Ken, your solution is somewhat cleaner that mine; using the hashed basket id as the unique resource label, not using a transaction and using Session as the owner, and selecting the top 1, plus it's more readable on the forum. Many thanks

Aiden Leech Mr or Mrs. 500 Points: 576 More actions · Answer 14

Ninja,

That was a fair assumption. It's asp.net 2.0 application, effectively using a type submit button with a javascript qty check onclick returning bool whether to submit or not, so it should only be submitting once. Maybe the users *cough* customers are clicking twice on the button or something.

Thanks for posting your reply as I didn't know that an image will act as a submit also.

Cheers,

Aiden

Stored Procedure Running More Than Once

Cookies on SQLServerCentral