asked on

When to wrap sql statements in a Transaction

Not sure when Transaction should be used in a stored proc. I've never used a Transaction.
We have a stored proc that deadlocks and it's in a Transaction.

The reason the developer has it in a transaction is because it selects and updates some tables. He said a transaction was used because results of one table depends on the other one. For example, if table1 gets updated, table2 will use whatever was updated from table1.

But i dont think a transaction is needed for this. Also, can I use "nolock" for update statements?? Should the stored proc be broken into several stored procs and separate the insert/update?

>>Also, can I use "nolock" for update statements??
No, not on the table you're updating, unless you're reading from it as well.

>>Should the stored proc be broken into several stored procs and separate the insert/update
It depends on what you're trying to accomplish in the proc. Post it on here and we'll look at it. It may just depend on how everything is setup though....it may be required...maybe not.

ASKER

it's a long one..but here it is...I dont think some of the RollBacks are needed ..for example:
IF NOT EXISTS(....)
BEGIN
      SET @RESULT = 3
      ROLLBACK
      RETURN @RESULT
END

But here's the stored proc:

-- these are being passed in:
@OLDLOC varchar (10),
@NEWLOC varchar (10),
@SKU varchar (20),
@MOVEQTY int,
@USERPID varchar (25),
@STARTDATE varchar(25),
@ENDDATE VARCHAR(25),
@RESULT int OUTPUT
 
---- 
DECLARE @CLASS varchar (4)
DECLARE @ACTQTY int
DECLARE @PID varchar (20)
 
SET @CLASS = '01'
SET @PID = 'MUO-00205'
 
/* VALIDATE VARIABLES */
IF NOT EXISTS(SELECT * FROM Location l WITH (NOLOCK) WHERE l.Loc_ID = '0001010501')
BEGIN
	select 3
	--ROLLBACK
	--RETURN @RESULT
END
IF NOT EXISTS(SELECT * FROM Location l WITH (NOLOCK) WHERE l.Loc_ID = @NEWLOC)
BEGIN
	SET @RESULT = 3
	--ROLLBACK
	RETURN @RESULT
END
IF @OLDLOC = @NEWLOC
BEGIN
	SET @RESULT = 2
	--ROLLBACK
	RETURN @RESULT
END
 
 
 
 
/* GET AVAILABLE AND ACTUAL QTY FROM LOCATION */
SELECT @ACTQTY = (
           SELECT sum (Actual_Qty)
           FROM   MU_Sku ms WITH (NOLOCK)
           WHERE  ms.mu_id = @OLDLOC
             AND  ms.sku = @SKU
       )
 
IF @MOVEQTY <= @ACTQTY
BEGIN
    
/********** PERFORM MOVE *************/
 
/* INSERT OR UPDATE ITEM INTO MU_SKU */
IF EXISTS (
          SELECT *
          FROM   mu_sku ms WITH (NOLOCK)
          WHERE  ms.MU_ID = @NEWLOC
            AND  ms.SKU = @SKU
       )
    BEGIN
        UPDATE mu_sku
        SET    Actual_Qty = Actual_Qty + @MOVEQTY,
               Update_User_ID = @USERPID,
               Update_PID = @PID
        FROM   mu_sku 
        WHERE  MU_ID = @NEWLOC
          AND  sku = @SKU
          
             IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error updating MU_SKU',16, 1)
            RETURN
        END
    END
    ELSE
    BEGIN
        INSERT INTO MU_SKU
        (
            MU_ID, SKU, Actual_Qty, Env_Code, Class, Receiver_Nbr, PO_Nbr,
            Receipt_Date, Update_User_ID, Update_PID
        )
        SELECT @NEWLOC AS MU_ID,
               SKU,
               @MOVEQTY AS Actual_Qty,
               Env_Code,
               Class,
               Receiver_Nbr,
               PO_Nbr,
               Receipt_Date,
               @USERPID AS Update_User_ID,
               @PID AS Update_PID
        FROM   mu_sku WITH (NOLOCK)
        WHERE  MU_ID = @OLDLOC
          AND  sku = @SKU
          
             IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error inserting MU_SKU',16, 1)
            RETURN
        END
    END
    
    
 
   
 
