From 91a2751e50e7d491f1f66d571ffc2db1095fc872 Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Sat, 25 Jul 2020 13:32:46 -0700 Subject: [PATCH 1/2] vttablet: automatically create db if absent This functionality facilitates the use case where we connect to an externally managed mysql, but whose database has not been created yet. If the DB we look for is not found, we can just create it. Signed-off-by: Sugu Sougoumarane --- go/test/endtoend/tabletmanager/tablet_test.go | 32 +++++++++++++ go/vt/dbconfigs/dbconfigs.go | 5 ++ go/vt/dbconfigs/dbconfigs_test.go | 3 ++ go/vt/vttablet/tabletserver/schema/engine.go | 48 +++++++++++++++++++ go/vt/vttablet/tabletserver/state_manager.go | 13 ++--- .../tabletserver/state_manager_test.go | 20 ++++++-- 6 files changed, 110 insertions(+), 11 deletions(-) diff --git a/go/test/endtoend/tabletmanager/tablet_test.go b/go/test/endtoend/tabletmanager/tablet_test.go index 8277ee13aed..a90b6e313b3 100644 --- a/go/test/endtoend/tabletmanager/tablet_test.go +++ b/go/test/endtoend/tabletmanager/tablet_test.go @@ -20,12 +20,44 @@ import ( "fmt" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "vitess.io/vitess/go/test/endtoend/cluster" "vitess.io/vitess/go/vt/log" ) +// TestEnsureDB tests that vttablet creates the db as needed +func TestEnsureDB(t *testing.T) { + defer cluster.PanicHandler(t) + + // Create new tablet + tablet := clusterInstance.NewVttabletInstance("replica", 0, "") + tablet.MysqlctlProcess = *cluster.MysqlCtlProcessInstance(tablet.TabletUID, tablet.MySQLPort, clusterInstance.TmpDirectory) + err := tablet.MysqlctlProcess.Start() + require.NoError(t, err) + + log.Info(fmt.Sprintf("Started vttablet %v", tablet)) + // Start vttablet process as replica. It won't be able to serve because there's no db. + err = clusterInstance.StartVttablet(tablet, "NOT_SERVING", false, cell, "dbtest", hostname, "0") + require.NoError(t, err) + + // Make it the master. + err = clusterInstance.VtctlclientProcess.ExecuteCommand("TabletExternallyReparented", tablet.Alias) + require.NoError(t, err) + + // It will still fail because the db is read-only. + assert.Equal(t, "NOT_SERVING", tablet.VttabletProcess.GetTabletStatus()) + status := tablet.VttabletProcess.GetStatusDetails() + assert.Contains(t, status, "read-only") + + // Switch to read-write and verify that that we go serving. + _ = clusterInstance.VtctlclientProcess.ExecuteCommand("SetReadWrite", tablet.Alias) + err = tablet.VttabletProcess.WaitForTabletType("SERVING") + require.NoError(t, err) + killTablets(t, tablet) +} + // TestLocalMetadata tests the contents of local_metadata table after vttablet startup func TestLocalMetadata(t *testing.T) { defer cluster.PanicHandler(t) diff --git a/go/vt/dbconfigs/dbconfigs.go b/go/vt/dbconfigs/dbconfigs.go index efabbe9d5e8..2c5d5441531 100644 --- a/go/vt/dbconfigs/dbconfigs.go +++ b/go/vt/dbconfigs/dbconfigs.go @@ -223,6 +223,11 @@ func (dbcfgs *DBConfigs) AppDebugWithDB() Connector { return dbcfgs.makeParams(&dbcfgs.appdebugParams, true) } +// AllPrivsConnector returns connection parameters for appdebug with no dbname set. +func (dbcfgs *DBConfigs) AllPrivsConnector() Connector { + return dbcfgs.makeParams(&dbcfgs.allprivsParams, false) +} + // AllPrivsWithDB returns connection parameters for appdebug with dbname set. func (dbcfgs *DBConfigs) AllPrivsWithDB() Connector { return dbcfgs.makeParams(&dbcfgs.allprivsParams, true) diff --git a/go/vt/dbconfigs/dbconfigs_test.go b/go/vt/dbconfigs/dbconfigs_test.go index 57f5b695b45..04acb3f9c15 100644 --- a/go/vt/dbconfigs/dbconfigs_test.go +++ b/go/vt/dbconfigs/dbconfigs_test.go @@ -227,6 +227,9 @@ func TestAccessors(t *testing.T) { if got, want := dbc.AppWithDB().connParams.DbName, "db"; got != want { t.Errorf("dbc.AppWithDB().DbName: %v, want %v", got, want) } + if got, want := dbc.AllPrivsConnector().connParams.DbName, ""; got != want { + t.Errorf("dbc.AllPrivsWithDB().DbName: %v, want %v", got, want) + } if got, want := dbc.AllPrivsWithDB().connParams.DbName, "db"; got != want { t.Errorf("dbc.AllPrivsWithDB().DbName: %v, want %v", got, want) } diff --git a/go/vt/vttablet/tabletserver/schema/engine.go b/go/vt/vttablet/tabletserver/schema/engine.go index d033c0fac5c..cdbcb635d9a 100644 --- a/go/vt/vttablet/tabletserver/schema/engine.go +++ b/go/vt/vttablet/tabletserver/schema/engine.go @@ -24,6 +24,7 @@ import ( "sync" "time" + "vitess.io/vitess/go/vt/dbconnpool" "vitess.io/vitess/go/vt/vtgate/evalengine" "golang.org/x/net/context" @@ -40,6 +41,7 @@ import ( "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" binlogdatapb "vitess.io/vitess/go/vt/proto/binlogdata" + topodatapb "vitess.io/vitess/go/vt/proto/topodata" vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) @@ -71,6 +73,9 @@ type Engine struct { conns *connpool.Pool ticks *timer.Timer + + // dbCreationFailed is for preventing log spam. + dbCreationFailed bool } // NewEngine creates a new Engine. @@ -110,6 +115,49 @@ func (se *Engine) InitDBConfig(cp dbconfigs.Connector) { se.cp = cp } +// EnsureConnectionAndDB ensures that we can connect to mysql. +// If tablet type is master and there is no db, then the database is created. +// This function can be called before opening the Engine. +func (se *Engine) EnsureConnectionAndDB(tabletType topodatapb.TabletType) error { + ctx := tabletenv.LocalContext() + conn, err := dbconnpool.NewDBConnection(ctx, se.env.Config().DB.AppWithDB()) + if err == nil { + conn.Close() + se.dbCreationFailed = false + return nil + } + if tabletType != topodatapb.TabletType_MASTER { + return err + } + if merr, isSQLErr := err.(*mysql.SQLError); !isSQLErr || merr.Num != mysql.ERBadDb { + return err + } + + // We are master and db is not found. Let's create it. + // We use allprivs instead of DBA because we want db create to fail if we're read-only. + conn, err = dbconnpool.NewDBConnection(ctx, se.env.Config().DB.AllPrivsConnector()) + if err != nil { + return vterrors.Wrap(err, "allprivs connection failed") + } + defer conn.Close() + + dbname := se.env.Config().DB.DBName + _, err = conn.ExecuteFetch(fmt.Sprintf("create database if not exists `%s`", dbname), 1, false) + if err != nil { + if !se.dbCreationFailed { + // This is the first failure. + log.Errorf("db creation failed for %v: %v, will keep retrying", dbname, err) + se.dbCreationFailed = true + } + return err + } + + se.dbCreationFailed = false + log.Infof("db %v created", dbname) + se.dbCreationFailed = false + return nil +} + // Open initializes the Engine. Calling Open on an already // open engine is a no-op. func (se *Engine) Open() error { diff --git a/go/vt/vttablet/tabletserver/state_manager.go b/go/vt/vttablet/tabletserver/state_manager.go index 00537d6e70b..9135dc38b8c 100644 --- a/go/vt/vttablet/tabletserver/state_manager.go +++ b/go/vt/vttablet/tabletserver/state_manager.go @@ -123,6 +123,7 @@ type stateManager struct { type ( schemaEngine interface { + EnsureConnectionAndDB(topodatapb.TabletType) error Open() error MakeNonMaster() Close() @@ -395,7 +396,7 @@ func (sm *stateManager) VerifyTarget(ctx context.Context, target *querypb.Target func (sm *stateManager) serveMaster() error { sm.watcher.Close() - if err := sm.connect(); err != nil { + if err := sm.connect(topodatapb.TabletType_MASTER); err != nil { return err } @@ -414,7 +415,7 @@ func (sm *stateManager) unserveMaster() error { sm.watcher.Close() - if err := sm.connect(); err != nil { + if err := sm.connect(topodatapb.TabletType_MASTER); err != nil { return err } @@ -428,7 +429,7 @@ func (sm *stateManager) serveNonMaster(wantTabletType topodatapb.TabletType) err sm.tracker.Close() sm.se.MakeNonMaster() - if err := sm.connect(); err != nil { + if err := sm.connect(wantTabletType); err != nil { return err } @@ -446,7 +447,7 @@ func (sm *stateManager) unserveNonMaster(wantTabletType topodatapb.TabletType) e sm.se.MakeNonMaster() - if err := sm.connect(); err != nil { + if err := sm.connect(wantTabletType); err != nil { return err } @@ -456,8 +457,8 @@ func (sm *stateManager) unserveNonMaster(wantTabletType topodatapb.TabletType) e return nil } -func (sm *stateManager) connect() error { - if err := sm.qe.IsMySQLReachable(); err != nil { +func (sm *stateManager) connect(tabletType topodatapb.TabletType) error { + if err := sm.se.EnsureConnectionAndDB(tabletType); err != nil { return err } if err := sm.se.Open(); err != nil { diff --git a/go/vt/vttablet/tabletserver/state_manager_test.go b/go/vt/vttablet/tabletserver/state_manager_test.go index 438fd347757..319ea21e772 100644 --- a/go/vt/vttablet/tabletserver/state_manager_test.go +++ b/go/vt/vttablet/tabletserver/state_manager_test.go @@ -79,7 +79,7 @@ func TestStateManagerServeMaster(t *testing.T) { verifySubcomponent(t, 9, sm.messager, testStateOpen) assert.False(t, sm.se.(*testSchemaEngine).nonMaster) - assert.True(t, sm.qe.(*testQueryEngine).isReachable) + assert.True(t, sm.se.(*testSchemaEngine).ensureCalled) assert.False(t, sm.qe.(*testQueryEngine).stopServing) assert.Equal(t, topodatapb.TabletType_MASTER, sm.target.TabletType) @@ -286,7 +286,7 @@ func TestStateManagerTransitionFailRetry(t *testing.T) { transitionRetryInterval = 10 * time.Millisecond sm := newTestStateManager(t) - sm.qe.(*testQueryEngine).failMySQL = true + sm.se.(*testSchemaEngine).failMySQL = true err := sm.SetServingType(topodatapb.TabletType_MASTER, testNow, StateServing, "") require.Error(t, err) @@ -613,7 +613,19 @@ func (tos testOrderState) State() testState { type testSchemaEngine struct { testOrderState - nonMaster bool + ensureCalled bool + nonMaster bool + + failMySQL bool +} + +func (te *testSchemaEngine) EnsureConnectionAndDB(tabletType topodatapb.TabletType) error { + if te.failMySQL { + te.failMySQL = false + return errors.New("intentional error") + } + te.ensureCalled = true + return nil } func (te *testSchemaEngine) Open() error { @@ -658,7 +670,6 @@ func (te *testReplTracker) Status() (time.Duration, error) { type testQueryEngine struct { testOrderState - isReachable bool stopServing bool failMySQL bool @@ -675,7 +686,6 @@ func (te *testQueryEngine) IsMySQLReachable() error { te.failMySQL = false return errors.New("intentional error") } - te.isReachable = true return nil } From bec97b80d951043e2de0e1c9e4fe4e8af2d94c8e Mon Sep 17 00:00:00 2001 From: Sugu Sougoumarane Date: Mon, 27 Jul 2020 20:21:02 -0700 Subject: [PATCH 2/2] vttablet: sequence InitMaster differently The feature to auo-create the database causes ISM to hang because it enforces semi-synch before it sets the tablet type. Enforcing semi-sync after tablet has become master allows the tabletserver to create the database without getting stuck waiting for yet-to-be-initialized replicas to ack. Signed-off-by: Sugu Sougoumarane --- go/vt/vttablet/tabletmanager/rpc_replication.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/go/vt/vttablet/tabletmanager/rpc_replication.go b/go/vt/vttablet/tabletmanager/rpc_replication.go index 1db7e7dccde..f6d9087584d 100644 --- a/go/vt/vttablet/tabletmanager/rpc_replication.go +++ b/go/vt/vttablet/tabletmanager/rpc_replication.go @@ -241,11 +241,6 @@ func (tm *TabletManager) InitMaster(ctx context.Context) (string, error) { return "", err } - // If using semi-sync, we need to enable it before going read-write. - if err := tm.fixSemiSync(topodatapb.TabletType_MASTER); err != nil { - return "", err - } - // Set the server read-write, from now on we can accept real // client writes. Note that if semi-sync replication is enabled, // we'll still need some replicas to be able to commit transactions. @@ -256,6 +251,13 @@ func (tm *TabletManager) InitMaster(ctx context.Context) (string, error) { if err := tm.changeTypeLocked(ctx, topodatapb.TabletType_MASTER); err != nil { return "", err } + + // Enforce semi-sync after changing the type to master. Otherwise, the + // master will hang while trying to create the database. + if err := tm.fixSemiSync(topodatapb.TabletType_MASTER); err != nil { + return "", err + } + return mysql.EncodePosition(pos), nil }