Link to home
Start Free TrialLog in
Avatar of Allan
AllanFlag for United States of America

asked on

SQL Deadlock - SERIALIZABLE? (TABLOCKX)?

Hi Experts!

Thank you for reading this.

Trying to help out a co-worker with a deadlock issue.

It's a simple sproc that checks if a member is in a work table
if it's not then insert into the work table
if it is then delete the member from the work table and update the profile table

The deadlock seems to be on this table: MyCompany_memberReq_tbl

Here's the table for the work table:
CREATE TABLE [dbo].[MyCompany_memberReq_tbl](
	[SubscriberID] [char](25) NOT NULL,
	[LastName] [char](60) NOT NULL,
	[FirstName] [char](30) NOT NULL,
	[MiddleName] [char](30) NOT NULL,
	[DOB] [datetime] NOT NULL,
	[Gender] [char](1) NOT NULL,
	[RegisterID] [varchar](15) NULL,
	[Createdate] [datetime2](7) NULL,
	[CreateID] [char](15) NULL,
 CONSTRAINT [PK_memreq] PRIMARY KEY NONCLUSTERED 
(
	[SubscriberID] ASC,
	[LastName] ASC,
	[FirstName] ASC,
	[MiddleName] ASC,
	[DOB] ASC,
	[Gender] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
) ON [PRIMARY]

GO

SET ANSI_PADDING OFF
GO

ALTER TABLE [dbo].[MyCompany_memberReq_tbl] ADD  DEFAULT ('') FOR [SubscriberID]
GO

ALTER TABLE [dbo].[MyCompany_memberReq_tbl] ADD  DEFAULT ('') FOR [LastName]
GO

ALTER TABLE [dbo].[MyCompany_memberReq_tbl] ADD  DEFAULT ('') FOR [FirstName]
GO

ALTER TABLE [dbo].[MyCompany_memberReq_tbl] ADD  DEFAULT ('') FOR [MiddleName]
GO

ALTER TABLE [dbo].[MyCompany_memberReq_tbl] ADD  DEFAULT (getdate()) FOR [DOB]
GO

ALTER TABLE [dbo].[MyCompany_memberReq_tbl] ADD  DEFAULT ('') FOR [Gender]
GO

ALTER TABLE [dbo].[MyCompany_memberReq_tbl] ADD  DEFAULT ('') FOR [RegisterID]
GO

ALTER TABLE [dbo].[MyCompany_memberReq_tbl] ADD  DEFAULT (getdate()) FOR [Createdate]
GO

ALTER TABLE [dbo].[MyCompany_memberReq_tbl] ADD  DEFAULT (user_name()) FOR [CreateID]
GO

Open in new window

Here's the sproc:
ALTER PROCEDURE [dbo].[MyCompany_membercheck_SP](  
@subscriberid varchar(25),  
@lastname varchar(60),  
@firstname varchar(35)=NULL,  
@middlename varchar(30)=NULL,  
@dob DATETIME=NULL,  
@gender varchar(1)=NULL,  
@RegisterID varchar(30),  
@secondaryid varchar(15) output  
)  
AS  
BEGIN  
  
 DECLARE @MemberFound as char(1)  
  
 /* Begin look up DB, see if member is found */  
  IF(@gender ='U')  
  BEGIN  
   SET @gender = ' '  
  END  
  
  /* check DB, if member exists */  
  SELECT   
   @secondaryid = m.secondaryid  
  FROM   
   member m (NOLOCK) 
  WHERE  
   m.lastname = @lastname AND   
   m.firstname = @firstname AND   
   m.middlename = @middlename AND   
   m.dob = @dob AND   
   m.sex = @gender AND   
   m.carriermemid = @subscriberid  
  
 /* End look up DB, see if member is found */  
  
 /* if found, update to post member and clear out work table record, if it exists   
  if not found, secondaryID is null, look in work table and pass back Y if not in work table or N if in work table
 */  
 IF (@secondaryid IS NULL) --member not in DB yet  
 BEGIN  
		BEGIN TRANSACTION  
		/* check WT for last name match */  
		IF NOT EXISTS (  
		 select  1 from MyCompany_memberReq_tbl with (TABLOCKX) where lastname = @lastname ) -- match on last name, TABLOCKX Use an exclusive lock on a table. This lock prevents others from reading or updating the table and is held until the end of the statement or transaction.  
			 BEGIN  
					/* create work record, so other incomings can find and wait*/  
					INSERT INTO MyCompany_memberReq_tbl (lastname,firstname,middlename,dob,gender,subscriberid,RegisterID)  
					VALUES (@lastname,@firstname,@middlename,@dob,@gender,@subscriberid,@RegisterID)  
			      
					/*send back Y to indicate creation for member */  
					SET @MemberFound = 'Y'  
			 END  
		ELSE  
		 BEGIN  
			/* return N if there is no need to create the member, and wait if somone has last name */  
			SET @MemberFound = 'N'   
		 END  
	    
		COMMIT TRANSACTION  
	  
		--set secondaryID to indicate if member should be created or not, depending if another earlier transaction already did it  
		SET @secondaryid = @MemberFound  
	    
		/* update profile table to indicate process, member still not in DB */  
		UPDATE  
		 MyCompany_profile_tbl    
		SET   
		 status ='PREMEMBER'  
		WHERE   
		 RegisterID = @RegisterID  	     
 END  
 ELSE  
	 BEGIN  
			/* member found in DB, clear out work table record if it exist */  
			DELETE FROM MyCompany_memberReq_tbl  
			where lastname = @lastname  
		   
			/* update profile table to indicate process, member now in DB */  
			UPDATE   
			 MyCompany_profile_tbl    
			SET   
			 status ='POSTMEMBER',  
			 secondaryid = @secondaryid  
			WHERE   
			 RegisterID = @RegisterID  
	 END  
  
END  

Open in new window

Here are some thoughts:

1. SQL DBA said the transaction has isolation level set to SERIALIZABLE.
   Wonder how's that's possible when in the sproc it doesn't set isolation level
   Perhaps READ COMMITTED should suffice?
   
2. Maybe it's not necessary to use (TABLOCKX) on the table MyCompany_memberReq_tbl since it's a lockdown on the table?
   TABLOCKX Use an exclusive lock on a table. This lock prevents others from reading or updating the table and is held until the end of the statement or transaction.

3. Explicitlly using BEGIN TRANSACTION may not be necessary since INSERT statement already has “implicit” transaction for itself?

This is for MS SQL 2008 R2.

Any comments you've is appreciated!
ASKER CERTIFIED SOLUTION
Avatar of Marten Rune
Marten Rune
Flag of Sweden image

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
I wholeheartedly agree with Marten, and don't want to give the impression I was trying to butt in. Design is the best solution to preventing issues to begin with. Take his suggestions seriously.
And any time you are going to do anything "out of the ordinary" make sure you fully understand what you are doing it and why. Research all the ramifications. I would hope the DBA who suggested using SERIALIZABLE understood the implications and didn't suggest using it in a heavily used multi-user OLTP system.

The BEGIN TRANS/COMMIT TRANS is useless in your code. If you are going to use transactions understand how to use them. I've seen people code a BEGIN TRANSACTION on line 1, and a COMMIT TRANSACTION on line 1345, thinking that if the code made it to line 1345, everything would be committed, otherwise everything would be rolled back. It isn't an all or nothing proposition unless you code it that way, and it takes a lot more than those two lines of code. EE can provide a wealth of information, but do further research. Don't just take the 'expert's word for it. Search Google and learn as much about why you are going to do something a specific way.
SOLUTION
Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of Allan

ASKER

Thank you all for your comments; really appreciate it. Still reading and learning..
Avatar of Allan

ASKER

Thanks guys!