Stored Procedure throws Violation of PRIMARY KEY constraint when called concurrently whit same parameters

Question

Stored Procedure throws Violation of PRIMARY KEY constraint when called concurrently whit same parameters

Viewing 6 posts - 31 through 35 (of 35 total)

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

giles.clapham SSCrazy Points: 2204 More actions · Answer 1

Sorry to reply to a 3 year old post but it is pertinent to a problem I've currently got.

I like Gail's article but the PK violation I'm getting is only a small part of a complex Stored Procedure, a great deal of which is in a Transaction.

If I put XLOCK, HOLDLOCK on my destination table the exclusive lock will be held for the duration of the Transaction which I can't afford to do.

 INSERT dbo.PersonWithUnviewedAlerts ( PersonID )
SELECT @PersonID
WHERE NOT EXISTS (
SELECT PersonID 
FROM dbo.PersonWithUnviewedAlerts WITH ( XLOCK, HOLDLOCK )
WHERE PersonID = @PersonID )

The same is true if I were to set Transaction Isolation Level Serializable, and in my tests it is also true if I take this code and put it in another Proc, which would be called from the main Proc.

So I've encapsulated the Insert statement within a TRY CATCH block, the catch simply allowing the Code to continue.

For clarity, the larger business logic is within a TRY CATCH where the catch rolls back the transaction and this would be a nested TRY CATCH.

BEGIN TRY

BEGIN TRAN

Main Code...

 BEGIN TRY
INSERT dbo.PersonWithUnviewedAlerts (PersonID)
SELECT @PersonID
WHERE NOT EXISTS ( SELECT PersonID FROM dbo.PersonWithUnviewedAlerts WHERE PersonID = @PersonID )
END TRY
BEGIN CATCH
PRINT 'Attempt to insert duplicate PK in dbo.PersonWithUnviewedAlerts';
END CATCH

More Code...

COMMIT TRAN

END TRY

BEGIN CATCH
IF @@TRANCOUNT > 0
ROLLBACK TRANSACTION;

END CATCH?

For MY business rules it works fine, but is there something wrong in implementing this?

I do believe in prevention rather than cure, but is the NOT EXISTS in my WHERE clause no longer required as I can let my CATCH take care of it?

Regards

Giles

Phil Parkin SSC Guru Points: 246978 More actions · Answer 2

If you're not really interested in seeing whether a duplicate insert was attempted, consider adding IGNORE_DUP_KEY = ON to your target table definition.

Your code then becomes much simpler.

The absence of evidence is not evidence of absence.
Martin Rees

You can lead a horse to water, but a pencil must be lead.
Stan Laurel

Jonathan AC Roberts SSCoach Points: 18361 More actions · Answer 3

Why are you outputting this line, whatever the error?

PRINT 'Attempt to insert duplicate PK in dbo.PersonWithUnviewedAlerts';

What if it's caught due to a different error?

giles.clapham SSCrazy Points: 2204 More actions · Answer 4

Jonathan AC Roberts, that's a very good point.

I'm currently Unit testing Phil Parkin's solution, which fits my brief and is elegant to boot.

Thanks for looking.

Giles

Jonathan AC Roberts SSCoach Points: 18361 More actions · Answer 5

giles.clapham wrote:

Jonathan AC Roberts, that's a very good point.
I'm currently Unit testing Phil Parkin's solution, which fits my brief and is elegant to boot.
Thanks for looking.
Giles

Personally I'd just leave it with the NOT EXISTS in and remove the TRY CATCH around it.

giles.clapham SSCrazy Points: 2204 More actions · Answer 6

Thanks Phil Parkin, it's worked very well.

Jonathan AC Roberts, I agree with keeping the NOT EXISTS. If I can prevent the Insert then I should and IGNORE_DUP_KEY is only ON to cope with the occasions where atomically two processes are entering at the same time.