/* INSERT OR UPDATE ITEM INTO LOC_ALLOCATION */
IF EXISTS (
          SELECT *
          FROM   Loc_Allocation la WITH (NOLOCK)
          WHERE  la.Loc_ID = @NEWLOC
            AND  la.Class = @CLASS
            AND  la.SKU = @SKU
       )
    BEGIN
        UPDATE Loc_Allocation
        SET    Actual_Qty = Actual_Qty + @MOVEQTY,
               Update_User_ID = @USERPID,
               Update_PID = @PID
        WHERE loc_id = @NEWLOC AND Class = @CLASS AND SKU = @SKU
        
             IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error updating Loc_Allocation',16, 1)
            RETURN
        END
        
    END
    ELSE
    BEGIN
        INSERT INTO Loc_Allocation
        (
            Loc_ID, SKU, Class, Actual_Qty, Update_User_ID, Update_PID
        )
        SELECT @NEWLOC AS Loc_ID,
               SKU,
               la.Class,
               @MOVEQTY AS Actual_Qty,
               @USERPID AS Update_User_ID,
               @PID AS Update_PID
        FROM   Loc_Allocation la WITH (NOLOCK)
        WHERE  la.Loc_ID = @OLDLOC
          AND  la.SKU = @SKU
          
            IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error inserting Loc_Allocation',16, 1)
            RETURN
        END
    END
 
/* INSERT RECORD INTO UP_INVENT */
INSERT INTO [DC01NBW].[dbo].[UP_Invent]
           ([Upload_Flag]
           ,[Message_Type]
           ,[Timestamp]
           ,[User_ID]
           ,[Mode]
           ,[Facility_Nbr]
           ,[Location]
           ,[MU_ID]
           ,[SKU]
           ,[Class]
           ,[Qty]
           ,[Inv_Function]
           ,[Reason_Code]
           ,[Comments]
           ,[Lot]
           ,[Serial_Nbr]
           ,[Exp_Date]
           ,[Mfg_Date]
           ,[Vendor_ID]
           ,[Function_ID]
           ,[Update_User_ID]
           ,[Update_PID])
     VALUES
           ('N'
           ,'0410'
           ,getdate()
           ,@USERPID
           ,'C'
           ,'01'
           ,@NEWLOC
           ,@NEWLOC
           ,@SKU
           ,@CLASS
           ,@MOVEQTY
           ,'MUO_CONS'
           ,''
           ,''
           ,''
           ,''
           ,'1/1/1970'
           ,'1/1/1970'
           ,''
           ,'MUO-00205'
           ,@USERPID
           ,'MUO-00205')
           
             IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error inserting Up_Invent',16, 1)
            RETURN
        END
 
 
/* UPDATE MOVEABLE UNIT TABLE */
 
IF EXISTS (
          SELECT *
          FROM   Moveable_Unit mu WITH (NOLOCK)
          WHERE  mu.Loc_ID = @NEWLOC
       )
    BEGIN
        UPDATE [DC01NBW].[dbo].[Moveable_Unit]
   SET [MU_ID] = @NEWLOC
      ,[MU_Type] = 'LOC'
      ,[MU_Status] = 'INVNT'
      ,[Class] = @CLASS
      ,[Receiver_Nbr] = ''
      ,[Loc_ID] = @NEWLOC
      ,[ParentMU_ID] = @NEWLOC
      ,[ParentMU_Type] = 'L'
      ,[Shippable] = 'N'
      ,[Breakable] = 'N'
      ,[Counted] = 'N'
      ,[User_ID] = ''
      ,[Hot_Flag] = 'N'
      ,[Storage_Date] = getdate()
      ,[SkidCartons] = 0
      ,[SkidPieces] = 0
      ,[Update_Date] = getdate()
      ,[Update_User_ID] = @USERPID
      ,[Update_PID] = @PID
 WHERE loc_id = @NEWLOC
   IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error updating Moveable Unit',16, 1)
            RETURN
        END
    END
    ELSE
    BEGIN
       INSERT INTO [DC01NBW].[dbo].[Moveable_Unit]
           ([MU_ID]
           ,[MU_Type]
           ,[MU_Status]
           ,[Class]
           ,[Receiver_Nbr]
           ,[Loc_ID]
           ,[ParentMU_ID]
           ,[ParentMU_Type]
           ,[Shippable]
           ,[Breakable]
           ,[Counted]
           ,[User_ID]
           ,[Hot_Flag]
           ,[Storage_Date]
           ,[SkidCartons]
           ,[SkidPieces]
           ,[Update_Date]
           ,[Update_User_ID]
           ,[Update_PID])
     VALUES
           (@NEWLOC
      ,'LOC'
      ,'INVNT'
      ,@CLASS
      ,''
      ,@NEWLOC
      ,@NEWLOC
      ,'L'
      ,'N'
      ,'N'
      ,'N'
      ,''
      ,'N'
      ,getdate()
      ,0
      ,0
      ,getdate()
      ,@USERPID
      ,@PID)
      
        IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error inserting Moveable Unit',16, 1)
            RETURN
        END
      
    END
 
 
    
