Trigger causing performance issues

  • Hello

    We have been facing locking/blocking issues with our application and after some monitoring, tracing activity, I have found that the following trigger (Please see code below) seems to be causing majority of the pb -

    The table tblTest has 48 million rows

    It has clustered index on id col and non-clustered indexes on serial_fk, test_fk , sequence cols

    I am guessing the best thing would be to eliminate or rewrite the trigger and not use CURSORs.

    But, a couple of questions here are -

    On reviewing the output of syslockinfo,.sp_lock, i see that the UPDATE from this trigger is causing multiple page-level IX locks and escalated to a table level X lock. Is this occuring because of the CURSOR or the UPDATE ?

    What are some of the suggestions to improve performance in this scenario? Any feedback is appreciated .. Thanks

    Code:

    CREATE TRIGGER [update_test_status] ON [dbo].[tblTest]

    AFTER INSERT

    AS

    declare @id int

    declare @Sequence int

    declare @serial_fk int

    declare @test_fk int

    declare @f_status int

    declare testdata_cur CURSOR LOCAL FOR

    select [id],test_fk,serial_fk,[sequence] from inserted where status=0

    OPEN testdata_cur

    FETCH NEXT FROM testdata_cur

    INTO @id,@test_fk,@serial_fk,@sequence

    set @f_status=@@FETCH_STATUS

    while @f_status=0

    begin

    update tblTest set status=1 where id<>@ID and serial_fk=@serial_fk and test_fk=@Test_fk and sequence=@Sequence

    FETCH NEXT FROM testdata_cur

    INTO @id,@test_fk,@serial_fk,@sequence

    set @f_status=@@FETCH_STATUS

    end

    CLOSE testdata_cur

    DEALLOCATE testdata_cur

  • 1st suggestion - rewrite the trigger and remove the cursor.

    2nd suggestion - same as the first. 😉

    What's the idea behind the trigger? What are you trying to do when the table gets updated?

    Gail Shaw
    Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
    SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

    We walk in the dark places no others will enter
    We stand on the bridge and no one may pass
  • Kill the cursor - no doubt about that. multiple updates launched against a 48M row table, should more than likely be avoided.

    Also - looks to me that you'd end up with a REALLY interest result if you had multiple rows with matching serial_fk, test_FK and sequence with a status of 0 in the same batch of inserted rows. Actually not 100% sure what would happen, except that it looks like NO record would end up with a status of 0. I'm assuming you're trying to maintain a unique "active" or some such status by that set of column values - is this the best way to do that?

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • I am the DBA working with the application team on this, but from my understanding .. the intent is to update the status on a bunch of test results , but retain the the old test results in the table with the old status .. something along those lines ..

    I agree the CURSOR needs to go .. but one clarification ,

    Is it the CURSOR that is causing the page and table locks or the UPDATE ..

    I see that the UPDATE is not going to be able to use the index because of the <> in the WHERE clause correct?

  • aruram (1/30/2008)


    I am the DBA working with the application team on this, but from my understanding .. the intent is to update the status on a bunch of test results , but retain the the old test results in the table with the old status .. something along those lines ..

    OK, then maybe something like this (untested and quick. NB, test carefully!)

    Update tblTest Set Status = 1 from

    tbltest inner join inserted i on tblTest.ID<>i.id AND tblTest.serial_fk=i.serial_fk and tblTest.test_fk=i.Test_fk and tblTest.sequence=i.Sequence

    where i.status = 0

    I agree the CURSOR needs to go .. but one clarification ,

    Is it the CURSOR that is causing the page and table locks or the UPDATE ..

    Probably not causing them, but certainly prolonging them. The transaction started by the initial update lasts until the trigger completes, which means none of the locks will be released until the trigger completes

    I see that the UPDATE is not going to be able to use the index because of the <> in the WHERE clause correct?

    Not necessarily. Inequality doesn't by itself prevent index seeke. From a quick look, a good index for that update will be

    serial_fk, test_fk, sequence, id (ID last because of the inequality, all others are equality.)

    Again, test that. I haven't tried any of this code out.

    Gail Shaw
    Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
    SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

    We walk in the dark places no others will enter
    We stand on the bridge and no one may pass
  • Well - it's likely not going to use the clustered index to do seeks because of the <>. That being said - you may get it to use a non-clustered one, especially if you made it a compound index on all three "extra" fields. You might need to figure out which of the three is most selective, so you can make it the "left edge" of your index.

    As to the business process aspect - keeping that status there is technically de-normalizing your data, since you're now storing the status of a given testing "batch" within the detail records, and not in a single place. I'd consider moving that out to a "testing batch table", which would remove the need for this trigger altogether. Of course - there are a lot of other reasons why you might not want to do that, so that's a choice to consider.

    As long as every row in a given "batch" has the same sequence number per test_fk+serial_fk combination, you could be safe I suppose.

    finally - if the status is a "binary valued field", then I'd consider not updating every record that's already of status=1 all over again. Meaning - add "and status=0" to my update criteria...

    ----------------------------------------------------------------------------------
    Your lack of planning does not constitute an emergency on my part...unless you're my manager...or a director and above...or a really loud-spoken end-user..All right - what was my emergency again?

  • That was certainly helpful .. Thanks !

    Another qn - Is there a way to tune or force the UPDATE to use row/key locks instead of page/table locks? .. and if so, is that advisable to do? as SQL by default handles locking mechanisms appropriately

    But I am concerned that it is holding an X lock on the table for time period significant enough to block other processes from accessing it.

  • Yes and no.

    I prefer not to force row locks unless I know that only a very small number of rows are going to be updated. Preferably tune the trigger so that it doesn't hold the locks for very long, and let SQL handle the lock escalation as necessary.

    Gail Shaw
    Microsoft Certified Master: SQL Server, MVP, M.Sc (Comp Sci)
    SQL In The Wild: Discussions on DB performance with occasional diversions into recoverability

    We walk in the dark places no others will enter
    We stand on the bridge and no one may pass
  • I would start not from performance but from incorrect logic of the trigger.

    If 2 or more lines are inserted than ALL rows in the table with serial_fk=@serial_fk and test_fk=@Test_fk and sequence=@Sequence get updated to Status = 1.

    Is it what you expect to get?

    _____________
    Code for TallyGenerator

  • Here is an article talking about inserting large amount of data into a table that has a trigger would cause performance problem. Also there is an example not using cursor in the trigger.

    http://searchsqlserver.techtarget.com/general/0,295582,sid87_gci1186129,00.html

    http://searchsqlserver.techtarget.com/general/0,295582,sid87_gci1186130,00.html

    Hope it helps.

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

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