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

Remove unnecessary usage of FLUSH TABLES WITH READ LOCK #4849

Merged
merged 1 commit into from
May 7, 2019
Merged
Show file tree
Hide file tree
Changes from all 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
7 changes: 2 additions & 5 deletions go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 1 addition & 3 deletions go/vt/mysqlctl/mysql_daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 1 addition & 10 deletions go/vt/mysqlctl/reparent.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member Author

@dasl- dasl- Apr 30, 2019

Choose a reason for hiding this comment

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

if you approve of this idea, we could even get rid of the DemoteMaster method and associated RPC calls, since this becomes just a wrapper around MasterPosition.

We can use the MasterPosition RPC definitions instead. I am not sure how to update the protobuf definitions everywhere though...

cmds := []string{
"FLUSH TABLES WITH READ LOCK",
"UNLOCK TABLES",
}
if err = mysqld.ExecuteSuperQueryList(context.TODO(), cmds); err != nil {
return rp, err
}
return mysqld.MasterPosition()
}

Expand Down
4 changes: 2 additions & 2 deletions go/vt/vttablet/tabletmanager/rpc_replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions go/vt/wrangler/testlib/planned_reparent_shard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down