release-22.1: add new search criteria (limit, sort) to sql stats pages #103130
Conversation
This file contains 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
|
Thanks for opening a backport. Please check the backport criteria before merging:
If some of the basic criteria cannot be satisfied, ensure that the exceptional criteria are satisfied within.
Add a brief release justification to the body of your PR to justify this backport. Some other things to consider:
|
|
This change is |
xinhaoz
force-pushed
the
backport22.1-98815
branch
3 times, most recently
from
acba7b8
to
f64d806
Compare
This commit adds a new field, fetch_mode, which is a message containing an enum specifying what kind of stats to return from the combined stmts endpoint. The options are - StmtStatsOnly - TxnStatsOnly Leaving this field null will fetch both stmts and txns. If `TxnStatsOnly` is specified, we will also include the stmt stats for the stms in each txn in the returned txn payload. This is because in the client app we need the stmt stats to properly build the txn fingerprint pages. In the future, we will split this API instead of using this fetch mode flag, but this is currently preferred to make it easier to backport for perf improvements. Epic: none Part of: cockroachdb#97252 Part of: cockroachdb#97875 Release note: None
Previously, both the stmt and txns fingerprints pages were using the same api response from the /statements. This was because we need the stmts response to build txns pages. Howevever having to fetch both types of stats can slow down the request. Now that we can specify the fetch_mode as part of the request, we can split the 2 into their own api calls and redux fields. Epic: none Part of: cockroachdb#97875 Release note: None
This commit adds limit and sort fields to the combined statements request. These params can be used to specify how much data is returned and the priority at which to return data (e.g. top-k). The current sort options are: - service latency - cpu time - p99 (statements only) - contention time - execution count Epic: none Part of: cockroachdb#97876 Part of: cockroachdb#97875 Release note: None
xinhaoz
force-pushed
the
backport22.1-98815
branch
from
f4611b1
to
a5f1cb3
Compare
xinhaoz
changed the title
This commit changes the table used for the combined stats endpoint from the view combining persisted and in-memory stats to the view that is just a wrapper around the system table. Thus, we are now reading only persisted stats from the system table for the combined stats endpoint. This commit updates tests using the combined api to flush stats before using the api. Epic: None Release note (ui change): On the SQL Activity fingerprints pages, users will not see stats that have not yet been flushed to disk.
xinhaoz
force-pushed
the
backport22.1-98815
branch
from
a5f1cb3
to
30371a9
Compare
maryliag
reviewed
May 16, 2023
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @xinhaoz)
pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx line 364 at r10 (raw file):
const { pagination } = this.state; this.setState({ pagination: { ...pagination, current } }); this.props.onPageChanged != null && this.props.onPageChanged(current);
we actually need this check, because this function could be empty (is just used for analytics), and I just had to create a PR to add this back onhttps://github.com//pull/103484/files#diff-b7e775c116135547929b4e0aa104a33aa09a26137d076187cbbc7e1504f1acb0R411-R413
(I'll be backporting that fix for 23.1 and 22.2, but since you haven't merge this yet make sense to already do the right thing here)
This commit adds new knobs to the sql stats fingerprint pages. Users can now specify a limit and sort priority with their sql stats fingerprints requests. Closes: cockroachdb#97876 Part of: cockroachdb#97875 Release note: None
Previously, the calculatation for the `% of runtime` column for the sql activity pages, was performed on the client app by summing the (execCount* avgAvcLat) from all statements returned. This was okay when we were returning a large number of stmts (20,000) but now that we have reduced that limit significantly (default 100, with a max provided option of 500), performing this calculation on the client side won't give us the full picture of total runtime of the workload in the requested time interval. This commit modifies the statements API to return the total estimated runtime of the workload so that we can continue to show the '% of runtime` column. We also now provide this as a server sort option, so that users can request the top-k stmts or txns by % of runtime. Epic: none Release note (ui change): Users can request top-k stmts by % runtime in sql activity fingerprints pages.
Create new Search Criteria component, and add to Statements and Transactions page. This commit also updates the UI, with the new position of filters and reset sql stats. Part Of cockroachdb#98891 Release note (ui change): Add Search Criteria to Statements and Transactions pages, updates UX with improvements.
When we are on a new cluster-ui version that sends user configured limit and sort values in the sql stats request, we may be talking to an older server version that does not support those request params. In that scenario, The server response will be that of the request without limit and sort. This commit checks the response from the server so that if we find that the payload limit is not equal to the requested limit, we manually sort and truncate the response to match the parameters the user sees on the UI. Epic: none Release note: None
This change adds some missing props to the txnDetailsPage story file. Epic: none Release note: None
Previously the db tables page was using formatDate from the antd project, which was causing test failures. This commit swaps the usage to use moment's format function directly. Epic: none Release note: None
xinhaoz
force-pushed
the
backport22.1-98815
branch
from
30371a9
to
b3bf6af
Compare
xinhaoz
commented
May 17, 2023
There was a problem hiding this comment.
Reviewable status:
pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.tsx line 364 at r10 (raw file):
Previously, maryliag (Marylia Gutierrez) wrote…
we actually need this check, because this function could be empty (is just used for analytics), and I just had to create a PR to add this back onhttps://github.com//pull/103484/files#diff-b7e775c116135547929b4e0aa104a33aa09a26137d076187cbbc7e1504f1acb0R411-R413
(I'll be backporting that fix for 23.1 and 22.2, but since you haven't merge this yet make sense to already do the right thing here)
Done.
maryliag
approved these changes
May 18, 2023
There was a problem hiding this comment.
can you just add a loom confirming everything is working on 22.1 since the current loom was for the main branch?
otheriwse
Reviewed 3 of 3 files at r1, 21 of 21 files at r2, 2 of 2 files at r3, 4 of 4 files at r4, 12 of 20 files at r5, 34 of 34 files at r11, 12 of 12 files at r12, 20 of 20 files at r13, 4 of 4 files at r14, 2 of 2 files at r15, 1 of 1 files at r16, all commit messages.
Reviewable status:
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.
Backport 7/7 commits from #98815.
/cc @cockroachdb/release
See individual commits.
DB-Console: https://www.loom.com/share/86989713b95b4a728632e323c12088fe
Release justification: code yellow - high priority fixes