Trigger causing deadlocks

  • We have a trigger which is causing increasingly frequent deadlocks to occur at a particular time of day. The trigger is responsible for ensuring that only the latest record is marked as "active". Please see the details below. I have also pasted modified definitions of the tables involved.

    The xml_deadlock_report (using the system_health XE) is showing the deadlock to be being caused by two or more occurrences of the trigger blocking itself:

    - waitresource = PAGE

    - transactionname = "UPDATE"

    - isolationlevel = "read committed (2)"

    Lock modes were initially (S), with attempts to escalate to (IX) and (SIX).

    Can anyone please suggest how we can improve this to prevent the blocking/deadlocks from occurring?

    The tables currently contain the following numbers of rows:

    - Records = 121019 rows (1,699MB)

    - Records_Detail_1 = 136055 rows (29MB)

    - Records_Detail_2 = 346470 rows (16MB)

    CREATE TABLE dbo.Records (

    RecordID int IDENTITY(1,1) NOT NULL,

    OtherID int NOT NULL,

    Active bit NOT NULL,

    OutboundXML xml NULL,

    InboundXML xml NULL,

    Reference uniqueidentifier NULL,

    UID int NULL,

    UTimeStamp datetime NULL,

    CONSTRAINT PK_Records PRIMARY KEY CLUSTERED (RecordID)

    )

    GO

    CREATE NONCLUSTERED INDEX idx_Reference ON dbo.Records (Reference)

    GO

    CREATE TABLE dbo.Records_Detail_1 (

    RecordID int NOT NULL,

    Detail1ID int NOT NULL,

    Reference uniqueidentifier NOT NULL,

    URL nvarchar(255) NULL,

    UID int NULL,

    UTimeStamp datetime NULL,

    CONSTRAINT PK_Records_Detail_1 PRIMARY KEY CLUSTERED (RecordID, Detail1ID),

    CONSTRAINT FK_RD1_RecordID FOREIGN KEY(RecordID) REFERENCES dbo.Records (RecordID)

    )

    GO

    CREATE TABLE dbo.Records_Detail_2 (

    RecordID int NOT NULL,

    Detail2ID int NOT NULL,

    Reference uniqueidentifier NOT NULL,

    UID int NULL,

    UStamp datetime NULL,

    CONSTRAINT PK_Records_Detail_2 PRIMARY KEY CLUSTERED (RecordID, Detail2ID),

    CONSTRAINT FK_RD2_RecordID FOREIGN KEY(RecordID) REFERENCES dbo.Records (RecordID)

    )

    GO

    CREATE TRIGGER dbo.trg_MakeLatestRecordActive ON dbo.Records

    FOR INSERT, UPDATE

    AS

    SET NOCOUNT OFF

    IF EXISTS

    (

    SELECT i.RecordID

    FROM inserted i

    INNER JOIN dbo.Records R ON R.OtherID = i.OtherID AND R.RecordID <> i.RecordID AND R.Active = 1

    LEFT OUTER JOIN Deleted d ON d.RecordID = i.RecordID

    WHERE i.Active = 1

    AND ISNULL(d.Active, 0) = 0

    )

    UPDATE R

    SET Active = 0

    FROM inserted i

    INNER JOIN dbo.Records R ON R.OtherID = i.OtherID AND R.RecordID <> i.RecordID AND R.Active = 1

    LEFT OUTER JOIN Deleted d ON d.RecordID = i.RecordID

    WHERE i.Active = 1

    AND ISNULL(d.Active, 0) = 0

    GO

  • You may need an additional index or two to help speed up those queries.

    I'd consider adding one on the Records table (OtherId, RecordId, ACTIVE).

    The absence of evidence is not evidence of absence
    - Martin Rees
    The absence of consumable DDL, sample data and desired results is, however, evidence of the absence of my response
    - Phil Parkin

  • Will an index not just potentially slow things down further?

    Can you see any way in which this trigger can be improved? This seems to be our sole (and frequent) source of deadlocks!

  • zoggling (11/21/2016)


    Will an index not just potentially slow things down further?

    Can you see any way in which this trigger can be improved? This seems to be our sole (and frequent) source of deadlocks!

    Can you add it in a test environment and check? I would expect an improvement, but these things need to be tested.

    The absence of evidence is not evidence of absence
    - Martin Rees
    The absence of consumable DDL, sample data and desired results is, however, evidence of the absence of my response
    - Phil Parkin

  • Unfortately the test environments are not exhibiting the same symptoms, and neither have they been provisioned with comparable resources. This makes it more difficult to reproduce.

    Is there another approach we can take?

  • Consider changing your design. I prefer to not persist this type of value in my data. A view would be a great way to expose the 'IsActive' value and keep it current without having to rely on trigger logic to constantly 'mark' a row as most recent.

    John Rowan

    ======================================================
    ======================================================
    Forum Etiquette: How to post data/code on a forum to get the best help[/url] - by Jeff Moden

  • John Rowan (11/21/2016)


    Consider changing your design. I prefer to not persist this type of value in my data. A view would be a great way to expose the 'IsActive' value and keep it current without having to rely on trigger logic to constantly 'mark' a row as most recent.

    Could you please confirm how this would work? Would you update data via the view, or just use the view to display the active record using logic accordingly?

  • zoggling (11/21/2016)


    John Rowan (11/21/2016)


    Consider changing your design. I prefer to not persist this type of value in my data. A view would be a great way to expose the 'IsActive' value and keep it current without having to rely on trigger logic to constantly 'mark' a row as most recent.

    Could you please confirm how this would work? Would you update data via the view, or just use the view to display the active record using logic accordingly?

    The latter. No specific updates would be required.

    The absence of evidence is not evidence of absence
    - Martin Rees
    The absence of consumable DDL, sample data and desired results is, however, evidence of the absence of my response
    - Phil Parkin

  • I guess this trigger would do what you want to do:

    CREATE TRIGGER dbo.trg_MakeLatestRecordActive ON dbo.Records

    FOR INSERT, UPDATE

    AS

    UPDATE R

    SET Active = CASE WHEN R.RecordID = i.LastRecordID THEN 1 ELSE 0 END

    FROM (SELECT OtherID, MAX(RecordID) LastRecordID

    FROM inserted

    GROUP BY OtherID

    ) i

    INNER JOIN dbo.Records R ON R.OtherID = i.OtherID

    And I'd suggest adding an index on (OtherID, RecordID).

    Depending on the load and most typical queries - probably to make it clustered.

    _____________
    Code for TallyGenerator

  • Sergiy (11/21/2016)


    I guess this trigger would do what you want to do:

    CREATE TRIGGER dbo.trg_MakeLatestRecordActive ON dbo.Records

    FOR INSERT, UPDATE

    AS

    UPDATE R

    SET Active = CASE WHEN R.RecordID = i.LastRecordID THEN 1 ELSE 0 END

    FROM (SELECT OtherID, MAX(RecordID) LastRecordID

    FROM inserted

    GROUP BY OtherID

    ) i

    INNER JOIN dbo.Records R ON R.OtherID = i.OtherID

    And I'd suggest adding an index on (OtherID, RecordID).

    Depending on the load and most typical queries - probably to make it clustered.

    The estimated execution plan only shows marginal gains for this trigger definition when compared with the original, and also suggests a similar index be created. We will re-consider the index option.

    We have also discovered that another scheduled process is spiking the processor queue length and % processor time on all of the logical processors around the times these deadlocks are occurring, so this may be exacerbating the issue.

    Thank you all for your help.

  • The logic for UPDATE action is not so clear to me.

    What outcome is expected after updates?

    Does "last record" mean "last changed"?

    _____________
    Code for TallyGenerator

  • zoggling (11/22/2016)


    The estimated execution plan only shows marginal gains for this trigger definition when compared with the original, and also suggests a similar index be created. We will re-consider the index option.

    If you're relying on things like % of Batch even for Actual Execution plans to determine which code is faster, you're making a very big mistake. You cannot rely on execution plans to tell you which is the best code it's an estimate. Here's a classic example. It's why a lot of people think that rCTEs that "increment" are good. Run the following code with the actual execution plan on. Decide from that which you think is the faster code. Then, look at the output in the "Messages" tab and see the truth.

    DECLARE @Bitbucket INT

    ;

    RAISERROR('--===== Reursive CTE =====',0,0) WITH NOWAIT;

    SET STATISTICS TIME,IO ON;

    WITH cte AS

    (

    SELECT N = 1

    UNION ALL

    SELECT N = N + 1

    FROM cte

    WHERE N < 1000000

    )

    SELECT @Bitbucket = N

    FROM cte

    OPTION (MAXRECURSION 0)

    ;

    SET STATISTICS TIME,IO OFF;

    PRINT REPLICATE('-',119)

    ;

    RAISERROR('--===== Pseudo-Cursor =====',0,0) WITH NOWAIT;

    SET STATISTICS TIME,IO ON;

    SELECT TOP 1000000

    @Bitbucket = ROW_NUMBER() OVER (ORDER BY (SELECT NULL))

    FROM sys.all_columns ac1

    CROSS JOIN sys.all_columns ac2

    ;

    SET STATISTICS TIME,IO OFF;

    PRINT REPLICATE('-',119);

    GO

    --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)

Viewing 12 posts - 1 through 11 (of 11 total)

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