-
Notifications
You must be signed in to change notification settings - Fork 310
Fix active xa rollback failure and added error message judgment #875
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
base: master
Are you sure you want to change the base?
Fix active xa rollback failure and added error message judgment #875
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #875 +/- ##
=========================================
Coverage ? 38.03%
=========================================
Files ? 186
Lines ? 11243
Branches ? 0
=========================================
Hits ? 4276
Misses ? 6542
Partials ? 425 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pkg/datasource/sql/conn_xa.go
Outdated
|
||
errMsg := err.Error() | ||
// Check for XAER_RMFAIL error with "already ended" message | ||
if strings.Contains(errMsg, "XAER_RMFAIL") && strings.Contains(errMsg, "already ended") { |
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.
The string matching approach in isXAER_RMFAILAlreadyEnded() is fragile
Consider using error codes or types instead of string matching
Different database vendors might format error messages differently
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.
Okay, I'm trying to reproduce him and add constant Err code judgment.
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.
LGTM
If we can improve the unit test coverage, that would be even better. |
What this PR does:
with :#672 Fix active xa rollback failure and added error message judgment
Which issue(s) this PR fixes:
Fixes #
#708
Special notes for your reviewer:
Does this PR introduce a user-facing change?: