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

sql, insights: record failed transactions due to commit #110898

Merged
merged 1 commit into from Sep 27, 2023

Conversation

xinhaoz
Copy link
Member

@xinhaoz xinhaoz commented Sep 19, 2023

Previously we did not record failed transactions due to a failed
commit in the insights system. This commit captures those transactions
in the insights system by propagating the payload error associated with
a failed commit to the stats recording step. If the transaction error
is nil, we will attempt to find the last statement error for scenarios
where the error is not propagated to the payload of TxnCommit/Aborted
states.

In addition, the Status (completed or failed) of a txn is now based
on whether the transaction has committed at the time of insights recording.
Previously it was also based on the presence of failed stmts.

Fixes: #110846

Release note (ui change): In the Insights page in DB Console, users can now see
failed transactions and their error messages when the transaction fails on the
COMMIT stage.

Pasted Graphic

@cockroach-teamcity
Copy link
Member

This change is Reviewable

@xinhaoz xinhaoz force-pushed the record-txn-commit-errs branch 7 times, most recently from ebedfa8 to a0483f0 Compare September 22, 2023 16:07
@xinhaoz xinhaoz marked this pull request as ready for review September 22, 2023 16:11
@xinhaoz xinhaoz requested a review from a team as a code owner September 22, 2023 16:11
@xinhaoz xinhaoz requested a review from a team September 22, 2023 16:11
Previously we did not record failed transactions due to a failed
commit in the insights system. This commit captures those transactions
in the insights system by propagating the payload error associated with
a failed commit to the stats recording step. If the transaction error
is nil, we will attempt to find the last statement error for scenarios
where the error is not propagated to the payload of TxnCommit/Aborted
states.

In addition, the `Status` (completed or failed) of a txn is now based
on whether the transaction has committed at the time of insights recording.
Previously it was also based on the presence of failed stmts.

Fixes: cockroachdb#110846

Release note (ui change): In the Insights page in DB Console, users can now see
failed transactions and their error messages when the transaction fails on the
COMMIT stage.
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.

:lgtm:

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

Copy link
Contributor

@annrpom annrpom left a comment

Choose a reason for hiding this comment

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

:lgtm: thanks for your great comments! found myself getting stuck on why the code did something & reading the comments gave me proper cxontext

Reviewed all commit messages.
Reviewable status: :shipit: complete! 2 of 0 LGTMs obtained (waiting on @xinhaoz)

@xinhaoz
Copy link
Member Author

xinhaoz commented Sep 27, 2023

TFTR!
bors r+

@craig
Copy link
Contributor

craig bot commented Sep 27, 2023

Build succeeded:

@craig craig bot merged commit ed6b6d7 into cockroachdb:master Sep 27, 2023
8 checks passed
@xinhaoz xinhaoz deleted the record-txn-commit-errs branch April 1, 2024 17:06
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.

insights: record txns that failed on commit
4 participants