From 17f6dd7f071f254cf74f564585a7c94480b7b4a4 Mon Sep 17 00:00:00 2001 From: deepthi Date: Mon, 21 Sep 2020 15:18:53 -0700 Subject: [PATCH 1/2] tm: call setReadOnly inside ChangeTabletType so that a tablet doesn't advertise itself as MASTER while mysql is still read_only Signed-off-by: deepthi --- go/vt/vttablet/tabletmanager/rpc_replication.go | 6 ------ go/vt/vttablet/tabletmanager/tm_state.go | 5 +++++ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/go/vt/vttablet/tabletmanager/rpc_replication.go b/go/vt/vttablet/tabletmanager/rpc_replication.go index f6d9087584d..d5c1de80a0f 100644 --- a/go/vt/vttablet/tabletmanager/rpc_replication.go +++ b/go/vt/vttablet/tabletmanager/rpc_replication.go @@ -741,12 +741,6 @@ func (tm *TabletManager) PromoteReplica(ctx context.Context) (string, error) { return "", err } - // We call SetReadOnly only after the topo has been updated to avoid - // situations where two tablets are master at the DB level but not at the vitess level - if err := tm.MysqlDaemon.SetReadOnly(false); err != nil { - return "", err - } - return mysql.EncodePosition(pos), nil } diff --git a/go/vt/vttablet/tabletmanager/tm_state.go b/go/vt/vttablet/tabletmanager/tm_state.go index fad4a8b9048..80a4d5e05b5 100644 --- a/go/vt/vttablet/tabletmanager/tm_state.go +++ b/go/vt/vttablet/tabletmanager/tm_state.go @@ -163,6 +163,11 @@ func (ts *tmState) ChangeTabletType(ctx context.Context, tabletType topodatapb.T if err != nil { return err } + // We call SetReadOnly only after the topo has been updated to avoid + // situations where two tablets are master at the DB level but not at the vitess level + if err := ts.tm.MysqlDaemon.SetReadOnly(false); err != nil { + return err + } ts.tablet.Type = tabletType ts.tablet.MasterTermStartTime = masterTermStartTime From 7bfc73158991671ebcc6cd16f053febc71d192ce Mon Sep 17 00:00:00 2001 From: deepthi Date: Mon, 21 Sep 2020 20:00:34 -0700 Subject: [PATCH 2/2] tm: InitMaster does not need to call SetReadOnly because changeTypeLocked will. Fix tests Signed-off-by: deepthi --- go/test/endtoend/tabletmanager/tablet_test.go | 10 +++------- go/vt/vttablet/tabletmanager/rpc_replication.go | 4 ---- 2 files changed, 3 insertions(+), 11 deletions(-) diff --git a/go/test/endtoend/tabletmanager/tablet_test.go b/go/test/endtoend/tabletmanager/tablet_test.go index a90b6e313b3..8e6a1ad5a5e 100644 --- a/go/test/endtoend/tabletmanager/tablet_test.go +++ b/go/test/endtoend/tabletmanager/tablet_test.go @@ -46,15 +46,11 @@ func TestEnsureDB(t *testing.T) { 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()) + // It goes SERVING because TER calls ChangeTabletType which will also set the database to read-write + assert.Equal(t, "SERVING", tablet.VttabletProcess.GetTabletStatus()) status := tablet.VttabletProcess.GetStatusDetails() - assert.Contains(t, status, "read-only") + assert.Contains(t, status, "Serving") - // 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) } diff --git a/go/vt/vttablet/tabletmanager/rpc_replication.go b/go/vt/vttablet/tabletmanager/rpc_replication.go index d5c1de80a0f..9ca7d535088 100644 --- a/go/vt/vttablet/tabletmanager/rpc_replication.go +++ b/go/vt/vttablet/tabletmanager/rpc_replication.go @@ -244,10 +244,6 @@ func (tm *TabletManager) InitMaster(ctx context.Context) (string, error) { // 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. - if err := tm.MysqlDaemon.SetReadOnly(false); err != nil { - return "", err - } - if err := tm.changeTypeLocked(ctx, topodatapb.TabletType_MASTER); err != nil { return "", err }