/*  UPDATE OR DELETE MU_SKU */IF @MOVEQTY = @ACTQTY
    BEGIN
        DELETE mu_sku
        WHERE  MU_ID = @OLDLOC
          AND  SKU = @SKU
          
            IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error removing mu_sku',16, 1)
            RETURN
        END
    END
    ELSE
    BEGIN
        UPDATE MU_SKU
        SET    Actual_Qty = Actual_Qty - @MOVEQTY,
               Update_User_ID = @USERPID,
               Update_PID = @PID
        WHERE  MU_ID = @OLDLOC
          AND  SKU = @SKU
          
           IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error removing mu_sku',16, 1)
            RETURN
        END
    END
 
 
/*  UPDATE OR DELETE LOC_ALLOCATION */IF @MOVEQTY = @ACTQTY
    BEGIN
        DELETE loc_allocation
        WHERE  Loc_ID = @OLDLOC
          AND  sku = @SKU
          AND Class = @CLASS
          
           IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error remvoing Loc_Allocation',16, 1)
            RETURN
        END
    END
    ELSE
    BEGIN
        UPDATE Loc_Allocation
        SET    Actual_Qty = Actual_Qty - @MOVEQTY,
               Update_User_ID = @USERPID,
               Update_PID = @PID
        WHERE loc_id = @OLDLOC AND sku = @SKU AND Class = @CLASS
        
         
        IF @@ERROR <> 0
        BEGIN
            -- Rollback the transaction
            ROLLBACK
            
            -- Raise an error and return
            RAISERROR ('Error remvoing Loc_Allocation',16, 1)
            RETURN
        END
        
    END
    
    /*UPDATE OR DELETE MOVEABLE_UNIT*/
    
    
    
    
    
 
/* INSERT RECORD INTO UP_INVENT */
INSERT INTO [DC01NBW].[dbo].[UP_Invent]
           ([Upload_Flag]
           ,[Message_Type]
           ,[Timestamp]
           ,[User_ID]
           ,[Mode]
           ,[Facility_Nbr]
           ,[Location]
           ,[MU_ID]
           ,[SKU]
           ,[Class]
           ,[Qty]
           ,[Inv_Function]
           ,[Reason_Code]
           ,[Comments]
           ,[Lot]
           ,[Serial_Nbr]
           ,[Exp_Date]
           ,[Mfg_Date]
           ,[Vendor_ID]
           ,[Function_ID]
           ,[Update_User_ID]
           ,[Update_PID])
     VALUES
           ('N'
           ,'0410'
           ,getdate()
           ,@USERPID
           ,'C'
           ,'01'
           ,@OLDLOC
           ,@OLDLOC
           ,@SKU
           ,@CLASS
           ,@MOVEQTY * -1
           ,'MUO_CONS'
           ,''
           ,''
           ,''
           ,''
           ,'1/1/1970'
           ,'1/1/1970'
           ,''
           ,'MUO-00205'
           ,@USERPID
           ,'MUO-00205')
           
            IF @@ERROR <> 0
    BEGIN
        -- Rollback the transaction
        ROLLBACK
        
        -- Raise an error and return
        RAISERROR ('Error decrementing Up_Invent',16, 1)
        RETURN
    END
 
    
/*  UPDATE LOCATION */
UPDATE Location
    SET    Update_User_ID = @USERPID,
           Update_PID = @PID,
           Update_Date = getdate ()
    WHERE  Loc_ID = @OLDLOC
    
     IF @@ERROR <> 0
    BEGIN
        -- Rollback the transaction
        ROLLBACK
        
        -- Raise an error and return
        RAISERROR ('Error updating old location',16, 1)
        RETURN
    END
 
    UPDATE Location
    SET    Update_User_ID = @USERPID,
           Update_PID = @PID,
           Update_Date = getdate ()
    WHERE  Loc_ID = @NEWLOC
     IF @@ERROR <> 0
    BEGIN
        -- Rollback the transaction
        ROLLBACK
        
        -- Raise an error and return
        RAISERROR ('Error updating new location',16, 1)
        RETURN
    END
    
