sql: fix TestTrace flake from statement hints spans#166334
Merged
trunk-io[bot] merged 2 commits intocockroachdb:masterfrom Mar 24, 2026
Merged
sql: fix TestTrace flake from statement hints spans#166334trunk-io[bot] merged 2 commits intocockroachdb:masterfrom
trunk-io[bot] merged 2 commits intocockroachdb:masterfrom
Conversation
Statement hint queries (`CheckForStatementHintsInDB` and `GetStatementHintsFromDB`) are simple point lookups on `system.statement_hints` by hash. These should not incur the overhead of distributed execution. Force `distsql=off` via a session data override so the internal executor runs them locally. Previously, when the statement hints cache rangefeed had not yet initialized on a node, the cache would fall through to querying the database. On multi-node clusters, the DistSQL planner could decide to distribute this internal query (e.g. due to `ORDER BY` with no stats), producing DistSQL-related trace spans that leaked into the session trace. Informs cockroachdb#166212 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add `get-plan-hints` and `prepare stmt` to the always-optional spans list in `TestTrace`. Before the statement hints cache rangefeed initializes, the cache falls through to a database query whose spans leak into the session trace and cause unexpected span name mismatches. Fixes cockroachdb#166212 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
😎 Merged successfully - details. |
Member
|
This change is |
mw5h
requested review from
DrewKimball,
ZhouXing19 and
michae2
and removed request for
a team
michae2
approved these changes
Mar 23, 2026
Collaborator
michae2
left a comment
michae2
left a comment
There was a problem hiding this comment.
@michae2 reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on DrewKimball and ZhouXing19).
|
Based on the specified backports for this PR, I applied new labels to the following linked issue(s). Please adjust the labels as needed to match the branches actually affected by the issue(s), including adding any known older branches. Issue #166212: branch-release-26.2. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
This was referenced Mar 24, 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
3 participants
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.
Summary
Fixes a flake in
TestTracecaused by the statement hints feature introducinginternal query spans that leak into the session trace.
Commit 1: Force
distsql=offfor statement hint queries(
CheckForStatementHintsInDB,GetStatementHintsFromDB). These are simplepoint lookups that don't need distributed execution.
Commit 2: Add
get-plan-hintsandprepare stmtto the always-optionalspans list in
TestTraceto handle the case where the hints cache rangefeedhasn't initialized yet and the internal query runs.
The root cause: before the
StatementHintsCacherangefeed initializeshintedHasheson a node, every statement falls through to queryingsystem.statement_hintsvia the internal executor. This internal query wasbeing distributed (due to
ORDER BYwith no stats), producing DistSQL spans(
/cockroach.sql.distsqlrun.DistSQL/SetupFlow) that the test didn't expectwhen
distsql=off.Fixes #166212
🤖 Generated with Claude Code