Not dropped trigger with not existing conversation handle blocks all DML operations

[tomasfabian]

Hi, when the trigger is trying to drop itself, combined with the conversation_cursor, it "locks" all upcoming DML operations and the trigger is not dropped:

The DML operations are failing with error: _Msg 592, Level 16, State 1, Procedure tr_dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender, Line 49 [Batch Start Line 0] Cannot find object ID 910491330 in database ID 5.

Where the object id 910491330 is the above mentioned trigger. This is a highly critical issue since the whole table is unusable.

You can reproduce it with this trigger snippet:

create or ALTER TRIGGER [dbo].[tr_dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender] ON [dbo].[Template] 
WITH EXECUTE AS SELF
AFTER insert, update, delete AS 
BEGIN     
   DECLARE @schema_id INT;
        SELECT @schema_id = schema_id FROM sys.schemas WITH (NOLOCK) WHERE name = N'dbo';

        IF EXISTS (SELECT * FROM sys.triggers WITH (NOLOCK) WHERE object_id = OBJECT_ID(N'[dbo].[tr_dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender]')) DROP TRIGGER [dbo].[tr_dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender];
		--IF EXISTS (SELECT * FROM sys.service_queues WITH (NOLOCK) WHERE schema_id = @schema_id AND name = N'dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender') EXEC (N'ALTER QUEUE [dbo].[dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender] WITH ACTIVATION (STATUS = OFF)');

        DECLARE @conversation_handle UNIQUEIDENTIFIER;
        SELECT conversation_handle INTO #Conversations FROM sys.conversation_endpoints WITH (NOLOCK) WHERE far_service LIKE N'dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_%' ORDER BY is_initiator ASC;
        DECLARE conversation_cursor CURSOR FAST_FORWARD FOR SELECT conversation_handle FROM #Conversations;
        OPEN conversation_cursor;
        FETCH NEXT FROM conversation_cursor INTO @conversation_handle;
        WHILE @@FETCH_STATUS = 0 
        BEGIN
            END CONVERSATION @conversation_handle WITH CLEANUP;
            FETCH NEXT FROM conversation_cursor INTO @conversation_handle;
        END
        CLOSE conversation_cursor;
        DEALLOCATE conversation_cursor;
        DROP TABLE #Conversations;

end

Thx

[christiandelbianco]

I do not maintain any more this Repository.

[pailyn]

Hey @christiandelbianco , just wanted to by to this comment, since in the last commit to the repository, the warning in the readme file indicating the repo is no longer maintained was removed.

This issue seems to have been re-requested a few times it seems, such as issue #229.

I'm commenting on this issue because I believe @tomasfabian has a point about the existence of the cursor creating a lock.

Based on @tomasfabian's example I feel the following snippet might work.

create or ALTER TRIGGER [dbo].[tr_dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender] ON [dbo].[Template] 
WITH EXECUTE AS SELF
AFTER insert, update, delete AS 
BEGIN     
   DECLARE @schema_id INT;
        SELECT @schema_id = schema_id FROM sys.schemas WITH (NOLOCK) WHERE name = N'dbo';

        IF EXISTS (SELECT * FROM sys.triggers WITH (NOLOCK) WHERE object_id = OBJECT_ID(N'[dbo].[tr_dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender]')) DROP TRIGGER [dbo].[tr_dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender];
		--IF EXISTS (SELECT * FROM sys.service_queues WITH (NOLOCK) WHERE schema_id = @schema_id AND name = N'dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender') EXEC (N'ALTER QUEUE [dbo].[dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_Sender] WITH ACTIVATION (STATUS = OFF)');

/***************************************************************/
        DECLARE @conversation_handle UNIQUEIDENTIFIER;
        SELECT top(1) @conversation_handle = conversation_handle 
		FROM sys.conversation_endpoints WITH (NOLOCK) 
		WHERE far_service LIKE N'dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_%' 
		ORDER BY is_initiator ASC;
	 
        WHILE @@ROWCOUNT != 0 
        BEGIN
            END CONVERSATION @conversation_handle WITH CLEANUP;      
			SELECT top(1) @conversation_handle = conversation_handle 
			FROM sys.conversation_endpoints WITH (NOLOCK) 
			WHERE far_service LIKE N'dbo_Template_c953a494-68ab-44c1-a74d-acabc98a13aa_%' 
			ORDER BY is_initiator ASC;
        END
