is WITH (ROWLOCK) hint a culprit in this code?

  • In the attached Stored Procedure, the yellow-highlighted code causes major blocking, especially when the SP gets executed 50 times at the same time or even more times.

    When low contention/concurrency times of the day happen, everything works fine. Exec this SP takes a second.

    But during highest concurrency periods happen, the entire .aspx web page from which this SP is called is HANGING and timing out after 3 minutes of freeze.

    Database Transaction Isolation level = Read Committed.

    My analysis suggests that the WITH (ROWLOCK) hint is the culprit, and if I just remove it, the operations will go smoothly and blocking will at least be reduced.  The table being updated in the identified culprit Update statement (highlighted)  has only a few thousand rows, no more than 10 000.

    This is a Prod issue, and I greatly appreciate your input.

     

     

    Likes to play Chess

  • Sorry. PDF file not allowed.

    Attaching as WORD.

    Attachments:
    You must be logged in to view attached files.

    Likes to play Chess

  • Shouldn't matter, unless it's part of a (much) larger transaction.  A rowlock (at least) will be required to do the UPDATE anyway.  But you could remove it, just in case.

    Much more likely to resolve the issue would be to make sure the WHERE clause can be satisfied by an index and that it doesn't end up resulting in a table/index scan.

    SQL DBA,SQL Server MVP(07, 08, 09) A socialist is someone who will give you the shirt off *someone else's* back.

  • code here instead of word doc - easier for most to see

    one thing i would change is the fact that the code, if the record does not exist already, does an insert followed by an update of the record just created - record should be created with the required values without the need for the update to happen

     

     
    /* DECLARE @oGID int

    EXEC [prGetGidv2]
    @chrObjectName = 'DAN_TESTV2'
    ,@relGidValue = @oGID OUTPUT
    ,@QtyNeeded = 100

    SELECT @oGID
    */
    ALTER PROCEDURE [dbo].[prGetGid]
    (@chrObjectName VARCHAR(30),
    @relGidValue DECIMAL(12,0) OUTPUT,
    @QtyNeeded DECIMAL(12,0) = 1)
    AS
    DECLARE
    @l_today DATETIME,
    @rowcount INT,
    @retry BIT

    BEGIN

    SET NOCOUNT ON

    SET @l_today = GETDATE()
    SET @relGidValue = -1
    SET @retry = 0

    BEGIN TRY
    /*<DocGen_Description>Ensure to validate the object name. If object name is blank or null then raise the error
    message: "Object Name is blank. Contact product support." </DocGen_Description>*/
    IF (RTRIM(@chrObjectName) = '' OR @chrObjectName = NULL)
    BEGIN
    RAISERROR ('Object Name is blank. Contact product support.', 16, 1);
    END;
    /*<DocGen_Description>Ensure to validate the quantity needed. If quantity needed is less than 1 or null then raise the error
    message: "Quantity needed is less than 1. Contact product support."</DocGen_Description>*/
    IF (@QtyNeeded < 1 OR @QtyNeeded = NULL)
    BEGIN
    RAISERROR ('Quantity needed is less than 1. Contact product support.', 16, 1);
    END

    Retry:

    UPDATE
    GID_Values WITH(ROWLOCK)
    SET
    gid_value = gid_value + @QtyNeeded,
    last_touch = @l_today,
    @relGidValue = gid_value + 1
    WHERE
    [object_name] = RTRIM(@chrObjectName)

    SET @rowcount = @@ROWCOUNT

    IF @rowcount = 0 AND @retry = 0
    BEGIN
    BEGIN TRY
    INSERT INTO GID_Values ([object_name], gid_value, last_touch)
    VALUES (RTRIM(@chrObjectName), 0, @l_today)

    SET @retry = 1

    GOTO Retry
    END TRY
    BEGIN CATCH
    /*<DocGen_Description>In case error in creating unique identifier for entity then raise the error message: "Error Creating an Unique Identifier For Entity."
    </DocGen_Description>*/
    RAISERROR ('Error Creating an Unique Identifier For Entity. ', 16, 1)
    END CATCH;
    END
    /*<DocGen_Description>If no records added then raise the error message: "Error Getting Unique Identifier For Entity After Retry."
    </DocGen_Description>*/
    ELSE IF @rowcount = 0
    RAISERROR ('Error Getting Unique Identifier For Entity After Retry. ', 16, 1)

    END TRY
    BEGIN CATCH
    IF (XACT_STATE()) <> 0
    BEGIN
    ROLLBACK TRANSACTION;
    END;
    DECLARE @ErrorMessage NVARCHAR(4000);
    DECLARE @ErrorSeverity INT;
    DECLARE @ErrorState INT;

    SELECT
    @ErrorMessage = ERROR_MESSAGE(),
    @ErrorSeverity = ERROR_SEVERITY(),
    @ErrorState = ERROR_STATE();

    RAISERROR (@ErrorMessage, @ErrorSeverity, @ErrorState);

    END CATCH;

    /*</DocGen_Tool>*/
    RETURN
    END

  • Query hints should absolutely be the exception, not the rule. So, I'm always inclined to remove them. However, using Chesterton's Fence as a model, we need to ask, why is it there in the first place? I suspect it's an attempt to deal with deadlocks. I'd pull it, but, let's talk about that.

    In testing, you'll pull it. Performance will either, stay the same, or increase some. Then, you'll hit production. Whatever inspired this to be put in place is due to a production level load, lots of transactions all at once. Deadlocks were occurring, probably because of classic coding issues, two different modification paths to the data. A rowlock ensure that no one could accidently take out locks, but now, acts as a blocker. So, you need to plan to identify and fix the core deadlock issue when you put this change in place.

    That's just a guess though.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • Thanks.

    But how exactly do you suggest it can be re-written to do that?

    Likes to play Chess

  • I can't say without knowing it's actually hitting deadlocks. Then, it's about the deadlocks encountered. Sorry, no way to know what that is from where I'm sitting.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • Sorry I should have mentioned it. yes, it occasionally hits deadlocks.

    Many times it is just blocking timeouts, a few times here and there there are deadlocks.

    Likes to play Chess

  • when the proc is called 100 times a minute or more, that's when deadlocks start. at something like 50 hits a minute or less, no deadlocks, only occasional blocking chains, caused by same SP against same SP calls.

    index is working well.

     

    Likes to play Chess

  • From what I can see, the current code tries an update.  If there's nothing to update, it does an insert and then returns to try the same update again.  That makes no sense to me and should probably be fixed.

    Also, instead of WITH(ROWLOCK), try WITH(UPDLOCK) instead.  I have no scientific explanation for why I make that suggestion.  I can only tell you that I saw it resolve a deadlock issue nearly a decade ago.

    --Jeff Moden


    RBAR is pronounced "ree-bar" and is a "Modenism" for Row-By-Agonizing-Row.
    First step towards the paradigm shift of writing Set Based code:
    ________Stop thinking about what you want to do to a ROW... think, instead, of what you want to do to a COLUMN.

    Change is inevitable... Change for the better is not.


    Helpful Links:
    How to post code problems
    How to Post Performance Problems
    Create a Tally Function (fnTally)

  • And, deadlocks are fundamentally a performance tuning issue, so look at the exec plan to be sure there are no tuning opportunities. Then, determine what the deadlocks are exactly from and see if it's due to the classic deadly embrace or something else. Possibly rearranging the order of operations between two different queries could help with the deadlocks. Eliminating or mitigating the deadlocks w/o the UPDLOCK or ROWLOCK will reduce blocking and timeouts. However, it's all going to require a lot of documentation and testing to arrive at a happy place.

    "The credit belongs to the man who is actually in the arena, whose face is marred by dust and sweat and blood"
    - Theodore Roosevelt

    Author of:
    SQL Server Execution Plans
    SQL Server Query Performance Tuning

  • Should I just replace  Update-->Insert-->Update (retry) with IF EXISTS   [update] ELSE [insert] ?

    Likes to play Chess

  • Also, attached is the doc. that shows table structure, and a few typical calls to this SP (some of them yes, do happen within large(r) transactions), fired 100 times a minute, called from hundreds of other SPs.

    Attachments:
    You must be logged in to view attached files.

    Likes to play Chess

  • Could it be that the SP is called by long running processes (transactions) that are updating the same row (with the same [object_name]) and one of the transactions hasn't finished while another process is calling the SP to update the same object_name?

  • yes, I believe it can be, since at certain high contention times this SP is called 100 times a minute.

    But once your statement is confirmed, what can possibly be done to remediate such timeouts/blocks/deadlocks ?

    This SP is the most called SP among 10000 SPs that we have in this database. It is in the core of each business transacion, and any db operatoin.

    Should removing (ROWLOCK) hint and changing Isolation Level from Read Committed inside this SP to  Read Uncommitted be tried? But in such case, wouldn't it greately violate the integrity of business operations and rules?

     

    Likes to play Chess

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

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