Why is this code deleting only 1 row at a time instead of using the condition and deleting many rows?

  • /****** Object: StoredProcedure [dbo].[dbo.ServiceLog] Script Date: 07/18/2014 14:30:59 ******/

    SET ANSI_NULLS ON

    GO

    SET QUOTED_IDENTIFIER ON

    GO

    ALTER proc [dbo].[ServiceLogPurge]

    -- Purge records dbo.ServiceLog older than 3 months:

    -- Purge records in small portions to avoid locking production tables

    -- for a long time. The process takes longer, but can co-exist with

    -- normal usage of the tables.

    @KeepDays int = 90 -- Purge records older than midnight @KeepDays days ago

    as

    set nocount on

    declare @ServiceLogID int,

    @Createddatetime,

    @Errorint,

    @Messagevarchar(8000)

    -- Purge records that qualify for purging:

    declare QualifyingServiceLogID cursor

    forselect ServiceLogID

    from ServiceLog (nolock)

    where Created < convert( char(10), dateadd( day, -@KeepDays, getdate() ), 121 )

    order by ServiceLogID

    open QualifyingServiceLogID

    fetch next from QualifyingServiceLogID into @ServiceLogID

    if exists ( select 1 from ServiceLog where ServiceLogID = @ServiceLogID ) begin

    begin try

    deleteServiceLog

    whereServiceLogID = @ServiceLogID

    end try

    begin catch

    select@Error = error_number(),

    @Message = 'failed to delete ServiceLog for cleanup after the last failed execution. Error ' +

    convert( varchar, error_number() ) + ': ' + error_message()

    goto Failed

    end catch

    fetch next from QualifyingServiceLogID into @ServiceLogID

    end

    -- Error-handling:

    Failed:

    raiserror( 'ServiceLog has %s', 11, 1, @Message )

    close QualifyingServiceLogID

    deallocate QualifyingServiceLogID

    *** Getting this error below when executing the code ***

    Msg 102, Level 15, State 1, Procedure ServiceLogPurge, Line 45

    Incorrect syntax near 'Failed:'.

  • Because the cursor is set to delete one ServiceLogID at a time.

    Even after reading the comments, I'm not sure that it's a good solution, but I don't know the entire story.

    Luis C.
    General Disclaimer:
    Are you seriously taking the advice and code from someone from the internet without testing it? Do you at least understand it? Or can it easily kill your server?

    How to post data/code on a forum to get the best help: Option 1 / Option 2
  • I don't know why you would cursor through to delete a Log one record at a time.

    Why not change it to a set-based approach?

    Jason...AKA CirqueDeSQLeil
    _______________________________________________
    I have given a name to my pain...MCM SQL Server, MVP
    SQL RNNR
    Posting Performance Based Questions - Gail Shaw[/url]
    Learn Extended Events

  • delete ServiceLog

    where ServiceLogID = @ServiceLogID

    That's a single row removal, not a range. The question is why they chose this technique. It's typically a poor method unless you've got significant concurrency concerns.


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • You guys are all right, this is madness I've created here. I'm trying to avoid blocking since there are rows that will be inserting in table frequently. Can one of provide some examples to avoid blocking but at the same time delete data from this table only keeping 3 months? Set based sounds like a good approach. Thanks all.

  • davidsalazar01 (7/18/2014)


    You guys are all right, this is madness I've created here. I'm trying to avoid blocking since there are rows that will be inserting in table frequently. Can one of provide some examples to avoid blocking but at the same time delete data from this table only keeping 3 months? Set based sounds like a good approach. Thanks all.

    declare QualifyingServiceLogID cursor

    for select ServiceLogID

    from ServiceLog (nolock)

    where Created < convert( char(10), dateadd( day, -@KeepDays, getdate() ), 121 )

    order by ServiceLogID

    Is Created actually a text field? Is it indexed properly? Can we see the schema on ServiceLog?


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • Assuming this process is run once a day and you want current day plus last 90 days, give this a try:

    declare @BatchSize int = 10000,

    @CurrentDate date = getdate(),

    @KeepDays int = 90;

    while @BatchSize > 0

    begin

    delete top (@BatchSize)

    from dbo.ServiceLog

    where

    Created < dateadd(day,-(@KeepDays),@CurrentDate);

    set @BatchSize = @@ROWCOUNT;

    end

  • Lynn, curiousity, why wouldn't you set that to 4,500 rows instead of 10k to avoid table lock escalation?


    - Craig Farrell

    Never stop learning, even if it hurts. Ego bruises are practically mandatory as you learn unless you've never risked enough to make a mistake.

    For better assistance in answering your questions[/url] | Forum Netiquette
    For index/tuning help, follow these directions.[/url] |Tally Tables[/url]

    Twitter: @AnyWayDBA

  • Evil Kraig F (7/18/2014)


    Lynn, curiousity, why wouldn't you set that to 4,500 rows instead of 10k to avoid table lock escalation?

    That is really just a suggestion. The OP can set it to whatever value desired. I picked 10,000 because it was a number. It also depends on how long it takes to delete a batch. I could also have put a WAITFOR statement in the while loop to slow down the deletes some more to allow other processes to access the table as well. In addition, I have no idea how many rows of data there are in the table nor how many are added a day. Plus, if run during a period of low activity it may not even matter if there is a table lock or not.

    So, setting it to a specific value really is a "it depends" kind of thing.

  • All comments are good pointers and the sample code provided really helps a lot. I appreciate your help.

Viewing 10 posts - 1 through 9 (of 9 total)

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