Allow for dynamic configuration of DDL settings

This allows configuring the vtgate DDL flags dynamically. With this,
both online DDL and direct DDL can be disabled and enabled in vtgate
without having to restart.

Signed-off-by: Dirkjan Bussink <[email protected]>

go/vt/vtgate/dynamicconfig/config.go

@@ -0,0 +1,6 @@
+package dynamicconfig
+
+type DDL interface {
+	OnlineEnabled() bool
+	DirectEnabled() bool
+}

go/vt/vtgate/engine/ddl.go

@@ -25,6 +25,7 @@ import (
 	"vitess.io/vitess/go/vt/schema"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/dynamicconfig"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 
@@ -42,8 +43,7 @@ type DDL struct {
 	NormalDDL *Send
 	OnlineDDL *OnlineDDL
 
-	DirectDDLEnabled bool
-	OnlineDDLEnabled bool
+	Config dynamicconfig.DDL
 
 	CreateTempTable bool
 }
@@ -107,12 +107,12 @@ func (ddl *DDL) TryExecute(ctx context.Context, vcursor VCursor, bindVars map[st
 
 	switch {
 	case ddl.isOnlineSchemaDDL():
-		if !ddl.OnlineDDLEnabled {
+		if !ddl.Config.OnlineEnabled() {
 			return nil, schema.ErrOnlineDDLDisabled
 		}
 		return vcursor.ExecutePrimitive(ctx, ddl.OnlineDDL, bindVars, wantfields)
 	default: // non online-ddl
-		if !ddl.DirectDDLEnabled {
+		if !ddl.Config.DirectEnabled() {
 			return nil, schema.ErrDirectDDLDisabled
 		}
 		return vcursor.ExecutePrimitive(ctx, ddl.NormalDDL, bindVars, wantfields)

go/vt/vtgate/engine/ddl_test.go

@@ -27,13 +27,23 @@ import (
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
 )
 
+type ddlConfig struct{}
+
+func (ddlConfig) DirectEnabled() bool {
+	return true
+}
+
+func (ddlConfig) OnlineEnabled() bool {
+	return true
+}
+
 func TestDDL(t *testing.T) {
 	ddl := &DDL{
 		DDL: &sqlparser.CreateTable{
 			Table: sqlparser.NewTableName("a"),
 		},
-		DirectDDLEnabled: true,
-		OnlineDDL:        &OnlineDDL{},
+		Config:    ddlConfig{},
+		OnlineDDL: &OnlineDDL{},
 		NormalDDL: &Send{
 			Keyspace: &vindexes.Keyspace{
 				Name:    "ks",

go/vt/vtgate/executor.go

@@ -1174,7 +1174,11 @@ func (e *Executor) buildStatement(
 	reservedVars *sqlparser.ReservedVars,
 	bindVarNeeds *sqlparser.BindVarNeeds,
 ) (*engine.Plan, error) {
-	plan, err := planbuilder.BuildFromStmt(ctx, query, stmt, reservedVars, vcursor, bindVarNeeds, enableOnlineDDL, enableDirectDDL)
+	cfg := &dynamicViperConfig{
+		onlineDDL: enableOnlineDDL,
+		directDDL: enableDirectDDL,
+	}
+	plan, err := planbuilder.BuildFromStmt(ctx, query, stmt, reservedVars, vcursor, bindVarNeeds, cfg)
 	if err != nil {
 		return nil, err
 	}

go/vt/vtgate/executor_ddl_test.go

@@ -27,8 +27,8 @@ import (
 
 func TestDDLFlags(t *testing.T) {
 	defer func() {
-		enableOnlineDDL = true
-		enableDirectDDL = true
+		enableOnlineDDL.Set(true)
+		enableDirectDDL.Set(true)
 	}()
 	testcases := []struct {
 		enableDirectDDL bool
@@ -57,8 +57,8 @@ func TestDDLFlags(t *testing.T) {
 		t.Run(fmt.Sprintf("%s-%v-%v", testcase.sql, testcase.enableDirectDDL, testcase.enableOnlineDDL), func(t *testing.T) {
 			executor, _, _, _, ctx := createExecutorEnv(t)
 			session := NewSafeSession(&vtgatepb.Session{TargetString: KsTestUnsharded})
-			enableDirectDDL = testcase.enableDirectDDL
-			enableOnlineDDL = testcase.enableOnlineDDL
+			enableDirectDDL.Set(testcase.enableDirectDDL)
+			enableOnlineDDL.Set(testcase.enableOnlineDDL)
 			_, err := executor.Execute(ctx, nil, "TestDDLFlags", session, testcase.sql, nil)
 			if testcase.wantErr {
 				require.EqualError(t, err, testcase.err)

go/vt/vtgate/planbuilder/builder.go

@@ -28,6 +28,7 @@ import (
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/dynamicconfig"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
@@ -63,6 +64,16 @@ func singleTable(ks, tbl string) string {
 	return fmt.Sprintf("%s.%s", ks, tbl)
 }
 
+type staticConfig struct{}
+
+func (staticConfig) OnlineEnabled() bool {
+	return true
+}
+
+func (staticConfig) DirectEnabled() bool {
+	return true
+}
+
 // TestBuilder builds a plan for a query based on the specified vschema.
 // This method is only used from tests
 func TestBuilder(query string, vschema plancontext.VSchema, keyspace string) (*engine.Plan, error) {
@@ -92,12 +103,12 @@ func TestBuilder(query string, vschema plancontext.VSchema, keyspace string) (*e
 	}
 
 	reservedVars := sqlparser.NewReservedVars("vtg", reserved)
-	return BuildFromStmt(context.Background(), query, result.AST, reservedVars, vschema, result.BindVarNeeds, true, true)
+	return BuildFromStmt(context.Background(), query, result.AST, reservedVars, vschema, result.BindVarNeeds, staticConfig{})
 }
 
 // BuildFromStmt builds a plan based on the AST provided.
-func BuildFromStmt(ctx context.Context, query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, bindVarNeeds *sqlparser.BindVarNeeds, enableOnlineDDL, enableDirectDDL bool) (*engine.Plan, error) {
-	planResult, err := createInstructionFor(ctx, query, stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+func BuildFromStmt(ctx context.Context, query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, bindVarNeeds *sqlparser.BindVarNeeds, cfg dynamicconfig.DDL) (*engine.Plan, error) {
+	planResult, err := createInstructionFor(ctx, query, stmt, reservedVars, vschema, cfg)
 	if err != nil {
 		return nil, err
 	}
@@ -154,7 +165,7 @@ func buildRoutePlan(stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVa
 	return f(stmt, reservedVars, vschema)
 }
 
-func createInstructionFor(ctx context.Context, query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, enableOnlineDDL, enableDirectDDL bool) (*planResult, error) {
+func createInstructionFor(ctx context.Context, query string, stmt sqlparser.Statement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
 	switch stmt := stmt.(type) {
 	case *sqlparser.Select, *sqlparser.Insert, *sqlparser.Update, *sqlparser.Delete:
 		configuredPlanner, err := getConfiguredPlanner(vschema, stmt, query)
@@ -169,13 +180,13 @@ func createInstructionFor(ctx context.Context, query string, stmt sqlparser.Stat
 		}
 		return buildRoutePlan(stmt, reservedVars, vschema, configuredPlanner)
 	case sqlparser.DDLStatement:
-		return buildGeneralDDLPlan(ctx, query, stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+		return buildGeneralDDLPlan(ctx, query, stmt, reservedVars, vschema, cfg)
 	case *sqlparser.AlterMigration:
-		return buildAlterMigrationPlan(query, stmt, vschema, enableOnlineDDL)
+		return buildAlterMigrationPlan(query, stmt, vschema, cfg)
 	case *sqlparser.RevertMigration:
-		return buildRevertMigrationPlan(query, stmt, vschema, enableOnlineDDL)
+		return buildRevertMigrationPlan(query, stmt, vschema, cfg)
 	case *sqlparser.ShowMigrationLogs:
-		return buildShowMigrationLogsPlan(query, vschema, enableOnlineDDL)
+		return buildShowMigrationLogsPlan(query, vschema, cfg)
 	case *sqlparser.ShowThrottledApps:
 		return buildShowThrottledAppsPlan(query, vschema)
 	case *sqlparser.ShowThrottlerStatus:
@@ -189,7 +200,7 @@ func createInstructionFor(ctx context.Context, query string, stmt sqlparser.Stat
 	case *sqlparser.ExplainStmt:
 		return buildRoutePlan(stmt, reservedVars, vschema, buildExplainStmtPlan)
 	case *sqlparser.VExplainStmt:
-		return buildVExplainPlan(ctx, stmt, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+		return buildVExplainPlan(ctx, stmt, reservedVars, vschema, cfg)
 	case *sqlparser.OtherAdmin:
 		return buildOtherReadAndAdmin(query, vschema)
 	case *sqlparser.Analyze:

go/vt/vtgate/planbuilder/ddl.go

@@ -9,6 +9,7 @@ import (
 	vschemapb "vitess.io/vitess/go/vt/proto/vschema"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/dynamicconfig"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
@@ -43,11 +44,11 @@ func (fk *fkContraint) FkWalk(node sqlparser.SQLNode) (kontinue bool, err error)
 // a session context. It's only when we Execute() the primitive that we have that context.
 // This is why we return a compound primitive (DDL) which contains fully populated primitives (Send & OnlineDDL),
 // and which chooses which of the two to invoke at runtime.
-func buildGeneralDDLPlan(ctx context.Context, sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, enableOnlineDDL, enableDirectDDL bool) (*planResult, error) {
+func buildGeneralDDLPlan(ctx context.Context, sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
 	if vschema.Destination() != nil {
 		return buildByPassPlan(sql, vschema, true)
 	}
-	normalDDLPlan, onlineDDLPlan, err := buildDDLPlans(ctx, sql, ddlStatement, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+	normalDDLPlan, onlineDDLPlan, err := buildDDLPlans(ctx, sql, ddlStatement, reservedVars, vschema, cfg)
 	if err != nil {
 		return nil, err
 	}
@@ -61,15 +62,12 @@ func buildGeneralDDLPlan(ctx context.Context, sql string, ddlStatement sqlparser
 	}
 
 	eddl := &engine.DDL{
-		Keyspace:  normalDDLPlan.Keyspace,
-		SQL:       normalDDLPlan.Query,
-		DDL:       ddlStatement,
-		NormalDDL: normalDDLPlan,
-		OnlineDDL: onlineDDLPlan,
-
-		DirectDDLEnabled: enableDirectDDL,
-		OnlineDDLEnabled: enableOnlineDDL,
-
+		Keyspace:        normalDDLPlan.Keyspace,
+		SQL:             normalDDLPlan.Query,
+		DDL:             ddlStatement,
+		NormalDDL:       normalDDLPlan,
+		OnlineDDL:       onlineDDLPlan,
+		Config:          cfg,
 		CreateTempTable: ddlStatement.IsTemporary(),
 	}
 	tc := &tableCollector{}
@@ -94,7 +92,7 @@ func buildByPassPlan(sql string, vschema plancontext.VSchema, isDDL bool) (*plan
 	return newPlanResult(send), nil
 }
 
-func buildDDLPlans(ctx context.Context, sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, enableOnlineDDL, enableDirectDDL bool) (*engine.Send, *engine.OnlineDDL, error) {
+func buildDDLPlans(ctx context.Context, sql string, ddlStatement sqlparser.DDLStatement, reservedVars *sqlparser.ReservedVars, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*engine.Send, *engine.OnlineDDL, error) {
 	var destination key.Destination
 	var keyspace *vindexes.Keyspace
 	var err error
@@ -113,9 +111,9 @@ func buildDDLPlans(ctx context.Context, sql string, ddlStatement sqlparser.DDLSt
 		}
 		err = checkFKError(vschema, ddlStatement, keyspace)
 	case *sqlparser.CreateView:
-		destination, keyspace, err = buildCreateViewCommon(ctx, vschema, reservedVars, enableOnlineDDL, enableDirectDDL, ddl.Select, ddl)
+		destination, keyspace, err = buildCreateViewCommon(ctx, vschema, reservedVars, cfg, ddl.Select, ddl)
 	case *sqlparser.AlterView:
-		destination, keyspace, err = buildCreateViewCommon(ctx, vschema, reservedVars, enableOnlineDDL, enableDirectDDL, ddl.Select, ddl)
+		destination, keyspace, err = buildCreateViewCommon(ctx, vschema, reservedVars, cfg, ddl.Select, ddl)
 	case *sqlparser.DropView:
 		destination, keyspace, err = buildDropView(vschema, ddlStatement)
 	case *sqlparser.DropTable:
@@ -197,7 +195,7 @@ func buildCreateViewCommon(
 	ctx context.Context,
 	vschema plancontext.VSchema,
 	reservedVars *sqlparser.ReservedVars,
-	enableOnlineDDL, enableDirectDDL bool,
+	cfg dynamicconfig.DDL,
 	ddlSelect sqlparser.SelectStatement,
 	ddl sqlparser.DDLStatement,
 ) (key.Destination, *vindexes.Keyspace, error) {
@@ -214,7 +212,7 @@ func buildCreateViewCommon(
 		expressions = append(expressions, sqlparser.Clone(p.SelectExprs))
 		return nil
 	})
-	selectPlan, err := createInstructionFor(ctx, sqlparser.String(ddlSelect), ddlSelect, reservedVars, vschema, enableOnlineDDL, enableDirectDDL)
+	selectPlan, err := createInstructionFor(ctx, sqlparser.String(ddlSelect), ddlSelect, reservedVars, vschema, cfg)
 	if err != nil {
 		return nil, nil, err
 	}

go/vt/vtgate/planbuilder/migration.go

@@ -27,6 +27,7 @@ import (
 	"vitess.io/vitess/go/vt/schema"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/vterrors"
+	"vitess.io/vitess/go/vt/vtgate/dynamicconfig"
 	"vitess.io/vitess/go/vt/vtgate/engine"
 	"vitess.io/vitess/go/vt/vtgate/planbuilder/plancontext"
 	"vitess.io/vitess/go/vt/vtgate/vindexes"
@@ -80,8 +81,8 @@ func buildAlterMigrationThrottleAppPlan(query string, alterMigration *sqlparser.
 	}), nil
 }
 
-func buildAlterMigrationPlan(query string, alterMigration *sqlparser.AlterMigration, vschema plancontext.VSchema, enableOnlineDDL bool) (*planResult, error) {
-	if !enableOnlineDDL {
+func buildAlterMigrationPlan(query string, alterMigration *sqlparser.AlterMigration, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
+	if !cfg.OnlineEnabled() {
 		return nil, schema.ErrOnlineDDLDisabled
 	}
 
@@ -118,8 +119,8 @@ func buildAlterMigrationPlan(query string, alterMigration *sqlparser.AlterMigrat
 	return newPlanResult(send), nil
 }
 
-func buildRevertMigrationPlan(query string, stmt *sqlparser.RevertMigration, vschema plancontext.VSchema, enableOnlineDDL bool) (*planResult, error) {
-	if !enableOnlineDDL {
+func buildRevertMigrationPlan(query string, stmt *sqlparser.RevertMigration, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
+	if !cfg.OnlineEnabled() {
 		return nil, schema.ErrOnlineDDLDisabled
 	}
 	dest, ks, tabletType, err := vschema.TargetDestination("")
@@ -147,8 +148,8 @@ func buildRevertMigrationPlan(query string, stmt *sqlparser.RevertMigration, vsc
 	return newPlanResult(emig), nil
 }
 
-func buildShowMigrationLogsPlan(query string, vschema plancontext.VSchema, enableOnlineDDL bool) (*planResult, error) {
-	if !enableOnlineDDL {
+func buildShowMigrationLogsPlan(query string, vschema plancontext.VSchema, cfg dynamicconfig.DDL) (*planResult, error) {
+	if !cfg.OnlineEnabled() {
 		return nil, schema.ErrOnlineDDLDisabled
 	}
 	dest, ks, tabletType, err := vschema.TargetDestination("")

go/vt/vtgate/planbuilder/simplifier_test.go

@@ -135,12 +135,12 @@ func keepSameError(query string, reservedVars *sqlparser.ReservedVars, vschema *
 	}
 	rewritten, _ := sqlparser.RewriteAST(stmt, vschema.CurrentDb(), sqlparser.SQLSelectLimitUnset, "", nil, nil, nil)
 	ast := rewritten.AST
-	_, expected := BuildFromStmt(context.Background(), query, ast, reservedVars, vschema, rewritten.BindVarNeeds, true, true)
+	_, expected := BuildFromStmt(context.Background(), query, ast, reservedVars, vschema, rewritten.BindVarNeeds, staticConfig{})
 	if expected == nil {
 		panic("query does not fail to plan")
 	}
 	return func(statement sqlparser.SelectStatement) bool {
-		_, myErr := BuildFromStmt(context.Background(), query, statement, reservedVars, vschema, needs, true, true)
+		_, myErr := BuildFromStmt(context.Background(), query, statement, reservedVars, vschema, needs, staticConfig{})
 		if myErr == nil {
 			return false
 		}
@@ -162,7 +162,7 @@ func keepPanicking(query string, reservedVars *sqlparser.ReservedVars, vschema *
 			}
 		}()
 		log.Errorf("trying %s", sqlparser.String(statement))
-		_, _ = BuildFromStmt(context.Background(), query, statement, reservedVars, vschema, needs, true, true)
+		_, _ = BuildFromStmt(context.Background(), query, statement, reservedVars, vschema, needs, staticConfig{})
 		log.Errorf("did not panic")
 
 		return false