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

Make SetMaster idempotent. #5300

Merged
merged 1 commit into from
Oct 15, 2019
Merged
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
132 changes: 80 additions & 52 deletions go/vt/vttablet/tabletmanager/rpc_replication.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,12 +334,17 @@ func (agent *ActionAgent) demoteMaster(ctx context.Context, revertPartialFailure
tablet := agent.Tablet()
wasMaster := tablet.Type == topodatapb.TabletType_MASTER
wasServing := agent.QueryServiceControl.IsServing()
wasReadOnly, err := agent.MysqlDaemon.IsReadOnly()
if err != nil {
return "", err
}

// If we are a master tablet, stop accepting new queries and wait for
// in-flight queries to complete. If we are not master, there's no need
// to stop the queryservice in order to ensure the guarantee we are being
// asked to provide, which is that no writes are occurring.
if wasMaster {
// If we are a master tablet and not yet read-only, stop accepting new
// queries and wait for in-flight queries to complete. If we are not master,
// or if we are already read-only, there's no need to stop the queryservice
// in order to ensure the guarantee we are being asked to provide, which is
// that no writes are occurring.
if wasMaster && !wasReadOnly {
// Tell Orchestrator we're stopped on purpose for demotion.
// This is a best effort task, so run it in a goroutine.
go func() {
Expand Down Expand Up @@ -373,10 +378,6 @@ func (agent *ActionAgent) demoteMaster(ctx context.Context, revertPartialFailure
// set MySQL to read-only mode. If we are already read-only because of a
// previous demotion, or because we are not master anyway, this should be
// idempotent.
wasReadOnly, err := agent.MysqlDaemon.IsReadOnly()
if err != nil {
return "", err
}
if *setSuperReadOnly {
// Setting super_read_only also sets read_only
if err := agent.MysqlDaemon.SetSuperReadOnly(true); err != nil {
Expand Down Expand Up @@ -517,7 +518,19 @@ func (agent *ActionAgent) SetMaster(ctx context.Context, parentAlias *topodatapb
}
defer agent.unlock()

return agent.setMasterLocked(ctx, parentAlias, timeCreatedNS, forceStartSlave)
if err := agent.setMasterLocked(ctx, parentAlias, timeCreatedNS, forceStartSlave); err != nil {
return err
}

// Always refresh the tablet, even if we may not have changed it.
// It's possible that we changed it earlier but failed to refresh.
// Note that we do this outside setMasterLocked() because this should never
// be done as part of setMasterRepairReplication().
if err := agent.refreshTablet(ctx, "SetMaster"); err != nil {
return err
}

return nil
}

func (agent *ActionAgent) setMasterRepairReplication(ctx context.Context, parentAlias *topodatapb.TabletAlias, timeCreatedNS int64, forceStartSlave bool) (err error) {
Expand All @@ -537,11 +550,6 @@ func (agent *ActionAgent) setMasterRepairReplication(ctx context.Context, parent
}

func (agent *ActionAgent) setMasterLocked(ctx context.Context, parentAlias *topodatapb.TabletAlias, timeCreatedNS int64, forceStartSlave bool) (err error) {
parent, err := agent.TopoServer.GetTablet(ctx, parentAlias)
if err != nil {
return err
}

// End orchestrator maintenance at the end of fixing replication.
// This is a best effort operation, so it should happen in a goroutine
defer func() {
Expand All @@ -555,66 +563,86 @@ func (agent *ActionAgent) setMasterLocked(ctx context.Context, parentAlias *topo
}()
}()

// See if we were replicating at all, and should be replicating
// Change our type to REPLICA if we used to be MASTER.
// Being sent SetMaster means another MASTER has been successfully promoted,
// so we convert to REPLICA first, since we want to do it even if other
// steps fail below.
_, err = agent.TopoServer.UpdateTabletFields(ctx, agent.TabletAlias, func(tablet *topodatapb.Tablet) error {
if tablet.Type == topodatapb.TabletType_MASTER {
tablet.Type = topodatapb.TabletType_REPLICA
return nil
}
return topo.NewError(topo.NoUpdateNeeded, agent.TabletAlias.String())
})
if err != nil {
return err
}

// See if we were replicating at all, and should be replicating.
wasReplicating := false
shouldbeReplicating := false
rs, err := agent.MysqlDaemon.SlaveStatus()
if err == nil && (rs.SlaveIORunning || rs.SlaveSQLRunning) {
wasReplicating = true
status, err := agent.MysqlDaemon.SlaveStatus()
if err == mysql.ErrNotSlave {
// This is a special error that means we actually succeeded in reading
// the status, but the status is empty because replication is not
// configured. We assume this means we used to be a master, so we always
// try to start replicating once we are told who the new master is.
shouldbeReplicating = true
} else if err == mysql.ErrNotSlave {
// If we used to be a master, or if we started as a replica but never
// found out who the master is, we always try to start replicating once
// we are told who the new master is via SetMaster.
// Since we continue in the case of this error, make sure 'status' is
// in a known, empty state.
status = mysql.SlaveStatus{}
} else if err != nil {
// Abort on any other non-nil error.
return err
}
if status.SlaveIORunning || status.SlaveSQLRunning {
wasReplicating = true
shouldbeReplicating = true
}
if forceStartSlave {
shouldbeReplicating = true
}

// If using semi-sync, we need to enable it before connecting to master.
if *enableSemiSync {
tt := agent.Tablet().Type
if tt == topodatapb.TabletType_MASTER {
tt = topodatapb.TabletType_REPLICA
}
if err := agent.fixSemiSync(tt); err != nil {
return err
}
// If we are currently MASTER, assume we are about to become REPLICA.
tabletType := agent.Tablet().Type
if tabletType == topodatapb.TabletType_MASTER {
tabletType = topodatapb.TabletType_REPLICA
}

// Sets the master.
if err := agent.MysqlDaemon.SetMaster(ctx, topoproto.MysqlHostname(parent.Tablet), int(topoproto.MysqlPort(parent.Tablet)), wasReplicating, shouldbeReplicating); err != nil {
if err := agent.fixSemiSync(tabletType); err != nil {
return err
}

// change our type to REPLICA if we used to be the master
typeChanged := false
_, err = agent.TopoServer.UpdateTabletFields(ctx, agent.TabletAlias, func(tablet *topodatapb.Tablet) error {
if tablet.Type == topodatapb.TabletType_MASTER {
tablet.Type = topodatapb.TabletType_REPLICA
typeChanged = true
return nil
}
return topo.NewError(topo.NoUpdateNeeded, agent.TabletAlias.String())
})
// Update the master address only if needed.
// We don't want to interrupt replication for no reason.
parent, err := agent.TopoServer.GetTablet(ctx, parentAlias)
if err != nil {
return err
}

// if needed, wait until we get the replicated row, or our
// context times out
if shouldbeReplicating && timeCreatedNS != 0 {
if err := agent.MysqlDaemon.WaitForReparentJournal(ctx, timeCreatedNS); err != nil {
masterHost := topoproto.MysqlHostname(parent.Tablet)
masterPort := int(topoproto.MysqlPort(parent.Tablet))
if status.MasterHost != masterHost || status.MasterPort != masterPort {
// This handles both changing the address and starting replication.
if err := agent.MysqlDaemon.SetMaster(ctx, masterHost, masterPort, wasReplicating, shouldbeReplicating); err != nil {
return err
}
} else if shouldbeReplicating {
// The address is correct. Just start replication if needed.
if !status.SlaveRunning() {
if err := agent.MysqlDaemon.StartSlave(agent.hookExtraEnv()); err != nil {
return err
}
}
}
if typeChanged {
if err := agent.refreshTablet(ctx, "SetMaster"); err != nil {

// If needed, wait until we replicate the specified row,
// or our context times out.
if shouldbeReplicating && timeCreatedNS != 0 {
if err := agent.MysqlDaemon.WaitForReparentJournal(ctx, timeCreatedNS); err != nil {
return err
}
agent.runHealthCheckLocked()
}

return nil
}

Expand Down