/* ADD TO PRODUCTIVITY */
INSERT INTO Productivity
    (
        Start_Date, End_Date, User_ID, Activity, Qty, Unit_Desc, Upload_Flag,
        Update_User_ID, Update_PID
    )
VALUES
    (
        @STARTDATE, @ENDDATE, @USERPID, 'MU Consolidation', @MOVEQTY, 'Piece',
        'N', @USERPID, 'MUO-00006'
    )
    
     IF @@ERROR <> 0
    BEGIN
        -- Rollback the transaction
        ROLLBACK
        
        -- Raise an error and return
        RAISERROR ('Error updating productivity',16, 1)
        RETURN
    END
    
    COMMIT
SET @RESULT = 0
    RETURN @RESULT
END
ELSE
BEGIN
 	ROLLBACK
SET @RESULT = 1
  RETURN @RESULT
END

Open in new window

ASKER

There's Begin Transaction in the beginning and Commit at the end.

ASKER CERTIFIED SOLUTION

membership

Create a free account to see this answer

Signing up is free and takes 30 seconds. No credit card required.

ASKER

i think it should be inside a transaction because as you said :everything does happen, or nothing happens.

Let me ask you this..looking at it..
1.do you think i should put nolock on the insert and updates as well?
2. Should I remove the unnecessary rollbacks...like from "not exists" section
3. would this be better as separate stored procs..call the first one , then second, then third..then forth..with all in a transaction?

Do you have adequate indexes on all of the fields where criteria is set...some searches could be spending a long amount of time trying to find rows while other records are locked...you know what I mean?

ASKER

ah, let me check.

ASKER

I checked the "where" clause of the sql statements in there and there are indexes on the fields.

My manager said this SP gets called every 5 to 10 seconds from different processes. Adding the nolock has helped but he still sees deadlocks..he said he sees this as "victim" in the deadlock.
I havent myself monitored it. I'm just looking at it to see if there's somewhere I can tune it...

so breaking it into several SPs wont do much?

ASKER

yeah, i see index-seek for one of the select statements.. will check the rest...

ASKER

For example, in here, I shouldnt have a "nolock" for the "update"

IF EXISTS (
SELECT *
FROM ... ms WITH (NOLOCK)
WHERE .... )
BEGIN
UPDATE mu_sku
SET ... FROM ... WHERE MU_ID = @NEWLOC
AND sku = @SKU

ASKER

no, i cant use nolock on update statements:

https://www.experts-exchange.com/questions/22806129/nolock-on-update-statements.html

right (see my first coment)..can't use nolock on updates

putting the statements into their own stored procs probably will not help...as the transaction would still encompass the entire unit of work.

did you try the sqldiag tool?

ASKER

going to try the sqldiag now.

ASKER

I get buffer overrun error when running the command. It pops an error msg. Displays buffer overrun. I click ok on it..then in the command prompt..i see "connecting to <database>" and list of error log text files.

ASKER

but it did create the sqldiag.txt. But it created it on D drive. I ran that sqldiag on C drive. I see MSSQL folders both on c and d drive.

ASKER

yeah, sqldiag.txt shows today's date and I see stored proc's name in there. It has line numbers too. Is it accurate..the line numbers...does it actually pinpoint where the deadlock happened??

ASKER

no silly me, that sqldiag.txt is someone else's text. I create mine in the c drive like the article:
C:\Program Files\Microsoft SQL Server\MSSQL\Binn\SQLDiag E O C:\SQLDiagOutput.txt

but i get an error as I mentioned above.

ASKER

** There's only ONE commit in that stored proc. Shouldnt there be a commit after each update/insert??

ASKER

I'm looking at the errorlog file.This also lists the deadlocks. Not sure why i get that error above when i run sqldiag. This gets me going for now at least

No, not really. The commit at the end commits the ENTIRE transaction to the db. If you want to just commit the individual updates, then strip out all of the transaction logic....

ASKER

I know why sqldiag gives me an error. this is sql2000. Not 2005. sqldiag is only for sql2005 seems like it.

SOLUTION

membership

Create a free account to see this answer

Signing up is free and takes 30 seconds. No credit card required.

ASKER

Thanks, knew that about nolock. Let me look at your link. I might try rowlock in there.

ASKER

SQL's errorlog points to 2 lines. Not sure if these are actual lines or estimates.