Added exclusions for sp_server_diagnostics and WAITFOR wait types to GetLongRunningQueriesAsync() method.#362
Conversation
…_DIAGNOSTIC wait types, and WAITFOR wait types.
…to allow future configurability on the number of long-running queries returned. Added optional parameter to control display of WAITFOR types in future.
…n instead of janky string addition.
…clusions Feature/long running query exclusions
…c() method. Removed System.Collections.Generic using statement as it is unnecessary.
…KUPTHREAD-to-long-running-queries Added exclusion for backup-related waits to GetLongRunningQueriesAsync() method.
erikdarlingdata
left a comment
There was a problem hiding this comment.
Thanks for this — filtering by r.wait_type instead of query text is a much better approach, and the sp_server_diagnostics exclusion is a real need for WSFC/AG environments.
A few things to address:
WAITFOR RECEIVE regression
The old code filtered t.text NOT LIKE N'%waitfor receive%' which caught Service Broker waits. The new r.wait_type <> N'WAITFOR' doesn't cover that — Service Broker uses BROKER_RECEIVE_WAITFOR as its wait type. Please add that to the exclusions so we don't regress.
Add XE_LIVE_TARGET_TVF exclusion
Please also exclude XE_LIVE_TARGET_TVF — Extended Events live target reads show up as long-running and aren't actionable.
Unused parameters
The excludeSpServerDiagnostics, excludeWaitFor, excludeBackupWaits, and maxLongRunningQueryCount parameters aren't used by any caller. Let's keep the method signature simple — just apply the exclusions unconditionally. If we need configurability later, we can add it then.
Minor code issues
if (maxLongRunningQueryCount <= 5)clamps to minimum 5 — if the intent is to prevent invalid values, this should be<= 0or< 1string LongRunningQueryCount— PascalCase for a local variable (should be camelCase per C# conventions)- Trailing
};after the if block (semicolon unnecessary) - The
//using System.Collections.Generic;comment-out inLandingPage.xaml.csis unrelated to this PR — please revert that file
Overall the direction is good, just needs some cleanup. Thanks!
…dded miscWaitsFilter to exclude XE_LIVE_TARGET_TVF waits. Removed unused parameters. Corrected minimum value for maxLongRunningQueryCount (minimum 1 instead of 5).
…KUPTHREAD-to-long-running-queries Feature/add exclusion for backupthread to long running queries
|
Requested changes have been made. |
erikdarlingdata
left a comment
There was a problem hiding this comment.
Looks good — all the requested changes from the first review are addressed. The wait_type filtering approach is solid.
A couple minor style nits (non-blocking):
-
String interpolation is unnecessary — since the four filter strings are always applied unconditionally, they could just be inlined directly into the SQL constant. The separate variables +
@$""interpolation adds indirection without benefit. Not a blocker. -
SP_SERVER_DIAGNOSTICS LIKE —
r.wait_type NOT LIKE N'%SP_SERVER_DIAGNOSTICS%'could ber.wait_type <> N'SP_SERVER_DIAGNOSTICS'since that's the exact wait type name. Minor.
Approving as-is — these can be cleaned up later if desired.
|
Regarding the I'm in the process of adding similar exclusions to the |
|
@HannahVernon do you want to piggyback this PR for the Lite changes too, or handle that in a separate one? WRT the waits, I'd only ever seen it sawing away on |
|
I already setup a separate branch for Lite. |
|
Should i add the specific |
|
@HannahVernon Fine as-is, thank you again for the contribution! You will be remembered forever as an Open Source Heroine. |
What does this PR do?
Adds exclusions to
GetLongRunningQueriesAsync()to excludesp_server_diagnosticsandWAITFORwait types. Adds parameters toGetLongRunningQueriesAsync()for future use to allow precise control over exclusion of those wait types, along with an optional parameter to control the maximum number of long running queries returned to the caller.Which component(s) does this affect?
How was this tested?
Tested against SQL Server 2022 with sp_server_diagnostics wait types caused by Windows Server Failover Cluster monitoring, and via a manual query with
WAITFOR DELAY '00:30:00';Checklist
dotnet build -c Debug)