-
Notifications
You must be signed in to change notification settings - Fork 993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
improved performance #3394
improved performance #3394
Conversation
Update sp_BlitzIndex.sql with #h and #os to improve speed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed hardcoded database name
Unsubscribe
…On Mon, Nov 13, 2023, 2:50 PM Henrik Staun Poulsen ***@***.***> wrote:
***@***.**** commented on this pull request.
fixed hardcoded database name
—
Reply to this email directly, view it on GitHub
<#3394 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACJBXIKYOU7ZWCSMOYI443DYEKB2FAVCNFSM6AAAAAA7JHADV6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMRYGI2TMNZQGA>
.
You are receiving this because you are subscribed to this thread.Message
ID:
<BrentOzarULTD/SQL-Server-First-Responder-Kit/pull/3394/review/1728256700@
github.com>
|
sp_BlitzIndex.sql
Outdated
@@ -1435,40 +1442,100 @@ BEGIN TRY | |||
|
|||
--NOTE: If you want to use the newer syntax for 2012+, you'll have to change 2147483647 to 11 on line ~819 | |||
--This change was made because on a table with lots of paritions, the OUTER APPLY was crazy slow. | |||
SET @dsql = N'SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED; | |||
DROP TABLE if exists #h |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hate to say this, but what is the #h table named for? Can we use a more descriptive name? I'm thinking of the people who have to come in behind you (cough, me, cough) and read and interpret this over time to maintain it. Same with the #os table. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#h for Henrik?
#os for Other Stuff?
There are 2 things that are hard in computing; cache invalidation, naming things and off-by-one errors.
I'll think of a better name, and find a smallish (15 TB) database to test the result set; before and after.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use more descriptive names for #h and #os? I haven't yet tested it to make sure it returns the right results, but conceptually I like what you're doing here, I just need to test it after I know what those tables are for. Thanks!
STOP emailing me
…On Thu, Dec 21, 2023 at 2:51 PM Henrik Staun Poulsen < ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In sp_BlitzIndex.sql
<#3394 (comment)>
:
> @@ -1435,40 +1442,100 @@ BEGIN TRY
--NOTE: If you want to use the newer syntax for 2012+, you'll have to change 2147483647 to 11 on line ~819
--This change was made because on a table with lots of paritions, the OUTER APPLY was crazy slow.
- SET @Dsql = N'SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
+DROP TABLE if exists #h
#h for Henrik?
#os for Other Stuff?
There are 2 things that are hard in computing; cache invalidation, naming
things and off-by-one errors.
I'll think of a better name, and find a smallish (15 TB) database to test
the result set; before and after.
—
Reply to this email directly, view it on GitHub
<#3394 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ACJBXIPEV6PWBANS4VKTLM3YKSOMVAVCNFSM6AAAAAA7JHADV6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOJTGY2TQMBUGY>
.
You are receiving this because you commented.Message ID:
<BrentOzarULTD/SQL-Server-First-Responder-Kit/pull/3394/review/1793658046@
github.com>
|
@pydss you set yourself to watch the entire Github repo. You'll need to un-watch the repo - none of us can do that for you. Go to http:/firstresponderkit.org and click Unwatch at the top of the screen. |
Named the new tables #dm_db_partition_stats_etc and #dm_db_index_operational_stats Tested on a smaller db (27TB) where the new version runs in 3 seconds, and the old 11 seconds. Same output. I'll see if I can get a result set from the old version on the larger database, now that there are a couple of holidays. Previously I have given up after 11 hours. My new version ran in 3 hours.
For reference, this fails on a client server I'm currently working on. It's a 2016 server which might be the issue,
Error Message;
|
@RichBenner ah yeah, good catch. It looks like in this pull request, @HenrikStaunPoulsen is grabbing ALL of the columns from dm_db_index_operational_stats, rather than just the ones we need, and those newer columns aren't available in all versions. I've got some time this morning, so lemme see if I can change that in his pull request. I think it's possible to edit somebody else's pull request, I just haven't done it recently. |
Removed columns we don't need from prior versions.
@RichBenner; good catch. I should have thought about that. Sorry. |
Confirm this now works on the 2016 instance I'm testing it on. As an aside, the current version of sp_BlitzIndex has been running for almost 6 days on the server and is yet to complete, this new version completed in just under 5 minutes and has output 96k rows of information into the output table. This is using the @mode = 4, @GetAllDatabases = 1 parameters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates! Looks good, works on my machine, merging into the dev branch, will be in the next release with credit to you in the release notes.
@HenrikStaunPoulsen you'll get a chuckle out of this: the query hint min_grant_percent was added in a later update to SQL Server 2012 & 2014. It wasn't in RTM, so sp_BlitzIndex is failing on older versions. |
improved performance from 11+ hours to less than 3 hours on my clients database, by writing data to two #temp tables, and reporting of those.