server, ui: show internal stats with custer setting enabled#114743
server, ui: show internal stats with custer setting enabled#114743craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
xinhaoz
left a comment
There was a problem hiding this comment.
Thanks for fixing 🔥 Can we add a unit test to verify the proper table is used when we have a valid activity table for the time range but show internal queries is on? Just in case this is accidentally removed or something.
| ); | ||
| return ( | ||
| (!appNames?.length && !isInternal) || | ||
| !appNames?.length || |
There was a problem hiding this comment.
Why was the && !isInternal removed? I thought we wanted to align with the txns page where we only show internal stmts when they are specifically selected in the app name filter.
There was a problem hiding this comment.
Ah nvm, just read that we're swapping the behaviour here.
There was a problem hiding this comment.
It's also updated on the Txn page with the new behaviour
3057f08 to
7bdf5c7
Compare
maryliag
left a comment
There was a problem hiding this comment.
Test added
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @dhartunian and @xinhaoz)
Previously, when the cluster setting `sql.stats.response.show_internal.enabled` was set to true, we were still trying to use the activity tables to return data to the UI, but that tables doesn't have data from internal stats. This commit does that proper check to not use the Activity tables if that value is enabled. Also, since enabling that setting means the user wants to see the internal stats, this commit also changes the UI so that it returns the internal stats, oterhwise it might be confusing selecting top 100 but the UI only showing 40 for example. Fixes cockroachdb#114460 Release note (bug fix): SQL Activity properly showing internal statements when cluster setting `sql.stats.response.show_internal.enabled` is set to true.
7bdf5c7 to
8ce19b4
Compare
|
TFTR! bors r+ |
|
Build succeeded: |
|
Encountered an error creating backports. Some common things that can go wrong:
You might need to create your backport manually using the backport tool. error creating merge commit from 8ce19b4 to blathers/backport-release-23.1-114743: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.1.x failed. See errors above. error creating merge commit from 8ce19b4 to blathers/backport-release-23.2-114743: POST https://api.github.com/repos/cockroachdb/cockroach/merges: 409 Merge conflict [] you may need to manually resolve merge conflicts with the backport tool. Backport to branch 23.2.x failed. See errors above. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
Previously, when the cluster setting
sql.stats.response.show_internal.enabledwas set to true, we were still trying to use the activity tables to return data to the UI, but that tables doesn't have data from internal stats.This commit does that proper check to not use the Activity tables if that value is enabled.
Also, since enabling that setting means the user wants to see the internal stats, this commit also changes the UI so that it returns the internal stats, oterhwise it might be confusing selecting top 100 but the UI only showing 40 for example.
Fixes #114460
https://www.loom.com/share/8f3279ae4fd441999c24c68fb0fa8b14
Release note (bug fix): SQL Activity properly showing internal statements when cluster setting
sql.stats.response.show_internal.enabledis set to true.