Skip to content

Fix blocked process report plan lookup (#867) #868

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.

Sign up for GitHub

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

Merged
erikdarlingdata merged 1 commit into from
Apr 20, 2026
+191 −1
Merged

Fix blocked process report plan lookup (#867) #868

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion Lite/Controls/ServerTab.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,38 @@
</MenuItem>
</ContextMenu>

<!-- Context menu for Blocked Process Reports: separate Blocked/Blocking plan
actions; no Get Actual Plan because re-executing a blocked query is a foot-gun. -->
<ContextMenu x:Key="BlockedProcessContextMenu">
<MenuItem Header="Copy Cell" Click="CopyCell_Click">
<MenuItem.Icon><TextBlock Text="&#x1F4CB;"/></MenuItem.Icon>
</MenuItem>
<MenuItem Header="Copy Row" Click="CopyRow_Click">
<MenuItem.Icon><TextBlock Text="&#x1F4C4;"/></MenuItem.Icon>
</MenuItem>
<MenuItem Header="Copy All Rows" Click="CopyAllRows_Click">
<MenuItem.Icon><TextBlock Text="&#x1F4D1;"/></MenuItem.Icon>
</MenuItem>
<Separator/>
<MenuItem Header="Export to CSV..." Click="ExportToCsv_Click">
<MenuItem.Icon><TextBlock Text="&#x1F4CA;"/></MenuItem.Icon>
</MenuItem>
<Separator/>
<MenuItem Header="View Blocked Plan" Click="ViewBlockedSidePlan_Click">
<MenuItem.Icon><TextBlock Text="&#x1F50D;"/></MenuItem.Icon>
</MenuItem>
<MenuItem Header="View Blocking Plan" Click="ViewBlockingSidePlan_Click">
<MenuItem.Icon><TextBlock Text="&#x1F50D;"/></MenuItem.Icon>
</MenuItem>
</ContextMenu>

<Style x:Key="GridRowStyle" TargetType="DataGridRow">
<Setter Property="ContextMenu" Value="{StaticResource DataGridContextMenu}"/>
</Style>

<!-- Row style for blocking grid - highlight long-duration blocks -->
<Style x:Key="BlockingRowStyle" TargetType="DataGridRow" BasedOn="{StaticResource GridRowStyle}">
<Style x:Key="BlockingRowStyle" TargetType="DataGridRow">
<Setter Property="ContextMenu" Value="{StaticResource BlockedProcessContextMenu}"/>
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dropping BasedOn="{StaticResource GridRowStyle}" is fine today because GridRowStyle only sets ContextMenu, which this style now overrides. Flagging so the next setter added to GridRowStyle isn't silently skipped here — easy landmine.

Generated by Claude Code

<Style.Triggers>
<DataTrigger Binding="{Binding IsLongBlock}" Value="True">
<Setter Property="Background" Value="#33FF6B6B"/>
Expand Down
98 changes: 98 additions & 0 deletions Lite/Controls/ServerTab.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4861,6 +4861,104 @@ private async void GetActualPlan_Click(object sender, RoutedEventArgs e)
catch { return null; }
}

// ── Blocked Process Report plan lookup ──

/* SQL Server writes this 42-byte all-zero handle into executionStack frames
for dynamic SQL / system contexts where no persistent sql_handle exists.
Filter matches sp_HumanEventsBlockViewer's XPath exclusion. */
private static readonly string ZeroSqlHandle = "0x" + new string('0', 84);

private async void ViewBlockedSidePlan_Click(object sender, RoutedEventArgs e)
=> await ShowBlockedProcessPlanAsync(sender, blockingSide: false);

private async void ViewBlockingSidePlan_Click(object sender, RoutedEventArgs e)
=> await ShowBlockedProcessPlanAsync(sender, blockingSide: true);

private async System.Threading.Tasks.Task ShowBlockedProcessPlanAsync(object sender, bool blockingSide)
{
if (sender is not MenuItem menuItem) return;
var grid = FindParentDataGrid(menuItem);
if (grid?.CurrentItem is not BlockedProcessReportRow row) return;

var sideLabel = blockingSide ? "Blocking" : "Blocked";
var spid = blockingSide ? row.BlockingSpid : row.BlockedSpid;
var queryText = blockingSide ? row.BlockingSqlText : row.BlockedSqlText;
var label = $"Est Plan - {sideLabel} SPID {spid}";

var frames = ExtractBlockedProcessFrames(row.BlockedProcessReportXml, blockingSide);
if (frames.Count == 0)
{
MessageBox.Show(
$"The {sideLabel.ToLowerInvariant()} process report has no resolvable sql_handle. " +
"This usually means the query ran as dynamic SQL or a system context — " +
"SQL Server records a zero handle in that case and the plan can't be recovered.",
"No Plan Available", MessageBoxButton.OK, MessageBoxImage.Information);
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExtractBlockedProcessFrames returns an empty list on any failure path: XML parse error, missing blocked-process/blocking-process element, missing executionStack, or all frames filtered out as zero handles. All of those land here and the user is told "no resolvable sql_handle ... dynamic SQL or system context", even when the real cause is malformed XML or a collector bug. Consider differentiating "no frames in stack" from "parse failed" from "all handles zero" — at least so support triage doesn't get wild-goose-chased.

Generated by Claude Code

return;
}

string? planXml = null;
try
{
var connStr = _server.GetConnectionString(_credentialService);
foreach (var f in frames)
{
planXml = await LocalDataService.FetchPlanBySqlHandleAsync(
connStr, row.DatabaseName, f.SqlHandle, f.StmtStart, f.StmtEnd);
if (!string.IsNullOrEmpty(planXml)) break;
}
}
catch { }
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Swallowed exception. If SqlConnection.Open fails (network, auth, firewall) or the SQL itself throws (permissions on DMVs, login disabled), control falls through to the else branch and the user is told "The plan ... is no longer in the plan cache" — a misleading diagnosis for a connection-layer failure. At minimum, distinguish connection/permission errors from "not found" so the dialog doesn't lie about the cause.

Generated by Claude Code


if (!string.IsNullOrEmpty(planXml))
{
OpenPlanTab(planXml, label, queryText);
PlanViewerTabItem.IsSelected = true;
}
else
{
MessageBox.Show(
$"The plan for the {sideLabel.ToLowerInvariant()} query is no longer in the plan cache on {_server.ServerName}. " +
"Blocked process reports only give us a sql_handle — if that plan has been evicted, we can't recover it.",
"No Plan Available", MessageBoxButton.OK, MessageBoxImage.Information);
}
}

private static IReadOnlyList<(string SqlHandle, int StmtStart, int StmtEnd)> ExtractBlockedProcessFrames(
string bprXml, bool blockingSide)
{
var empty = Array.Empty<(string, int, int)>();
if (string.IsNullOrWhiteSpace(bprXml)) return empty;
try
{
var doc = System.Xml.Linq.XElement.Parse(bprXml);
var processContainer = blockingSide
? doc.Element("blocking-process")
: doc.Element("blocked-process");
var stack = processContainer?.Element("process")?.Element("executionStack");
if (stack == null) return empty;

var frames = new List<(string, int, int)>();
foreach (var frame in stack.Elements("frame"))
{
var handle = frame.Attribute("sqlhandle")?.Value;
if (string.IsNullOrWhiteSpace(handle)) continue;
if (string.Equals(handle, ZeroSqlHandle, StringComparison.OrdinalIgnoreCase)) continue;

int stmtStart = 0;
int stmtEnd = -1;
int.TryParse(frame.Attribute("stmtstart")?.Value, out stmtStart);
if (int.TryParse(frame.Attribute("stmtend")?.Value, out var se)) stmtEnd = se;

frames.Add((handle!, stmtStart, stmtEnd));
}
return frames;
}
catch
{
return empty;
}
}

// ── Active Queries Slicer ──

private async System.Threading.Tasks.Task LoadActiveQueriesSlicerAsync()
Expand Down
66 changes: 66 additions & 0 deletions Lite/Services/LocalDataService.QueryStats.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -569,6 +569,72 @@ ps.total_elapsed_time DESC
return result as string;
}

