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

  • VoldemarG

    Hall of Fame

    Points: 3549

    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.

     

     


    Voldemar
    likes to play chess

  • VoldemarG

    Hall of Fame

    Points: 3549

    Sorry. PDF file not allowed.

    Attaching as WORD.

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


    Voldemar
    likes to play chess

  • ScottPletcher

    SSC Guru

    Points: 98285

    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) Prosecutor James Blackburn, in closing argument in the Fatal Vision murders trial: "If in the future, you should cry a tear, cry one for them [the murder victims]. If in the future, you should say a prayer, say one for them. And if in the future, you should light a candle, light one for them."

  • frederico_fonseca

    SSChampion

    Points: 14297

    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

  • Grant Fritchey

    SSC Guru

    Points: 395842

    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

    The Scary DBA
    Author of: SQL Server 2017 Query Performance Tuning, 5th Edition and SQL Server Execution Plans, 3rd Edition
    Product Evangelist for Red Gate Software

Viewing 5 posts - 1 through 5 (of 5 total)

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