For this month’s T-SQL Tuesday, I asked y’all to write about the most recent ticket or issue that you closed. (If you want to see other peoples’ posts, you can check out the comments on that invite post, or wait til next week and I’ll publish a wrap-up of everyone’s answers.)
A past client emailed me with a performance emergency. Things had been going just fine, and then out of nowhere, things suddenly slowed down.
Their DBA opened up their monitoring software and saw a huge new bottleneck on the server. Their top wait type, by far, was blocking (LCK_M_SCH_M) – but that seemed odd given that their user quantity and query volume hadn’t changed, and of course…
Nobody Had Changed Anything™
To solve the problem faster, they got me involved. We started looking at the queries involved in the blocking.
Now if you’ve ever looked at the output of sp_BlitzWho, sp_WhoIsActive, or any monitoring tool, you’ll likely remember that when stored procedures are executed, you usually see something like this:
That doesn’t mean that the stored procedure is getting created every time. That’s just how SQL Server surfaces the code that’s being run. You have to open up the contents of that proc to see what’s going on.
Normally, when you open up a proc, it’s quite lengthy, but I’ve created a short one here to illustrate the problem:
When the client’s staff glanced at that screen (with their code of course, not mine), the code was several pages long, so it was only natural that they focused on the normal queries inside the proc.
However, I pointed out a couple of key problems:
- The stored proc didn’t have a BEGIN or END, therefore
- The GRANT EXECUTE was actually part of the proc
- The permissions change was running every time the proc ran, which needed a schema modification lock in order to run – and that’s where the blocking was coming from
It turned out that someone had recently added new permissions on a bunch of procs, and as part of that, they dutifully checked the permissions change into source control. Unfortunately, the way they made that change – by tacking the permissions onto the end of each object’s deployment script – caused the blocking issue.
A lot of palms covered faces that morning, but I love problems like that because they’re unusual, hard to spot with most monitoring tools, but once uncovered, are really simple to fix. (And I’ve actually encountered that exact issue a couple times in my career!)
19 Comments. Leave new
“Nobody Had Changed Anything™”
SELECT ROUTINE_NAME, LAST_ALTERED, CREATED
FROM INFORMATION_SCHEMA.ROUTINES
WHERE ROUTINE_NAME = ‘usp_getUser’
“Uh huh”
HAHAHA, we’ve all been there. Nice.
WOW. That is a Picard-level facepalm.
I’ve always wrapped my T-SQL functions in BEGIN/END, and my Oracle Procedures as well, but this is me making a note to go back and check T-SQL Procedures to check on that!
I had no idea you could create a procedure without begin/end I am surprised SQL is able to compile the input parameters correctly..
the BEGIN / END are optional (but should be always used for better readability). It is the missing GO after the last included command that causes the error.
For the SQL server everything between the “AS” and “GO” (or end of the file) are part of the procedure code if you run a CREATE OR ALTER statement.
The simple solution is what I always do when creating a new procedure or function. Let SSMS create the “template” and then fill it in. The template has the basics and the proper begin/end/go, so you can add the permissions after the compilation “go” and not worry if you missed something.
Why hand-create a procedure or function when the template is easy?
Not everyone develops their code in SSMS, though.
thank god for the amazing souls who use a real development tool that enforces some vague degree of decent formatting and code quality. Report writers who won’t adapt to it because it is ‘what they are used to’ while they produce the most abominable code imaginable and obviously being ‘used to’ it isn’t helping them at all drive me nuts.
True dat and I’m one of those. I’m also not for doing things over and over and over again so I have my own template for creating as sproc. It has probably prevented me from doing something stupid dozens of times, I just don’t know it.
I’m curious to know how consultants bill for an escalation like that from a past client?
You quickly make them a “current” client with a consulting agreement before starting work…
Lots of times I’ve seen this happen, normally it doesn’t affect that much, but it’s quite normal when you have a team of developers who are not real developers and don’t have much clue about the difference between a production script and a normal script to run on a development environment. Thank you for exposing this problem!
Great example of “no one changed anything”! Thank you for sharing!
“no one” is the worst employee at every company.
Hi, why missing “BEGIN … END” in the stored proc is a “problem”? Aren’t they optional?
What has once been fatal was missing the ‘GO’, not the BEGIN … END.
/* courtesy of ChatGpt for the assistance ************************************/
Drop procedure if exists dbo._trash_me;
/* incorrect way to add ms_description to the procedure **********/
GO
CREATE PROCEDURE dbo._trash_me as begin select 1 end
/* add ms_description to the procedure */
exec sp_addextendedproperty @name = N’MS_Description’, @value = N’here we GO’, @level0type = N’SCHEMA’, @level0name = N’dbo’, @level1type = N’PROCEDURE’, @level1name = N’_trash_me’
;
GO
/* find the position for string ‘sp_addextendedproperty’ in the procedure definition */
select charindex(‘sp_addextendedproperty’, ROUTINE_DEFINITION) from INFORMATION_SCHEMA.ROUTINES where ROUTINE_NAME = ‘_trash_me’
/* select the sp_addextendedproperty for the procedure */
select * from sys.extended_properties where major_id = object_id(‘dbo._trash_me’) and minor_id = 0
/* correct way to add ms_description to the procedure **********/
GO
Alter PROCEDURE dbo._trash_me as begin select 1 end
/* now with go */
GO
/* add ms_description to the procedure */
exec sp_addextendedproperty @name = N’MS_Description’, @value = N’here we GO’, @level0type = N’SCHEMA’, @level0name = N’dbo’, @level1type = N’PROCEDURE’, @level1name = N’_trash_me’
GO
/* find the position for string ‘sp_addextendedproperty’ in the procedure definition */
select charindex(‘sp_addextendedproperty’, ROUTINE_DEFINITION) from INFORMATION_SCHEMA.ROUTINES where ROUTINE_NAME = ‘_trash_me’;
/* select the sp_addextendedproperty for the procedure */
select * from sys.extended_properties where major_id = object_id(‘dbo._trash_me’) and minor_id = 0
GO
Drop procedure if exists dbo._trash_me
ps also works if you remove the begin … end in the procedure definition…
…yes, according to the documentation they are optional .. so why Brant said that’s a “problem”?!
[…] Performance emergency caused by GRANT PERMISSIONS by Brent Ozar – in which someone added a GRANT to the end of some stored procedures, and the GRANT was actually running every time the proc ran. […]