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

Wrap errors happening during PRS in a new vterrors code #17669

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go/vt/discovery/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -872,7 +872,7 @@ func (hc *HealthCheckImpl) TabletConnection(ctx context.Context, alias *topodata
hc.mu.Unlock()
if thc == nil || thc.Conn == nil {
// TODO: test that throws this error
return nil, vterrors.Errorf(vtrpc.Code_NOT_FOUND, "tablet: %v is either down or nonexistent", alias)
return nil, vterrors.VT15001(vtrpc.Code_NOT_FOUND, fmt.Sprintf("tablet: %v is either down or nonexistent", alias))
}
return thc.Connection(ctx), nil
}
Expand Down
21 changes: 21 additions & 0 deletions go/vt/vterrors/code.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ var (
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", "session invalidated: close/reopen connection if applicable: %s", "This error means that the opened transaction should be closed by the application and re-opened.")
Copy link
Member

Choose a reason for hiding this comment

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

"session invalidated" may be misleading

  • if you are in a transaction, that needs to be closed, and a new transaction opened.
  • if you are not in a transaction, a retry is probably what you need to do.
    @harshit-gangal wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

It is the transaction which no longer exists. They can continue to use the same connection.
"rollback", followed by a retry is fine.
outside of transaction, retry is enough as Deepthi pointed.
All 3 error message should have different error code as they happen at different point in time of PRS/ERS


// Errors is a list of errors that must match all the variables
// defined above to enable auto-documentation of error codes.
Errors = []func(args ...any) *VitessError{
Expand Down Expand Up @@ -207,6 +209,10 @@ var (
VT14004,
VT14005,
}

ErrorsWithNoCode = []func(code vtrpcpb.Code, args ...any) *VitessError{
VT15001,
}
)

type VitessError struct {
Expand Down Expand Up @@ -258,3 +264,18 @@ func errorWithState(id string, code vtrpcpb.Code, state State, short, long strin
}
}
}

func errorWithNoCode(id string, short, long string) func(code vtrpcpb.Code, args ...any) *VitessError {
return func(code vtrpcpb.Code, args ...any) *VitessError {
s := short
if len(args) != 0 {
s = fmt.Sprintf(s, args...)
}

return &VitessError{
Err: New(code, id+": "+s),
Description: long,
ID: id,
}
}
}
18 changes: 16 additions & 2 deletions go/vt/vterrors/vterrorsgen/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"text/template"

"vitess.io/vitess/go/mysql/sqlerror"
vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc"

"vitess.io/vitess/go/vt/vterrors"
)
Expand All @@ -31,10 +32,14 @@ const (
tmpl = `
| ID | Description | Error | MySQL Error Code | SQL State |
| --- | --- | --- | --- | --- |
{{- range $err := . }}
{{- range $err := .Errors }}
{{- $data := (call $err) }}
| {{ $data.ID }} | {{ $data.Description }} | {{ FormatError $data.Err }} | {{ ConvertStateToMySQLErrorCode $data.State }} | {{ ConvertStateToMySQLState $data.State }} |
{{- end }}
{{- range $err := .ErrorsWithNoCode }}
{{- $data := (call $err 0) }}
| {{ $data.ID }} | {{ $data.Description }} | {{ FormatError $data.Err }} | | {{ ConvertStateToMySQLState $data.State }} |
{{- end }}
`
)

Expand All @@ -53,7 +58,16 @@ func main() {
})
t = template.Must(t.Parse(tmpl))

err := t.ExecuteTemplate(os.Stdout, "template", vterrors.Errors)
type data struct {
Errors []func(args ...any) *vterrors.VitessError
ErrorsWithNoCode []func(code vtrpcpb.Code, args ...any) *vterrors.VitessError
}
d := data{
Errors: vterrors.Errors,
ErrorsWithNoCode: vterrors.ErrorsWithNoCode,
}

err := t.ExecuteTemplate(os.Stdout, "template", d)
if err != nil {
log.Fatal(err)
}
Expand Down
13 changes: 12 additions & 1 deletion go/vt/vttablet/grpctabletconn/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,15 @@ package grpctabletconn
import (
"context"
"io"
"strings"
"sync"

"github.com/spf13/pflag"
"google.golang.org/grpc"

"vitess.io/vitess/go/vt/proto/vtrpc"
"vitess.io/vitess/go/vt/vterrors"

"vitess.io/vitess/go/netutil"
"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/callerid"
Expand Down Expand Up @@ -132,7 +136,14 @@ func (conn *gRPCQueryClient) Execute(ctx context.Context, target *querypb.Target
}
er, err := conn.c.Execute(ctx, req)
if err != nil {
return nil, tabletconn.ErrorFromGRPC(err)
err = tabletconn.ErrorFromGRPC(err)
if err == nil {
return nil, nil
}
if vterrors.Code(err) == vtrpc.Code_UNAVAILABLE && strings.Contains(err.Error(), "connection refused") {
return nil, vterrors.VT15001(vtrpc.Code_UNAVAILABLE, err.Error())
}
return nil, err
}
return sqltypes.Proto3ToResult(er.Result), nil
}
Expand Down
2 changes: 1 addition & 1 deletion go/vt/vttablet/tabletserver/state_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ func (sm *stateManager) verifyTargetLocked(ctx context.Context, target *querypb.
return nil
}
}
return vterrors.Errorf(vtrpcpb.Code_FAILED_PRECONDITION, "%s: %v, want: %v or %v", vterrors.WrongTablet, target.TabletType, sm.target.TabletType, sm.alsoAllow)
return vterrors.VT15001(vtrpcpb.Code_FAILED_PRECONDITION, fmt.Sprintf("%s: %v, want: %v or %v", vterrors.WrongTablet, target.TabletType, sm.target.TabletType, sm.alsoAllow))
}
} else {
if !tabletenv.IsLocalContext(ctx) {
Expand Down
Loading