sp_QuickieStore: added sys.dm_db_tuning_recommendations to Expert Mode output#691
Conversation
… of closing #483.
erikdarlingdata
left a comment
There was a problem hiding this comment.
Code Review
Nice addition — surfacing sys.dm_db_tuning_recommendations in Expert Mode is a good call, and locking it behind @expert_mode = 1 is the right scope.
Three findings:
Bug: Status message text is backwards
In the OUTER APPLY, the plan_force_recommendation_status message says:
"Considered regressed from plan_id [recommended_plan_id]"
But recommended_plan_id is the better plan, not the regressed one. As written, it reads like the recommended plan is the bad one. Consider something like:
plan_force_recommendation_status =
N'Recommended plan_id ' +
CONVERT(nvarchar(20), qstr.recommended_plan_id) +
N' is scored ' +
CONVERT(nvarchar(20), qstr.score) +
N' out of 100 for being better than this plan.'Concern: SUBSTRING parsing throws on missing trailing comma
The CHARINDEX(',', ...) calls assume there's always a comma after regressedPlanId and recommendedPlanId. If either field is the last value in the flattened string (no trailing comma), CHARINDEX returns 0 and the SUBSTRING length goes negative:
-- Msg 536: Invalid length parameter passed to the SUBSTRING function.
With the current known JSON structure from Microsoft, both fields always have subsequent fields, so this works today. The risk is forward-compatibility if Microsoft changes the field ordering or removes fields in a future version. A fallback to LEN(detail) when CHARINDEX returns 0 would make it bulletproof.
Concern: OUTER APPLY runs unnecessarily without @expert_mode guard
The data collection into #tuning_recommendations is gated by @new = 1 AND @expert_mode = 1, but the OUTER APPLY is only inside the @new = 1 block. When @expert_mode = 0, the temp table is empty and the join is wasted work.
Fix: pull the OUTER APPLY out into its own block, same pattern as the wait stats CROSS APPLY above it:
) AS w'
);
END; /*End wait stats query*/
IF
(
@new = 1
AND @expert_mode = 1
)
BEGIN
SELECT
@sql +=
CONVERT
(
nvarchar(max),
N'
OUTER APPLY
(
SELECT TOP (1)
plan_force_recommendation_status =
...
FROM #tuning_recommendations AS qstr
WHERE qstr.regressed_plan_id = qsrs.plan_id
AND qstr.database_id = qsrs.database_id
ORDER BY
qstr.last_refresh DESC
) AS tr'
);
END; /*End tuning recommendations query*/
/*Strap on the query hash totals table*/This keeps it as its own independent block rather than nesting inside the wait stats @new = 1 check.
|
On the points:
|
…items @find_high_impact = 1 identifies the vital few queries consuming disproportionate resources. Scores queries across 6 dimensions (CPU, duration, reads, writes, memory, executions) using PERCENT_RANK, shows share-of-total per metric, and surfaces diagnostic signals (parameter sensitivity, plan instability, spills, wait time). Tested on SQL Server 2016-2025. Also fixes two items from Reece's tuning recommendations PR (#691): - SUBSTRING parsing: added IIF fallback when CHARINDEX returns 0 so field reordering in future SQL Server versions won't throw Msg 536 (invalid length parameter) - OUTER APPLY for plan_force_recommendation_status: pulled into its own @new = 1 AND @expert_mode = 1 block so it doesn't run as wasted work when expert mode is off Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
In hopes of closing #483. You will notice that I have copy and pasted Erik's JSON-shredding code from there.
Nothing truly intelligent done here. Erik has put a year and a half's more thought into this than me, so he will have better ideas for what data to include and what column name(s) to use. All that I have tried to do is make
sp_QuickieStoreget something useful fromsys.dm_db_tuning_recommendations. All of this is locked behind@Expert_Mode = 1.My only addition is the
plan_force_recommendation_statuscolumn in the mainsp_QuickieStoreoutput. It is shown in the screenshot below. To add context, the screenshot also shows part of a row fromsys.dm_db_tuning_recommendations, as well as the JSON. Neither of these have been imported intosp_QuickieStore. All that I did was add theplan_force_recommendation_statuscolumn. Getting a good screenshot of these wide tables is so hard that I might have to turn to PowerShell next time.For testing, I suggest copying this demo of Kendra Little's. Getting automatic tuning to kick in without help is hard.
I am not aware of any surprising cases that we need to defend against. Every column I have used has been from best guesses derived from playing with demo scripts. I am ignorant of the kinds of lessons that could only be learned from giving the feature a year of serious use. The only person I know of who has that experience would be Kendra Little. Incidentally, one of her main complaints about automatic tuning is that
sys.dm_db_tuning_recommendationsis not persisted. If Erik's new monitoring tool persists Expert Mode data, then this would solve one of Kendra's complaints. The other was regardingsys.sp_configure_automatic_tuning, which has already been fixed.