From 790e8fab44476cc2960b16ebe2f966d77ebb52ef Mon Sep 17 00:00:00 2001 From: dleibovic Date: Tue, 30 Apr 2019 16:53:16 -0400 Subject: [PATCH] Remove unnecessary usage of FLUSH TABLES WITH READ LOCK During PlannedReparentShard, the steps vitess takes to get replication position are basically (on the old master): 1. set @@global.read_only=true 2. FLUSH TABLES WITH READ LOCK 3. UNLOCK TABLES 4. SELECT @@GLOBAL.gtid_executed Steps 2 and 3 should not be necessary. After setting read_only=true, we should be able to get a consistent gtid_executed position. And FTWRL needs to wait for long running SELECTs to finish, so it seems unnecessarily expensive. By eliminating steps 2 and 3 we could reduce the amount of time that vitess is unable to serve queries. Signed-off-by: dleibovic --- go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go | 7 ++----- go/vt/mysqlctl/mysql_daemon.go | 4 +--- go/vt/mysqlctl/reparent.go | 11 +---------- go/vt/vttablet/tabletmanager/rpc_replication.go | 4 ++-- go/vt/wrangler/testlib/planned_reparent_shard_test.go | 8 ++++---- 5 files changed, 10 insertions(+), 24 deletions(-) diff --git a/go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go b/go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go index 9e3011b9d1c..166ed5a01c0 100644 --- a/go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go +++ b/go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go @@ -90,9 +90,6 @@ type FakeMysqlDaemon struct { // (as "%v:%v"). If it doesn't match, SetMaster will return an error. SetMasterInput string - // DemoteMasterPosition is returned by DemoteMaster - DemoteMasterPosition mysql.Position - // WaitMasterPosition is checked by WaitMasterPos, if the // same it returns nil, if different it returns an error WaitMasterPosition mysql.Position @@ -307,9 +304,9 @@ func (fmd *FakeMysqlDaemon) WaitForReparentJournal(ctx context.Context, timeCrea return nil } -// DemoteMaster is part of the MysqlDaemon interface +// Deprecated: use mysqld.MasterPosition() instead func (fmd *FakeMysqlDaemon) DemoteMaster() (mysql.Position, error) { - return fmd.DemoteMasterPosition, nil + return fmd.CurrentMasterPosition, nil } // WaitMasterPos is part of the MysqlDaemon interface diff --git a/go/vt/mysqlctl/mysql_daemon.go b/go/vt/mysqlctl/mysql_daemon.go index b3e62f9d3c8..93ba4a718ce 100644 --- a/go/vt/mysqlctl/mysql_daemon.go +++ b/go/vt/mysqlctl/mysql_daemon.go @@ -58,9 +58,7 @@ type MysqlDaemon interface { SetMaster(ctx context.Context, masterHost string, masterPort int, slaveStopBefore bool, slaveStartAfter bool) error WaitForReparentJournal(ctx context.Context, timeCreatedNS int64) error - // DemoteMaster waits for all current transactions to finish, - // and returns the current replication position. It will not - // change the read_only state of the server. + // Deprecated: use MasterPosition() instead DemoteMaster() (mysql.Position, error) WaitMasterPos(context.Context, mysql.Position) error diff --git a/go/vt/mysqlctl/reparent.go b/go/vt/mysqlctl/reparent.go index 803838a4d05..9cd884bae71 100644 --- a/go/vt/mysqlctl/reparent.go +++ b/go/vt/mysqlctl/reparent.go @@ -90,17 +90,8 @@ func (mysqld *Mysqld) WaitForReparentJournal(ctx context.Context, timeCreatedNS } } -// DemoteMaster will gracefully demote a master mysql instance to read only. -// If the master is still alive, then we need to demote it gracefully -// make it read-only, flush the writes and get the position +// Deprecated: use mysqld.MasterPosition() instead func (mysqld *Mysqld) DemoteMaster() (rp mysql.Position, err error) { - cmds := []string{ - "FLUSH TABLES WITH READ LOCK", - "UNLOCK TABLES", - } - if err = mysqld.ExecuteSuperQueryList(context.TODO(), cmds); err != nil { - return rp, err - } return mysqld.MasterPosition() } diff --git a/go/vt/vttablet/tabletmanager/rpc_replication.go b/go/vt/vttablet/tabletmanager/rpc_replication.go index 49413f44b16..9ef1439791b 100644 --- a/go/vt/vttablet/tabletmanager/rpc_replication.go +++ b/go/vt/vttablet/tabletmanager/rpc_replication.go @@ -364,9 +364,9 @@ func (agent *ActionAgent) DemoteMaster(ctx context.Context) (string, error) { return "", err } - pos, err := agent.MysqlDaemon.DemoteMaster() + pos, err := agent.MysqlDaemon.MasterPosition() if err != nil { - // if DemoteMaster failed, undo all the steps before + // if MasterPosition failed, undo all the steps before // 1. set server back to read-only false // setting read_only OFF will also set super_read_only OFF if it was set if err1 := agent.MysqlDaemon.SetReadOnly(false); err1 != nil { diff --git a/go/vt/wrangler/testlib/planned_reparent_shard_test.go b/go/vt/wrangler/testlib/planned_reparent_shard_test.go index 36448189d42..eae9ef390b3 100644 --- a/go/vt/wrangler/testlib/planned_reparent_shard_test.go +++ b/go/vt/wrangler/testlib/planned_reparent_shard_test.go @@ -75,7 +75,7 @@ func TestPlannedReparentShardNoMasterProvided(t *testing.T) { // old master oldMaster.FakeMysqlDaemon.ReadOnly = false oldMaster.FakeMysqlDaemon.Replicating = false - oldMaster.FakeMysqlDaemon.DemoteMasterPosition = newMaster.FakeMysqlDaemon.WaitMasterPosition + oldMaster.FakeMysqlDaemon.CurrentMasterPosition = newMaster.FakeMysqlDaemon.WaitMasterPosition oldMaster.FakeMysqlDaemon.SetMasterInput = topoproto.MysqlAddr(newMaster.Tablet) oldMaster.FakeMysqlDaemon.ExpectedExecuteSuperQueryList = []string{ "FAKE SET MASTER", @@ -181,7 +181,7 @@ func TestPlannedReparentShard(t *testing.T) { // old master oldMaster.FakeMysqlDaemon.ReadOnly = false oldMaster.FakeMysqlDaemon.Replicating = false - oldMaster.FakeMysqlDaemon.DemoteMasterPosition = newMaster.FakeMysqlDaemon.WaitMasterPosition + oldMaster.FakeMysqlDaemon.CurrentMasterPosition = newMaster.FakeMysqlDaemon.WaitMasterPosition oldMaster.FakeMysqlDaemon.SetMasterInput = topoproto.MysqlAddr(newMaster.Tablet) oldMaster.FakeMysqlDaemon.ExpectedExecuteSuperQueryList = []string{ "FAKE SET MASTER", @@ -328,7 +328,7 @@ func TestPlannedReparentShardPromoteSlaveFail(t *testing.T) { oldMaster.FakeMysqlDaemon.ReadOnly = false oldMaster.FakeMysqlDaemon.Replicating = false // set to incorrect value to make promote fail on WaitForMasterPos - oldMaster.FakeMysqlDaemon.DemoteMasterPosition = newMaster.FakeMysqlDaemon.PromoteSlaveResult + oldMaster.FakeMysqlDaemon.CurrentMasterPosition = newMaster.FakeMysqlDaemon.PromoteSlaveResult oldMaster.FakeMysqlDaemon.SetMasterInput = topoproto.MysqlAddr(newMaster.Tablet) oldMaster.FakeMysqlDaemon.ExpectedExecuteSuperQueryList = []string{ "FAKE SET MASTER", @@ -424,7 +424,7 @@ func TestPlannedReparentShardPromoteSlaveTimeout(t *testing.T) { // old master oldMaster.FakeMysqlDaemon.ReadOnly = false oldMaster.FakeMysqlDaemon.Replicating = false - oldMaster.FakeMysqlDaemon.DemoteMasterPosition = newMaster.FakeMysqlDaemon.WaitMasterPosition + oldMaster.FakeMysqlDaemon.CurrentMasterPosition = newMaster.FakeMysqlDaemon.WaitMasterPosition oldMaster.FakeMysqlDaemon.SetMasterInput = topoproto.MysqlAddr(newMaster.Tablet) oldMaster.FakeMysqlDaemon.ExpectedExecuteSuperQueryList = []string{ "FAKE SET MASTER",