Skip to content
This repository has been archived by the owner on Dec 16, 2022. It is now read-only.

Fix bug in vtgate when running with keyspace filtering #199

Merged
merged 5 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
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
5 changes: 5 additions & 0 deletions go/vt/discovery/healthcheck.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,11 @@ func init() {
flag.Var(&KeyspacesToWatch, "keyspaces_to_watch", "Specifies which keyspaces this vtgate should have access to while routing queries or accessing the vschema")
}

// FilteringKeyspaces returns true if any keyspaces have been configured to be filtered.
func FilteringKeyspaces() bool {
return len(KeyspacesToWatch) > 0
}

// TabletRecorder is a sub interface of HealthCheck.
// It is separated out to enable unit testing.
type TabletRecorder interface {
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/discoverygateway.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ func NewDiscoveryGateway(ctx context.Context, hc discovery.LegacyHealthCheck, se
}
var recorder discovery.LegacyTabletRecorder = dg.hc
if len(discovery.TabletFilters) > 0 {
if len(discovery.KeyspacesToWatch) > 0 {
if discovery.FilteringKeyspaces() {
log.Exitf("Only one of -keyspaces_to_watch and -tablet_filters may be specified at a time")
}

Expand All @@ -140,7 +140,7 @@ func NewDiscoveryGateway(ctx context.Context, hc discovery.LegacyHealthCheck, se
log.Exitf("Cannot parse tablet_filters parameter: %v", err)
}
recorder = fbs
} else if len(discovery.KeyspacesToWatch) > 0 {
} else if discovery.FilteringKeyspaces() {
recorder = discovery.NewLegacyFilterByKeyspace(recorder, discovery.KeyspacesToWatch)
}

Expand Down
20 changes: 18 additions & 2 deletions go/vt/vtgate/vcursor_impl.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import (
"golang.org/x/net/context"

"vitess.io/vitess/go/sqltypes"
"vitess.io/vitess/go/vt/discovery"
"vitess.io/vitess/go/vt/key"
"vitess.io/vitess/go/vt/sqlparser"
"vitess.io/vitess/go/vt/srvtopo"
Expand Down Expand Up @@ -140,7 +141,17 @@ func (vc *vcursorImpl) ExecuteVSchema(keyspace string, vschemaDDL *sqlparser.DDL
// the query and supply it here. Trailing comments are typically sent by the application for various reasons,
// including as identifying markers. So, they have to be added back to all queries that are executed
// on behalf of the original query.
func newVCursorImpl(ctx context.Context, safeSession *SafeSession, marginComments sqlparser.MarginComments, executor *Executor, logStats *LogStats, vm VSchemaOperator, vschema *vindexes.VSchema, resolver *srvtopo.Resolver, serv srvtopo.Server) (*vcursorImpl, error) {
func newVCursorImpl(
ctx context.Context,
safeSession *SafeSession,
marginComments sqlparser.MarginComments,
executor *Executor,
logStats *LogStats,
vm VSchemaOperator,
vschema *vindexes.VSchema,
resolver *srvtopo.Resolver,
serv srvtopo.Server,
) (*vcursorImpl, error) {
keyspace, tabletType, destination, err := parseDestinationTarget(safeSession.TargetString, vschema)
if err != nil {
return nil, err
Expand All @@ -151,7 +162,9 @@ func newVCursorImpl(ctx context.Context, safeSession *SafeSession, marginComment
return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "newVCursorImpl: transactions are supported only for master tablet types, current type: %v", tabletType)
}
var ts *topo.Server
if serv != nil {
// We don't have access to the underlying TopoServer if this vtgate is
// filtering keyspaces because we don't have an accurate view of the topo.
if serv != nil && !discovery.FilteringKeyspaces() {
ts, err = serv.GetTopoServer()
if err != nil {
return nil, err
Expand Down Expand Up @@ -477,6 +490,9 @@ func (vc *vcursorImpl) TabletType() topodatapb.TabletType {

// SubmitOnlineDDL implements the VCursor interface
func (vc *vcursorImpl) SubmitOnlineDDL(onlineDDl *schema.OnlineDDL) error {
if vc.topoServer == nil {
return vterrors.New(vtrpcpb.Code_INTERNAL, "Unable to apply DDL, missing toposerver in vcursor")
Copy link

Choose a reason for hiding this comment

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

one nit - this error might be too cryptic for an end user. I wonder if we should mention that probably they have a keyspace filter and that is incompatible with online DDLs

Copy link
Author

Choose a reason for hiding this comment

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

Fair enough -- I referenced filtered keyspaces in the error but since it's a bit indirect i'm going to leave the general commentary around why the failure is happening as well.

}
conn, err := vc.topoServer.ConnForCell(vc.ctx, topo.GlobalCell)
if err != nil {
return err
Expand Down
4 changes: 2 additions & 2 deletions go/vt/vtgate/vtgate.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func Init(ctx context.Context, serv srvtopo.Server, cell string, tabletTypesToWa

// If we want to filter keyspaces replace the srvtopo.Server with a
// filtering server
if len(discovery.KeyspacesToWatch) > 0 {
if discovery.FilteringKeyspaces() {
log.Infof("Keyspace filtering enabled, selecting %v", discovery.KeyspacesToWatch)
var err error
serv, err = srvtopo.NewKeyspaceFilteringServer(serv, discovery.KeyspacesToWatch)
Expand Down Expand Up @@ -495,7 +495,7 @@ func LegacyInit(ctx context.Context, hc discovery.LegacyHealthCheck, serv srvtop

// If we want to filter keyspaces replace the srvtopo.Server with a
// filtering server
if len(discovery.KeyspacesToWatch) > 0 {
if discovery.FilteringKeyspaces() {
log.Infof("Keyspace filtering enabled, selecting %v", discovery.KeyspacesToWatch)
var err error
serv, err = srvtopo.NewKeyspaceFilteringServer(serv, discovery.KeyspacesToWatch)
Expand Down