/***************************************************************/
end

All the cursor was doing in this case is going through a list of conversation handles for a and ending them, so i replaced cursor with a while loop which retrieved the top handle in the list and end the conversation before retrieving the next handle in the updated list. I don't seem to get the DML error when the trigger is modified as above.

Does this seem correct @christiandelbianco , @tomasfabian ?

[stoyanov-x]

Thanks @pailyn. We recently integrated this library in one of our products and it proved to be quite useful.

Unfortunately, we had a situation where, around the same time, for no apparent reason, 3 of 17 servers running the Service Broker queues (and, by extension, triggers) entered a fault state — it was impossible to insert new records, perhaps caused by the trigger entering a non-committed transaction (?).

We never managed to replicate this issue in an isolated environment. Eventually I noticed a performance improvement (read: issue does not happen in production) by removing all but 2 properties from the model we use to “observe” table changes.

Do you think this actually helped reduce the chance of this issue occurring?

Did you fork the code in order to make use of the adjusted stored procedure template in your piece of software?

[tomasfabian]

Hi @pailyn, @stanimirsellercloud (and of course everyone else who is interested in this topic) I would like to invite you to this conversation opened by @cbealperto08 few days ago in my repository. We can find a solution together. I can accept Pull requests and even release a NuGet package.

[pailyn]

Thanks Tomas, I'll take sometime to look at the extension you made and see about incorporating into the project i'm using it against.

On Thu, Sep 30, 2021 at 11:30 AM Tomas Fabian @.***> wrote:

Hi @pailyn https://github.com/pailyn, @stanimirsellercloud https://github.com/stanimirsellercloud I would like to invite you to this conversation https://github.com/tomasfabian/Joker/discussions/18 opened by @cbealperto08 https://github.com/cbealperto08 few days ago in my repository. We can find a solution together. I can accept Pull request and even release a NuGet package.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/christiandelbianco/monitor-table-change-with-sqltabledependency/issues/188#issuecomment-931430160, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5M4GY3RVYJEWBXYSOUPDUER7AZANCNFSM4KQKP2CA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

[stoyanov-x]

Hi @pailyn @tomasfabian Have you been able to review the suggested query change, without the use of CURSOR?

[pailyn]

I'm not sure how to make the change @tomasfabian in your sqldependency extension solution,

the modification to change the script to not use the cursor is as follows:

replace in @christiandelbianco 's ScriptDropAll string in the TableDependency.SqlClient\Resources\sqlscript.cs file the code

 PRINT N'SqlTableDependency: Ending conversations {0}.';
        SELECT conversation_handle INTO #Conversations FROM sys.conversation_endpoints WITH (NOLOCK) WHERE far_service LIKE N'{0}_%' ORDER BY is_initiator ASC;
        DECLARE conversation_cursor CURSOR FAST_FORWARD FOR SELECT conversation_handle FROM #Conversations;
        OPEN conversation_cursor;
        FETCH NEXT FROM conversation_cursor INTO @conversation_handle;
        WHILE @@FETCH_STATUS = 0 
        BEGIN
            END CONVERSATION @conversation_handle WITH CLEANUP;
            FETCH NEXT FROM conversation_cursor INTO @conversation_handle;
        END
        CLOSE conversation_cursor;
        DEALLOCATE conversation_cursor;
        DROP TABLE #Conversations;

with something like

PRINT N'SqlTableDependency: Ending conversations {0}.';
		SELECT top(1)  @conversation_handle = conversation_handle  FROM sys.conversation_endpoints WITH (NOLOCK) WHERE far_service LIKE N'{0}_%' ORDER BY is_initiator ASC;
		while @@ROWCOUNT >1
		BEGIN
			select @conversation_handle 
			END CONVERSATION @conversation_handle WITH CLEANUP;
			SSELECT top(1)  @conversation_handle = conversation_handle  FROM sys.conversation_endpoints WITH (NOLOCK) WHERE far_service LIKE N'{0}_%' ORDER BY is_initiator ASC;
		END

In @christiandelbianco 's code the ScriptDropAll seems to be part of both the table trigger and the stored proceedure for one of the service queues, while on in your sqltabledependency extension project @tomasfabian only the stored proceedure has it (from the base sqltabledependency application) .

[pailyn]

all the cursor does is iterate throught all the associated conversations and end each conversation handle.

The cursor replacement is essentially get the top associated conversation handle, drop it, and repeat until the query returns no rows.