Allow excluding internal statement, transactions, and sessions in SQL activity pages #70544
Comments
kevin-v-ngo
added
C-enhancement
|
Including Figma designs here for multi-select App input (uses same functionality as the 'statement type' multi-select dropdown)
|
|
It'd be great to remove internal/system statements ($ internal) by default. Users shouldn't need to troubleshoot these types of statements. |
|
@Annebirzin we would need some decision on how to have all of them selected except internal by default. Should that be a new value on the selection? |
Previously you could only select one App on the filter for Transaction and Statement page. This commits introduces a multi select option, making possible for the user select several apps at the same time and exclude internal results. This commits also properly sets the filter value for database with no values to (unset). Partially addresses cockroachdb#70544 Not included in this commit: - Select all apps except internal by default - Add app filter to Session tab Release note (ui change): App filter on Transaction and Statement pages is now multi select.
71731: sql: consistent aggregated timestamp when flushing sql stats r=matthewtodd a=Azhng Previously, when SQL Stats are flushed to system table, the aggregatedTs column for SQL Stats is calculated for each stats entry individually. This means that if a flush starts near the end of each hour, it is possible that different stats rows will be assigned two different aggregated timestamp. Currently, flusher flushes statement statistics first, and only when statement statistics are flushed, transaction statistics will be flushed into system table. This means that it is likely the transaction statistics will get assigned a different aggregatedTs than the statement statistics. Consequentially, when the frontend fetches SQL Stats through the CombinedStmtStats handler, the frontend default performs range scan at 1 hour interval. This triggers a range scan on the system table for that 1 hour range. This causes the statement statisitcs that got assigned a different aggregatedTs to be omitted from the result. This commit changed the flusher to only compute aggregatedTs once before the flush actually happen, and assign that aggregatedTs too *all* stmt/txn stats rows. Statements executed in the same aggregation interval can be looked up by the corresponding statement fingerprint ID stored in transaction stats metadata. Follow up to #71596 Release note: None 71897: ui:app filter is has multi select option r=maryliag a=maryliag Previously you could only select one App on the filter for Transaction and Statement page. This commits introduces a multi select option, making possible for the user select several apps at the same time and exclude internal results. This commits also properly sets the filter value for database with no values to (unset). Partially addresses #70544 Not included in this commit: - Select all apps except internal by default - Add app filter to Session tab Before <img width="308" alt="Screen Shot 2021-10-22 at 6 27 25 PM" src="https://user-images.githubusercontent.com/1017486/138529700-012a3530-a7ef-4bbe-bd81-0a0a49c5be61.png"> <img width="278" alt="Screen Shot 2021-10-22 at 6 29 37 PM" src="https://user-images.githubusercontent.com/1017486/138529713-0c9c309d-5255-4fe9-be75-5edb5bdf0580.png"> After <img width="273" alt="Screen Shot 2021-10-22 at 6 27 37 PM" src="https://user-images.githubusercontent.com/1017486/138529626-1966a7c1-fba5-4167-9c17-351cc0f8da3d.png"> <img width="273" alt="Screen Shot 2021-10-22 at 6 29 51 PM" src="https://user-images.githubusercontent.com/1017486/138529726-39ff6c0b-97ec-4ea4-af9f-aa2d3d15ac80.png"> Release note (ui change): App filter on Transaction and Statement pages is now multi select. Co-authored-by: Azhng <[email protected]> Co-authored-by: Marylia Gutierrez <[email protected]>
|
@maryliag I'm wondering if we can just hide internal apps by default without indicating this in the filter. (0 filters would be selected, but internal apps would be hidden). If a user selects and applies the 'internal' filter, then we show internal statements. I know it's a little inconsistent with how the other filters work, but seems like users aren't really going to care about internal statements. We could introduce a new filter for 'all non-internal' apps as you suggested, but I feel like this might be more confusing to the user. A third option would be to select all the non-internal apps by default (so the filter would be activated by default showing 'n' app filters) but I feel like this is a little strange and calling too much attention to the fact that we allow you to view internal statements. cc @kevin-v-ngo |
|
sounds good, I will do the approach of hiding internals by default |
|
Hm, can you clarify what you mean by 'apps with that info'? I think it might be okay not to further clarify that we are hiding internal apps by default since it's really an advanced user who would care about those statements (or possibly only our internal crl team). Maybe if we get feedback that it's confusing we can add messaging in the future. |
|
I realized my phrasing was confusing, I meant to say on the apps filter we can add the info icon |
|
should we also hide the statements where no app was set? those were also probably created by us, so I think we should hide them too be default |
|
Application names are optional by default so i'd assume we'd have quite a few user-created statements and transactions that have the no app set. Do you see quite a few created by us? I hope it's small since we should be following our own best practices and setting that value. |
|
Ah okay, wasn't aware of that. I'll keep the ones with unset App names then. |
|
I do several statements related to the jobs and job_attempts table being executed without an App
|
|
Hmm, i dont see those statements on my side:
Hopefully it's not too many. I'll check on serverless and create issues to track specifying the application name (using our internal nomenclature) so that they can be filtered out on these pages by default. |
Previosuly, the default value when no App filter was selected on Transactions and Statements page, we were showing all data, now we're excluding internal transaction/statements. Fixes cockroachdb#70544 Release note (ui change): The default value when no App is selected on Transactions and Statements filter is now excluding internal Transactions and Statements.
71543: roachtest: disable prometheus for scrub r=otan a=otan I'm unable to reproduce the error running roachtest myself, and it's the only set of tests affected. Resolves #71979 Release note: None 71927: sql/catalog/nstree: optimize with lazy initialization, pooling btrees r=ajwerner a=ajwerner This change comes in response to #66112 (comment). The thrust of the change is to lazily initialize the data structures so that when they are not used, they do not incur cost. Additionally, pool the underlying tree so that when they are used, the allocations are effectively free. This was originally deemed unimportant because the connExecutor descs.Collection is long-lived. Unfortunately, others, as used for the type resolve in distsql, are not. ``` name old time/op new time/op delta FlowSetup/vectorize=true/distribute=true-16 145µs ± 2% 139µs ± 3% -3.94% (p=0.000 n=19+20) FlowSetup/vectorize=true/distribute=false-16 141µs ± 3% 134µs ± 2% -4.66% (p=0.000 n=18+19) FlowSetup/vectorize=false/distribute=true-16 138µs ± 4% 132µs ± 4% -4.23% (p=0.000 n=20+20) FlowSetup/vectorize=false/distribute=false-16 133µs ± 3% 127µs ± 3% -4.41% (p=0.000 n=20+19) name old alloc/op new alloc/op delta FlowSetup/vectorize=true/distribute=true-16 38.1kB ± 3% 36.7kB ± 2% -3.58% (p=0.000 n=18+18) FlowSetup/vectorize=true/distribute=false-16 36.2kB ± 0% 34.9kB ± 0% -3.66% (p=0.000 n=18+16) FlowSetup/vectorize=false/distribute=true-16 42.6kB ± 0% 41.3kB ± 0% -3.11% (p=0.000 n=17+17) FlowSetup/vectorize=false/distribute=false-16 41.0kB ± 0% 39.7kB ± 0% -3.22% (p=0.000 n=17+17) name old allocs/op new allocs/op delta FlowSetup/vectorize=true/distribute=true-16 368 ± 0% 314 ± 0% -14.67% (p=0.000 n=16+17) FlowSetup/vectorize=true/distribute=false-16 354 ± 0% 300 ± 0% -15.25% (p=0.000 n=17+17) FlowSetup/vectorize=false/distribute=true-16 338 ± 1% 283 ± 1% -16.13% (p=0.000 n=19+19) FlowSetup/vectorize=false/distribute=false-16 325 ± 0% 271 ± 0% -16.62% (p=0.000 n=18+18) ``` Omitting a release note because I'm doubtful this meaningfully shows up on its own. Release note: None 71960: ui: Added informative warning for insufficient privileges r=nathanstilwell a=nathanstilwell <img width="972" alt="Screen Shot 2021-10-25 at 17 17 49" src="https://user-images.githubusercontent.com/397448/138772352-117456e1-10a8-475b-9b1c-79b0e2327558.png"> Previously, `/#/reports/nodes` would seem to hang in a loading state indefinitely when a DB Console user wouldn't have admin privileges for that endpoint. This was due to nodes data being empty from a 403 response. An error is captured in the application state for this response, so mapping this error as a prop to the component, the UI can distinguish between the nodes data simply being empty and being empty due to a restriction error. <img width="827" alt="Screen Shot 2021-10-25 at 17 16 07" src="https://user-images.githubusercontent.com/397448/138772135-33321d34-9728-489b-8549-1c8bd55cf650.png"> Detecting this error, we render an `<InlineAlert />` to notify the user of their lack of privileges. Release note (ui change): All Nodes report notifies user is they need more privileges to view information. 71966: ui: default filter on Transaction and Statement pages now exclude internals r=maryliag a=maryliag Previously, the default value when no App filter was selected on Transactions and Statements page, we were showing all data, now we're excluding internal transaction/statements. Fixes #70544 Release note (ui change): The default value when no App is selected on Transactions and Statements filter is now excluding internal Transactions and Statements. 71967: server,ui: transactionDetail page now shows statement stats scoped by transaction fingerprint ID r=matthewtodd a=Azhng Previously, transactionDetail page shows statement statistics aggregated across **all** executions of that statement fingerprint. This led to confusing UX and the statement statistics were subsequently removed from transactionDetail page. This commit reintroduces statement statistics on transactionDetail page. The statistics now are only aggregated across the statement fingerprints that are part of the selected transaction fingerprint. Resolves #59205 #65106 Release note (ui change): transactionDetail page now shows statement statistics scoped by the transaction fingeprint. https://user-images.githubusercontent.com/9267198/138916095-c91895d5-6f34-49b7-87dd-81f375b3f582.mov 71987: sql: skip TestSavepoints r=nkodali a=jbowens Refs: #70220 Reason: flaky test Generated by bin/skip-test. Release justification: non-production code changes Release note: None Co-authored-by: Oliver Tan <[email protected]> Co-authored-by: Andrew Werner <[email protected]> Co-authored-by: Nathan Stilwell <[email protected]> Co-authored-by: Marylia Gutierrez <[email protected]> Co-authored-by: Azhng <[email protected]> Co-authored-by: Jackson Owens <[email protected]>
Previously you could only select one App on the filter for Transaction and Statement page. This commits introduces a multi select option, making possible for the user select several apps at the same time and exclude internal results. This commits also properly sets the filter value for database with no values to (unset). Partially addresses cockroachdb#70544 Not included in this commit: - Select all apps except internal by default - Add app filter to Session tab Release note (ui change): App filter on Transaction and Statement pages is now multi select.
Previously, the default value when no App filter was selected on Transactions and Statements page, we were showing all data, now we're excluding internal transaction/statements. Fixes cockroachdb#70544 Release note (ui change): The default value when no App is selected on Transactions and Statements filter is now excluding internal Transactions and Statements.
We've received feedback that internal statements/transaction/sessions create noise on the overview pages where we should provide a mechanism to filter out these statements by default. This would allow users to quickly identify their application workload.
One possible option is to make the 'App" filter on the overview pages multi-select and have all Apps selected except for '(internal)' by default. Note this would also require us to add the App filter to the sessions page.
The text was updated successfully, but these errors were encountered: