Optimize slow query logger to reduce CPU usage#5601
Merged
Merged
Conversation
…eaker, timeout cap
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the slow-query Query Store diagnostic logger in SqlServerSearchService to reduce CPU overhead during transient database slowdowns by avoiding expensive text normalization scans and adding throttling/backoff behavior.
Changes:
- Adds a circuit breaker/backoff around Query Store diagnostic lookups to prevent repeated expensive failures during incidents.
- Uses faster Query Store lookup strategies: stored-proc lookup via
OBJECT_ID, and ad-hoc lookup via embedded/* HASH ... */comment with fallback to prior text matching. - Adds unit tests covering parameter-hash extraction and validating throttling/timeout constants.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/Microsoft.Health.Fhir.SqlServer/Features/Search/SqlServerSearchService.cs | Adds hash-based and stored-proc Query Store lookup paths, circuit breaker/backoff, and lookup-method tagging in logged stats. |
| src/Microsoft.Health.Fhir.SqlServer.UnitTests/Features/Search/SqlServerSearchServiceQueryStoreTests.cs | Adds tests for ExtractParameterHash and sanity checks for throttling/timeout constants. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5601 +/- ##
==========================================
- Coverage 77.29% 77.13% -0.16%
==========================================
Files 997 997
Lines 36564 36693 +129
Branches 5549 5559 +10
==========================================
+ Hits 28261 28304 +43
- Misses 6928 7016 +88
+ Partials 1375 1373 -2 🚀 New features to boost your workflow:
|
… clean up tests and harden ExtractParameterHash
…e-slow-query-logger
…, sync-comment SQL/test rationale - #1 Include exception type/message in the Warning log so operators can diagnose Query Store lookup failures without Debug logging. - #2 Add comment cross-referencing SqlQueryGenerator.ParametersHashStart for the hardcoded '/* HASH ' SQL literal. - #3 Use StringComparison.Ordinal for the hash markers (always emitted uppercase). - #4 Expand QueryStoreLookupTimeoutSeconds test rationale for the upper bound. - #6 Note the three lookup SQL consts share structure and must be kept in sync.
mikaelweave
reviewed
Jun 9, 2026
…ne effectiveQuery, XML-doc timeout const - Revert AI suggestion: exception is passed to LogWarning (queryable via env_ex_*), so its type/message is no longer duplicated in the message string. - Remove the ILogger parameter from FireAndForgetQueryStoreLookup and LogQueryStoreByTextAsync; use the class _logger field. - Inline the redundant effectiveQuery alias. - Convert the QueryStoreLookupTimeoutSeconds comment to an XML doc comment.
mikaelweave
approved these changes
Jun 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Optimizes
LogQueryStoreByTextAsyncinSqlServerSearchServiceto reduce CPUimpact during transient slowdowns.
Changes:
/* HASH ... */comment alreadyembedded by
SqlQueryGeneratorto match queries via hash instead of expensiveREPLACE+LIKEonquery_sql_text. Falls back to text matching for queries without hashes.OBJECT_IDlookup instead of text scanning.so a slow lookup never blocks for long.
lookup=diagnostic tag: Logs which lookup method was used(
Hash,TextNoHash,TextFallback,StoredProc) for observability.Related issues
Addresses AB191430
Testing
Describe how this change was tested.
FHIR Team Checklist
Semver Change (docs)
Patch|Skip|Feature|Breaking (reason)