Skip to content
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

ui/cluster-ui: fix no most recent stmt for active txns #88179

Merged
merged 1 commit into from
Sep 21, 2022

Conversation

xinhaoz
Copy link
Member

@xinhaoz xinhaoz commented Sep 19, 2022

Fixes #87738

Previously, active txns could have an empty 'Most Recent Statement' column, even if their executed statement count was non-zero. This was due to the most recent query text being populated by the active stmt, which could be empty at the time of querying. This commit populates the last statement text for a txn even when it is not currently executing a query.

This commit also removes the isFullScan field from active txn pages, as we cannot fill this field out without all stmts in the txn.

Release note (ui change): Full scan field is removed from active txn details page.

Release note (bug fix): active txns with non-zero
executed statement count now always have populated stmt text, even when no stmt is being executed.

@xinhaoz xinhaoz requested a review from a team September 19, 2022 21:33
@cockroach-teamcity
Copy link
Member

This change is Reviewable

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are there any tests you can add to confirm the fix?

Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained

@xinhaoz
Copy link
Member Author

xinhaoz commented Sep 20, 2022

I can add tests to confirm the field will be populated based on the sessions response. But actually, with this change we will now show the latest query text for an active txn even if that txn is not currently executing anything. Is that really what we want here? Or should we somehow signal the txn isn't currently executing a query while showing the latest stmt text? Currently the only signal is from the fact the statement id link isn't available.

Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's okay, the label it's already "most recent", so it doesn't mean it has (or not) to be something active

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @xinhaoz)

@xinhaoz
Copy link
Member Author

xinhaoz commented Sep 20, 2022

I was thinking more about in the active transaction details page, where it might be more confusing

@xinhaoz xinhaoz force-pushed the active-txns-fix branch 5 times, most recently from 9cf1967 to 8da8f7f Compare September 21, 2022 15:47
Fixes cockroachdb#87738

Previously, active txns could have an empty 'Most Recent
Statement' column, even if their executed statement count
was non-zero. This was due to the most recent query text
being populated by the active stmt, which could be empty
at the time of querying. This commit populates the last
statement text for a txn even when it is not currently
executing a query.

This commit also removes the `isFullScan` field from
active txn pages, as we cannot fill this field out
without all stmts in the txn.

Release note (ui change): Full scan field is removed
from active txn details page.

Release note (bug fix): active txns with non-zero
executed statement count now always have populated
stmt text, even when no stmt is being executed.
@xinhaoz
Copy link
Member Author

xinhaoz commented Sep 21, 2022

bors r+

@craig
Copy link
Contributor

craig bot commented Sep 21, 2022

Build succeeded:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Acceptance testing 22.2 - No value for "Most Recent Statement" during TPCC
3 participants