Wrap fatal TX errors in a new vterrors code#17669
Conversation
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
vterrors errorvterrors code
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17669 +/- ##
==========================================
- Coverage 67.59% 67.57% -0.02%
==========================================
Files 1598 1598
Lines 259983 260044 +61
==========================================
Hits 175727 175727
- Misses 84256 84317 +61 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Notes from further offline discussion:
We need to spend some more time exploring these options. |
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
| } | ||
| c := vterrors.Code(err) | ||
| m := err.Error() | ||
| if c == vtrpcpb.Code_UNAVAILABLE && strings.Contains(m, "connection refused") { |
There was a problem hiding this comment.
Let's use go/vt/vterrors/constants.go for the magic string
|
Hello! 👋 This Pull Request is now handled by arewefastyet. The current HEAD and future commits will be benchmarked. You can find the performance comparison on the arewefastyet website. |
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
…g-prs Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
…g-prs Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
…g-prs Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
…g-prs Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
deepthi
left a comment
There was a problem hiding this comment.
The behavior and tests look good. Most of my suggestions are cosmetic / code reuse etc.
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
b21ff45 to
fa29793
Compare
deepthi
left a comment
There was a problem hiding this comment.
The error message wording needs some changes. Rest LGTM
go/vt/vterrors/code.go
Outdated
| VT09029 = errorWithState("VT09029", vtrpcpb.Code_FAILED_PRECONDITION, CTERecursiveRequiresSingleReference, "In recursive query block of Recursive Common Table Expression %s, the recursive table must be referenced only once, and not in any subquery", "") | ||
| VT09030 = errorWithState("VT09030", vtrpcpb.Code_FAILED_PRECONDITION, CTEMaxRecursionDepth, "Recursive query aborted after 1000 iterations.", "") | ||
| VT09031 = errorWithoutState("VT09031", vtrpcpb.Code_FAILED_PRECONDITION, "Primary demotion is stalled", "") | ||
| VT09032 = errorWithoutState("VT09032", vtrpcpb.Code_FAILED_PRECONDITION, "previous transaction failed fatally. Issue a ROLLBACK query to resolve the failure.", "This error occurs after a VT15001 error was sent to the client. Future queries in the same session will continue to fail until the client resolves the issue by sending a ROLLBACK query.") |
There was a problem hiding this comment.
| VT09032 = errorWithoutState("VT09032", vtrpcpb.Code_FAILED_PRECONDITION, "previous transaction failed fatally. Issue a ROLLBACK query to resolve the failure.", "This error occurs after a VT15001 error was sent to the client. Future queries in the same session will continue to fail until the client resolves the issue by sending a ROLLBACK query.") | |
| VT09032 = errorWithoutState("VT09032", vtrpcpb.Code_FAILED_PRECONDITION, "previous transaction failed. Issue a ROLLBACK to resolve the failure.", "This error occurs after a VT15001 error was sent to the client. Later queries in the same session will continue to fail until the client sends a ROLLBACK.") |
go/vt/vterrors/code.go
Outdated
| VT14004 = errorWithoutState("VT14004", vtrpcpb.Code_UNAVAILABLE, "cannot find keyspace for: %s", "The specified keyspace could not be found.") | ||
| VT14005 = errorWithoutState("VT14005", vtrpcpb.Code_UNAVAILABLE, "cannot lookup sidecar database for keyspace: %s", "Failed to read sidecar database identifier.") | ||
|
|
||
| VT15001 = errorWithNoCode("VT15001", "transaction error, issue ROLLBACK and retry the transaction: %s", "The opened transaction must be ROLLBACK by the application and re-tried.") |
There was a problem hiding this comment.
| VT15001 = errorWithNoCode("VT15001", "transaction error, issue ROLLBACK and retry the transaction: %s", "The opened transaction must be ROLLBACK by the application and re-tried.") | |
| VT15001 = errorWithNoCode("VT15001", "transaction error, issue ROLLBACK and retry the transaction: %s", "Transaction must be rolled back by the application and re-tried.") |
…g-prs Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Signed-off-by: Florent Poinsard <florent.poinsard@outlook.fr>
Description
This Pull Request adds two new
vterrorscode:VT15001andVT15002:VT15001is returned to the client when an error happened during a transaction that makes the transaction invalid and unusable. When the primary is shutdown, a PRS happens, or there is a connection loss with the primary, this error will wrap the original error. When this error happens,vtgatewill, if applicable, rollback the transaction and clear the session.VT09032is returned after aVT15001was first issued to the client, to notify the client they need to explicitly send aROLLBACKtovtgateto acknowledge the fact that their previous transaction is "dead". This is a way to ensure that applications do not keep sending queries, thinking they're in a transaction, even after receiving aVT15001. Once aROLLBACKis received,VT09032will not be returned to the client - until a newVT15001happens.A new field is added to VTGate sessions' protobuf:
error_until_rollback. This field is set totruewhen we return aVT15001error to the client, after this, all queries besidesROLLBACKwill fail and returnVT09032. Once we receive aROLLBACKwe set the field tofalse. We only set the field totruewhen doing a "normal" execution, transactional statements (begin,rollback, etc) failing withVT15001will not set this field totrueas there is either no transaction (in the case ofbegin), or the user is already at the end of the transaction (rollback,commit).go/test/endtoend/reparent/newfeaturetest/reparent_in_tx_test.gocontains several new E2E tests for the changes made by this PR.Documentation
Related Issue(s)