-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
cluster-ui: update active execution and sessions details #85974
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 9 files at r1, all commit messages.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @kevin-v-ngo and @xinhaoz)
pkg/ui/workspaces/cluster-ui/src/activeExecutions/activeStatementUtils.ts
line 142 at r1 (raw file):
retries: activeTxn.num_auto_retries, statementCount: activeTxn.num_statements_executed, isFullScan: false,
here can't you use the information about the stmt above and if any of them were fullscan you can use it here
pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx
line 436 at r1 (raw file):
<div> <Text textType={TextTypes.Heading5} className={cx("details-header")}> Most Transaction Fingerprints Executed
nit: here you say you will show the most executed (giving the impression of listing the txn that were executed the most amount of time) and on the description right below you say the most recent. Make sure they both say the same thing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @kevin-v-ngo and @xinhaoz)
pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx
line 446 at r1 (raw file):
> {session.txn_fingerprint_ids.map((txnFingerprintID, i) => ( <div key={i}>{txnFingerprintID.toString(16)}</div>
can we make them links instead of just the text? something similar to the other links you have to txn/stmt on other pages?
Previously, maryliag (Marylia Gutierrez) wrote…
If we do that we'll have to make a request to fetch statement fingerprint stats (if none are loaded) and find the fingerprint id if it exists in the response. This is due to how we construct links to the statement fingerprint details (requires implicit txn attribute or app name). What are your thoughts? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @kevin-v-ngo and @xinhaoz)
pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx
line 446 at r1 (raw file):
Previously, xinhaoz (Xin Hao Zhang) wrote…
If we do that we'll have to make a request to fetch statement fingerprint stats (if none are loaded) and find the fingerprint id if it exists in the response. This is due to how we construct links to the statement fingerprint details (requires implicit txn attribute or app name). What are your thoughts?
I'm just think how useful the info is the way is showing today.
Thoughts @kevin-v-ngo ?
Fixes cockroachdb#85968 Closes cockroachdb#85912 Closes cockroachdb#85973 This commit adds new details to the active execution details pages: full scan (both stmt and txn), priority (txn only), and last retry reason (txn only). New information is also added to the sessions table and details pages: transaction count, active duration, recent txn fingerprint ids (cache size comes from a cluster setting). This commit also fixes a bug in the sessions overview UI where the duration for closed sessions was incorrectly calcualted based on the current time instead of the session end time. Release note (ui change): the following fields have been added to the active stmt/txn details pages: - Full Scan: indicates if the execution contains a full scan - Last Retry Reason (txn page only): the last recorded reason the txn was retried - Priority (txn page only): the txn priority The following fields have been added to the sessions table and page: - Transaction count: the number of txns executed by the session - Session active duration: the time a session spent executing txns - Session most recent fingerprint ids
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! 0 of 0 LGTMs obtained (waiting on @kevin-v-ngo and @maryliag)
pkg/ui/workspaces/cluster-ui/src/activeExecutions/activeStatementUtils.ts
line 142 at r1 (raw file):
Previously, maryliag (Marylia Gutierrez) wrote…
here can't you use the information about the stmt above and if any of them were fullscan you can use it here
Done.
pkg/ui/workspaces/cluster-ui/src/sessions/sessionDetails.tsx
line 436 at r1 (raw file):
Previously, maryliag (Marylia Gutierrez) wrote…
nit: here you say you will show the most executed (giving the impression of listing the txn that were executed the most amount of time) and on the description right below you say the most recent. Make sure they both say the same thing
Fixed!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the questions for @kevin-v-ngo
Reviewed 2 of 2 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @kevin-v-ngo)
Discussed offline -- going to address any of Kevin's suggestions for a session's txn fingerprint ids in follow up PRs. |
Build failed (retrying...): |
Build failed (retrying...): |
Build succeeded: |
Fixes #85968
Closes #85912
Closes #85973
This commit adds new details to the active execution details pages:
full scan (both stmt and txn), priority (txn only), and last retry
reason (txn only). New information is also added to the sessions
table and details pages: transaction count, active duration,
recent txn fingerprint ids (cache size comes from a cluster setting).
This commit also fixes a bug in the sessions overview UI where
the duration for closed sessions was incorrectly calcualted based
on the current time instead of the session end time.
Release note (ui change): the following fields have been added to
the active stmt/txn details pages:
txn was retried
The following fields have been added to the sessions table and page:
Retry reason populated example:
https://www.loom.com/share/e396d5aa7dda4d5995227154c6b5076b