From 4f9f7fecb1fbb3414e6defdba4e3efb2e66b9852 Mon Sep 17 00:00:00 2001 From: William Lu Date: Wed, 12 Apr 2023 21:17:12 +0000 Subject: [PATCH] Fix convertBoolToSemiSyncAction method to account for all semi sync actions Signed-off-by: William Lu --- .../reparent/plannedreparent/reparent_test.go | 35 +++++++++++++++++++ .../fakemysqldaemon/fakemysqldaemon.go | 5 +++ go/vt/mysqlctl/mysql_daemon.go | 1 + go/vt/mysqlctl/replication.go | 9 +++++ go/vt/vttablet/tabletmanager/rpc_actions.go | 9 +++-- .../vttablet/tabletmanager/rpc_replication.go | 16 ++++----- go/vt/vttablet/tabletmanager/tm_init.go | 2 +- 7 files changed, 65 insertions(+), 12 deletions(-) diff --git a/go/test/endtoend/reparent/plannedreparent/reparent_test.go b/go/test/endtoend/reparent/plannedreparent/reparent_test.go index d33992d5a81..0bd414e8925 100644 --- a/go/test/endtoend/reparent/plannedreparent/reparent_test.go +++ b/go/test/endtoend/reparent/plannedreparent/reparent_test.go @@ -360,6 +360,41 @@ func TestChangeTypeSemiSync(t *testing.T) { utils.CheckDBstatus(ctx, t, rdonly2, "Rpl_semi_sync_slave_status", "ON") } +// Tests that ChangeTabletType works even when semi-sync plugins are not loaded. +func TestChangeTypeWithoutSemiSync(t *testing.T) { + defer cluster.PanicHandler(t) + clusterInstance := utils.SetupReparentCluster(t, "none") + defer utils.TeardownCluster(clusterInstance) + tablets := clusterInstance.Keyspaces[0].Shards[0].Vttablets + + ctx := context.Background() + + primary, replica := tablets[0], tablets[1] + + // Unload semi sync plugins + for _, tablet := range tablets[0:4] { + qr := utils.RunSQL(ctx, t, "select @@global.super_read_only", tablet) + result := fmt.Sprintf("%v", qr.Rows[0][0].ToString()) + if result == "1" { + utils.RunSQL(ctx, t, "set global super_read_only = 0", tablet) + } + + utils.RunSQL(ctx, t, "UNINSTALL PLUGIN rpl_semi_sync_slave;", tablet) + utils.RunSQL(ctx, t, "UNINSTALL PLUGIN rpl_semi_sync_master;", tablet) + } + + utils.ValidateTopology(t, clusterInstance, true) + utils.CheckPrimaryTablet(t, clusterInstance, primary) + + // Change replica's type to rdonly + err := clusterInstance.VtctlclientProcess.ExecuteCommand("ChangeTabletType", replica.Alias, "rdonly") + require.NoError(t, err) + + // Change tablets type from rdonly back to replica + err = clusterInstance.VtctlclientProcess.ExecuteCommand("ChangeTabletType", replica.Alias, "replica") + require.NoError(t, err) +} + func TestReparentDoesntHangIfPrimaryFails(t *testing.T) { utilstest.SkipIfBinaryIsAboveVersion(t, 15, "vttablet") diff --git a/go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go b/go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go index 5402be5e540..2958ac3d0e6 100644 --- a/go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go +++ b/go/vt/mysqlctl/fakemysqldaemon/fakemysqldaemon.go @@ -643,6 +643,11 @@ func (fmd *FakeMysqlDaemon) SemiSyncClients() uint32 { return 0 } +// SemiSyncExtensionLoaded is part of the MysqlDaemon interface. +func (fmd *FakeMysqlDaemon) SemiSyncExtensionLoaded() bool { + return true +} + // SemiSyncSettings is part of the MysqlDaemon interface. func (fmd *FakeMysqlDaemon) SemiSyncSettings() (timeout uint64, numReplicas uint32) { return 10000000, 1 diff --git a/go/vt/mysqlctl/mysql_daemon.go b/go/vt/mysqlctl/mysql_daemon.go index c4c76224b3b..41268207d48 100644 --- a/go/vt/mysqlctl/mysql_daemon.go +++ b/go/vt/mysqlctl/mysql_daemon.go @@ -57,6 +57,7 @@ type MysqlDaemon interface { GetGTIDPurged(ctx context.Context) (mysql.Position, error) SetSemiSyncEnabled(source, replica bool) error SemiSyncEnabled() (source, replica bool) + SemiSyncExtensionLoaded() bool SemiSyncStatus() (source, replica bool) SemiSyncClients() (count uint32) SemiSyncSettings() (timeout uint64, numReplicas uint32) diff --git a/go/vt/mysqlctl/replication.go b/go/vt/mysqlctl/replication.go index 4aba50b8903..f1ac2b4b72a 100644 --- a/go/vt/mysqlctl/replication.go +++ b/go/vt/mysqlctl/replication.go @@ -664,3 +664,12 @@ func (mysqld *Mysqld) SemiSyncReplicationStatus() (bool, error) { } return false, nil } + +func (mysqld *Mysqld) SemiSyncExtensionLoaded() bool { + qr, err := mysqld.FetchSuperQuery(context.TODO(), "SELECT COUNT(*) > 0 AS plugin_loaded FROM information_schema.plugins WHERE plugin_name LIKE 'rpl_semi_sync%'") + if err != nil { + return false + } + pluginPresent, _ := qr.Rows[0][0].ToBool() + return pluginPresent +} diff --git a/go/vt/vttablet/tabletmanager/rpc_actions.go b/go/vt/vttablet/tabletmanager/rpc_actions.go index 1093c331a1a..27a8795a7a6 100644 --- a/go/vt/vttablet/tabletmanager/rpc_actions.go +++ b/go/vt/vttablet/tabletmanager/rpc_actions.go @@ -82,7 +82,7 @@ func (tm *TabletManager) ChangeType(ctx context.Context, tabletType topodatapb.T return err } defer tm.unlock() - return tm.changeTypeLocked(ctx, tabletType, DBActionNone, convertBoolToSemiSyncAction(semiSync)) + return tm.changeTypeLocked(ctx, tabletType, DBActionNone, tm.convertBoolToSemiSyncAction(semiSync)) } // ChangeType changes the tablet type @@ -142,9 +142,12 @@ func (tm *TabletManager) RunHealthCheck(ctx context.Context) { tm.QueryServiceControl.BroadcastHealth() } -func convertBoolToSemiSyncAction(semiSync bool) SemiSyncAction { +func (tm *TabletManager) convertBoolToSemiSyncAction(semiSync bool) SemiSyncAction { if semiSync { return SemiSyncActionSet } - return SemiSyncActionUnset + if tm.MysqlDaemon.SemiSyncExtensionLoaded() { + return SemiSyncActionUnset + } + return SemiSyncActionNone } diff --git a/go/vt/vttablet/tabletmanager/rpc_replication.go b/go/vt/vttablet/tabletmanager/rpc_replication.go index 3bee3383b3a..83dc87cccaf 100644 --- a/go/vt/vttablet/tabletmanager/rpc_replication.go +++ b/go/vt/vttablet/tabletmanager/rpc_replication.go @@ -294,7 +294,7 @@ func (tm *TabletManager) StartReplication(ctx context.Context, semiSync bool) er } }() - if err := tm.fixSemiSync(tm.Tablet().Type, convertBoolToSemiSyncAction(semiSync)); err != nil { + if err := tm.fixSemiSync(tm.Tablet().Type, tm.convertBoolToSemiSyncAction(semiSync)); err != nil { return err } return tm.MysqlDaemon.StartReplication(tm.hookExtraEnv()) @@ -380,13 +380,13 @@ func (tm *TabletManager) InitPrimary(ctx context.Context, semiSync bool) (string // 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.changeTypeLocked(ctx, topodatapb.TabletType_PRIMARY, DBActionSetReadWrite, convertBoolToSemiSyncAction(semiSync)); err != nil { + if err := tm.changeTypeLocked(ctx, topodatapb.TabletType_PRIMARY, DBActionSetReadWrite, tm.convertBoolToSemiSyncAction(semiSync)); err != nil { return "", err } // Enforce semi-sync after changing the tablet)type to PRIMARY. Otherwise, the // primary will hang while trying to create the database. - if err := tm.fixSemiSync(topodatapb.TabletType_PRIMARY, convertBoolToSemiSyncAction(semiSync)); err != nil { + if err := tm.fixSemiSync(topodatapb.TabletType_PRIMARY, tm.convertBoolToSemiSyncAction(semiSync)); err != nil { return "", err } @@ -427,7 +427,7 @@ func (tm *TabletManager) InitReplica(ctx context.Context, parent *topodatapb.Tab // is used on the old primary when using InitShardPrimary with // -force, and the new primary is different from the old primary. if tm.Tablet().Type == topodatapb.TabletType_PRIMARY { - if err := tm.changeTypeLocked(ctx, topodatapb.TabletType_REPLICA, DBActionNone, convertBoolToSemiSyncAction(semiSync)); err != nil { + if err := tm.changeTypeLocked(ctx, topodatapb.TabletType_REPLICA, DBActionNone, tm.convertBoolToSemiSyncAction(semiSync)); err != nil { return err } } @@ -450,7 +450,7 @@ func (tm *TabletManager) InitReplica(ctx context.Context, parent *topodatapb.Tab if tt == topodatapb.TabletType_PRIMARY { tt = topodatapb.TabletType_REPLICA } - if err := tm.fixSemiSync(tt, convertBoolToSemiSyncAction(semiSync)); err != nil { + if err := tm.fixSemiSync(tt, tm.convertBoolToSemiSyncAction(semiSync)); err != nil { return err } @@ -602,7 +602,7 @@ func (tm *TabletManager) UndoDemotePrimary(ctx context.Context, semiSync bool) e defer tm.unlock() // If using semi-sync, we need to enable source-side. - if err := tm.fixSemiSync(topodatapb.TabletType_PRIMARY, convertBoolToSemiSyncAction(semiSync)); err != nil { + if err := tm.fixSemiSync(topodatapb.TabletType_PRIMARY, tm.convertBoolToSemiSyncAction(semiSync)); err != nil { return err } @@ -672,7 +672,7 @@ func (tm *TabletManager) SetReplicationSource(ctx context.Context, parentAlias * // setReplicationSourceLocked also fixes the semi-sync. In case the tablet type is primary it assumes that it will become a replica if SetReplicationSource // is called, so we always call fixSemiSync with a non-primary tablet type. This will always set the source side replication to false. - return tm.setReplicationSourceLocked(ctx, parentAlias, timeCreatedNS, waitPosition, forceStartReplication, convertBoolToSemiSyncAction(semiSync)) + return tm.setReplicationSourceLocked(ctx, parentAlias, timeCreatedNS, waitPosition, forceStartReplication, tm.convertBoolToSemiSyncAction(semiSync)) } func (tm *TabletManager) setReplicationSourceRepairReplication(ctx context.Context, parentAlias *topodatapb.TabletAlias, timeCreatedNS int64, waitPosition string, forceStartReplication bool) (err error) { @@ -965,7 +965,7 @@ func (tm *TabletManager) PromoteReplica(ctx context.Context, semiSync bool) (str } // If using semi-sync, we need to enable it before going read-write. - if err := tm.fixSemiSync(topodatapb.TabletType_PRIMARY, convertBoolToSemiSyncAction(semiSync)); err != nil { + if err := tm.fixSemiSync(topodatapb.TabletType_PRIMARY, tm.convertBoolToSemiSyncAction(semiSync)); err != nil { return "", err } diff --git a/go/vt/vttablet/tabletmanager/tm_init.go b/go/vt/vttablet/tabletmanager/tm_init.go index cf1ea9f3d62..9983b30c64c 100644 --- a/go/vt/vttablet/tabletmanager/tm_init.go +++ b/go/vt/vttablet/tabletmanager/tm_init.go @@ -910,7 +910,7 @@ func (tm *TabletManager) initializeReplication(ctx context.Context, tabletType t // If using semi-sync, we need to enable it before connecting to primary. // We should set the correct type, since it is used in replica semi-sync tablet.Type = tabletType - if err := tm.fixSemiSync(tabletType, convertBoolToSemiSyncAction(reparentutil.IsReplicaSemiSync(durability, currentPrimary.Tablet, tablet))); err != nil { + if err := tm.fixSemiSync(tabletType, tm.convertBoolToSemiSyncAction(reparentutil.IsReplicaSemiSync(durability, currentPrimary.Tablet, tablet))); err != nil { return nil, err }