November 29, 2023 at 6:23 pm
Verified on latest CU 10 for SQL 2022. Confirmed works correctly on SQL 2019 (i.e. no open transaction).
When it cannot drop server because it is used in replication, the transaction count goes from 0 to 1 after the CATCH. This causes stored procedures with this code to fail, since the transaction count is unexpected.
Our workaround is to add code to check the @@TRANCOUNT, and if greater than count before calling, COMMIT TRANSACTION.
DECLARE @ServerName SYSNAME = 'yourInstance'
;
SELECT @@TRANCOUNT AS TranCount
;
BEGIN TRY
EXEC master..sp_dropserver @ServerName, 'droplogins'
;
END TRY
BEGIN CATCH
IF ERROR_NUMBER() NOT BETWEEN 20581 AND 20584 /* Cannot drop server because it is used in replication. */
THROW
END CATCH
;
SELECT @@TRANCOUNT AS TranCount
;
November 29, 2023 at 7:23 pm
I have not tested this, but any chance you have implicit transactions turned on in your instance?
If not, I'd open up a support case with Microsoft as nobody on this forum will be able to fix that for you...
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
November 29, 2023 at 7:52 pm
November 29, 2023 at 8:02 pm
I'd be a little curious if it is related to the exact process you are running (failing to drop the database due to replication) OR if it could be something that Microsoft is doing when you put in a "TRY/CATCH" statement... I wonder if MAYBE Microsoft is trying to be helpful and the TRY is starting an implicit transaction... I don't have a SQL 2022 CU10 instance to test it on (I only have up to 2017... I know I should upgrade, but it is on my to do list). OR if the catch is what is starting the implicit transaction? OR the IF or the THROW... Basically, I am curious which statement is tossing in the hidden "BEGIN TRANSACTION"... Could you do a SELECT on the trancount at the start of the TRY and the start of the CATCH and end of the catch? Just curious as to which statement is causing that to happen... I have a suspicion that it'll be the "BEGIN TRY" that is doing it so that your CATCH can roll things back and it is SQL trying to be helpful but accidentally causing potentially long running transactions...
The above is all just my opinion on what you should do.
As with all advice you find on a random internet forum - you shouldn't blindly follow it. Always test on a test server to see if there is negative side effects before making changes to live!
I recommend you NEVER run "random code" you found online on any system you care about UNLESS you understand and can verify the code OR you don't care if the code trashes your system.
November 30, 2023 at 6:02 pm
December 1, 2023 at 10:06 am
This was removed by the editor as SPAM
December 5, 2023 at 8:13 pm
Found that different behavior (2019) was "illusion". We had older version of our software on 2019, which still had a "fix" that was somehow deleted from current version. The fix was just checking for @@TRANCOUNT <> 0 and doing ROLLBACK if it was (inside CATCH, after THROW).
Still awaiting response to Microsoft Support Request.
February 9, 2024 at 11:08 pm
Tried working through Microsoft support and finally "accepted" them archiving it, basically "works as designed". Apparently, it is my fault for using TRY...CATCH and catching the error they were going to handle via checking @@error. If I called it without TRY...CATCH, sp_dropserver would have rolled back the transaction before returning. I tried to argue that they should be using TRY...CATCH internally to do the rollback instead of relying on @@error, or that they should document that this procedure should NOT be wrapped in TRY...CATCH when calling it. Oh Well!
Microsoft response:
Every step of that stored proc checks something in order of importance and handles situation where conditions are not met or encounters an error:
--------------
setting implicit transaction to off and checking tran count with an if then returning a value after raiserror
Checking to see if you have permissions - raising an error if you dont and returning a value.
Validating if @droplogins is not null and does not equal “droplogins” – if it meets these conditions it throws an error and returns a value
--Then we BEGIN TRANSACTION--
Checking and locking the server name and checking @@error if it is not equal to 0 – then rollback the transaction, raise an error, return a value.
If @droplogins is null and exists from some select statement where and or whatever conditions to detect no sysremotelogins or sysoledbusers except the default oledb-mapping – rollback the transaction, raise an error, return a value.
Check to see if a server is used by replication. If the return value is not 0 or @@error is something other than 0 – rollback the transaction and return a value
We check to see if a linked server loading into @is_linked
Check to see if this is a managed instance (cloud lifter)
Drop the server and remote/linked logins
Now we go through if/else if, commit the transaction, and return 0 for success.
-----------------
To give you an idea where the code is breaking for your usage testing for replication servers:
-- CHECK TO SEE IF THE SERVER IS USED BY REPLICATION.
if object_id('sp_MSrepl_check_server') is not null
begin
EXEC @ret = sp_MSrepl_check_server @server
if @ret <> 0 or @@error <> 0
begin
rollback tran
return 1
end
end
@@error is coming back as something other than zero or rather, sp_MSrepl_check_server is throwing an error, that is where your catch block is grabbing it – its not handled in your code which leaves the transaction as open because the code does not continue to rollback the transaction or return 1 for failure.
Consider using the return code instead since conditions are handled inside of the stored procedure.
EXEC @ret = sp_dropserver “servername” “droplogins”
If @ret <> 0
Begin
Do stuff like logging results to a table
end
This also makes It easy to drop in a loop with values reset on the loop because that stored procedure handles its own errors and provides a return value.
February 9, 2024 at 11:14 pm
Essentially Microsoft indicated it works as designed, and it was my fault for wrapping my call to sp_dropserver in TRY...CATCH. If I had not done so, the internal check of @@error would have rolled back the transaction. I tried to argue that the procedure sp_dropserver should use TRY...CATCH instead of @@error, or that it should be documented that it should not be called from inside a TRY...CATCH. Oh Well!
Microsoft Response:
Every step of that stored proc checks something in order of importance and handles situation where conditions are not met or encounters an error:
--------------
setting implicit transaction to off and checking tran count with an if then returning a value after raiserror
Checking to see if you have permissions - raising an error if you dont and returning a value.
Validating if @droplogins is not null and does not equal “droplogins” – if it meets these conditions it throws an error and returns a value
--Then we BEGIN TRANSACTION--
Checking and locking the server name and checking @@error if it is not equal to 0 – then rollback the transaction, raise an error, return a value.
If @droplogins is null and exists from some select statement where and or whatever conditions to detect no sysremotelogins or sysoledbusers except the default oledb-mapping – rollback the transaction, raise an error, return a value.
Check to see if a server is used by replication. If the return value is not 0 or @@error is something other than 0 – rollback the transaction and return a value
We check to see if a linked server loading into @is_linked
Check to see if this is a managed instance (cloud lifter)
Drop the server and remote/linked logins
Now we go through if/else if, commit the transaction, and return 0 for success.
-----------------
To give you an idea where the code is breaking for your usage testing for replication servers:
-- CHECK TO SEE IF THE SERVER IS USED BY REPLICATION.
if object_id('sp_MSrepl_check_server') is not null
begin
EXEC @ret = sp_MSrepl_check_server @server
if @ret <> 0 or @@error <> 0
begin
rollback tran
return 1
end
end
@@error is coming back as something other than zero or rather, sp_MSrepl_check_server is throwing an error, that is where your catch block is grabbing it – its not handled in your code which leaves the transaction as open because the code does not continue to rollback the transaction or return 1 for failure.
Consider using the return code instead since conditions are handled inside of the stored procedure.
EXEC @ret = sp_dropserver “servername” “droplogins”
If @ret <> 0
Begin
Do stuff like logging results to a table
end
This also makes It easy to drop in a loop with values reset on the loop because that stored procedure handles its own errors and provides a return value.
Viewing 9 posts - 1 through 8 (of 8 total)
You must be logged in to reply to this topic. Login to reply