Not dropped trigger with not existing conversation handle blocks all DML operations
Opened this issue · 8 comments
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
I do not maintain any more this Repository.
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 ?
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?
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.
Hi @pailyn @tomasfabian Have you been able to review the suggested query change, without the use of CURSOR?
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) .
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.