/// <summary>
/// Fetches a query plan on-demand by sql_handle + statement offsets.
/// Used for Blocked Process Reports, where query_hash is not present in the
/// XE event payload — only the sql_handle and offsets from executionStack frames.
/// </summary>
public static async Task<string?> FetchPlanBySqlHandleAsync(
string connectionString,
string databaseName,
string sqlHandleHex,
int statementStartOffset,
int statementEndOffset)
{
if (string.IsNullOrWhiteSpace(sqlHandleHex)) return null;
var handleBytes = HexStringToBytes(sqlHandleHex);
if (handleBytes == null || handleBytes.Length == 0) return null;

using var connection = new SqlConnection(connectionString);
await connection.OpenAsync();

var quotedDbName = await GetValidatedDatabaseNameAsync(connection, databaseName)
?? "[master]";
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?? "[master]" silently swaps to master when the database from the BPR XML isn't on the server (dropped/renamed, or the caller lacks VIEW ANY DEFINITION). The sibling FetchProcedurePlanOnDemandAsync at L537-540 returns null when validation fails — this one deviates without explanation.

Functionally the lookup works because sys.dm_exec_query_stats is server-scoped, but the caller must have rights to execute sp_executesql in master, which isn't guaranteed in hardened environments. I'd either (a) document the fallback, or (b) match the sibling's contract and return null so the caller's messaging stays honest.

Generated by Claude Code


var query = $@"
EXECUTE {quotedDbName}.sys.sp_executesql
N'
SELECT TOP (1)
query_plan_text = tqp.query_plan
FROM sys.dm_exec_query_stats AS qs
OUTER APPLY sys.dm_exec_text_query_plan(qs.plan_handle, qs.statement_start_offset, qs.statement_end_offset) AS tqp
WHERE qs.sql_handle = @h
AND qs.statement_start_offset = @stmt_start
AND qs.statement_end_offset = @stmt_end
AND tqp.query_plan IS NOT NULL
ORDER BY
qs.last_execution_time DESC
OPTION(RECOMPILE);',
N'@h varbinary(64), @stmt_start int, @stmt_end int',
@h, @stmt_start, @stmt_end;";

using var command = new SqlCommand(query, connection) { CommandTimeout = 30 };
command.Parameters.Add(new SqlParameter("@h", SqlDbType.VarBinary, 64) { Value = handleBytes });
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth confirming: the sqlhandle written into BPR <frame> elements and sys.dm_exec_query_stats.sql_handle are the same 64-byte binary identity for the same ad-hoc/proc statement. In practice they match, but there are historical edge cases where BPR frames carry a shorter handle (e.g., 42 bytes for certain system/dynamic contexts) that won't equate to a 64-byte query_stats handle. A smoke test against a known repro (deliberate key-range block, BPR captured, confirm plan resolves) would close the loop — I don't see a test asserting this.

Generated by Claude Code

command.Parameters.Add(new SqlParameter("@stmt_start", SqlDbType.Int) { Value = statementStartOffset });
command.Parameters.Add(new SqlParameter("@stmt_end", SqlDbType.Int) { Value = statementEndOffset });
var result = await command.ExecuteScalarAsync();
return result as string;
}

private static byte[]? HexStringToBytes(string hex)
{
var start = hex.StartsWith("0x", StringComparison.OrdinalIgnoreCase) ? 2 : 0;
var len = hex.Length - start;
if (len <= 0 || (len % 2) != 0) return null;
var bytes = new byte[len / 2];
for (int i = 0; i < bytes.Length; i++)
{
if (!byte.TryParse(hex.AsSpan(start + i * 2, 2),
System.Globalization.NumberStyles.HexNumber,
System.Globalization.CultureInfo.InvariantCulture,
out bytes[i]))
{
return null;
}
}
return bytes;
}
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HexStringToBytes and ExtractBlockedProcessFrames are pure, static, and trivially testable — yet Lite.Tests gains nothing in this PR. Minimal coverage worth adding: zero handle → filtered; odd-length hex → null; missing stmtend attribute → defaults applied; malformed XML → empty list; multi-frame stack picks frames in document order.

Generated by Claude Code


/// <summary>
/// Gets top procedures by CPU for a server.
/// </summary>
Expand Down
Loading