test: fix tests by doing nil checks and adding semi-sync monitor to f…

…ake tablet Signed-off-by: Manan Gupta <manan@planetscale.com>
planetscale · Feb 18, 2025 · 942eaf0 · 942eaf0
1 parent 6cf02ab
commit 942eaf0
Show file tree

Hide file tree

Showing 8 changed files with 72 additions and 10 deletions.
diff --git a/go/cmd/vtctldclient/command/framework_test.go b/go/cmd/vtctldclient/command/framework_test.go
@@ -30,12 +30,14 @@ import (
 	"vitess.io/vitess/go/vt/binlog/binlogplayer"
 	"vitess.io/vitess/go/vt/dbconfigs"
 	"vitess.io/vitess/go/vt/mysqlctl"
+	"vitess.io/vitess/go/vt/servenv"
 	"vitess.io/vitess/go/vt/topo"
 	"vitess.io/vitess/go/vt/topo/topoproto"
 	"vitess.io/vitess/go/vt/vtenv"
 	"vitess.io/vitess/go/vt/vttablet/grpctmserver"
 	"vitess.io/vitess/go/vt/vttablet/tabletconntest"
 	"vitess.io/vitess/go/vt/vttablet/tabletmanager"
+	"vitess.io/vitess/go/vt/vttablet/tabletmanager/semisyncmonitor"
 	"vitess.io/vitess/go/vt/vttablet/tabletmanager/vreplication"
 	"vitess.io/vitess/go/vt/vttablet/tabletservermock"
 	"vitess.io/vitess/go/vt/vttablet/tmclient"
@@ -158,6 +160,10 @@ func NewFakeTablet(t *testing.T, ts *topo.Server, cell string, uid uint32, table
 	}
 }
 
+var (
+	exporter = servenv.NewExporter("TestVtctldClientCommand", "")
+)
+
 // StartActionLoop will start the action loop for a fake tablet,
 // using ft.FakeMysqlDaemon as the backing mysqld.
 func (ft *FakeTablet) StartActionLoop(t *testing.T, ts *topo.Server) {
@@ -203,6 +209,7 @@ func (ft *FakeTablet) StartActionLoop(t *testing.T, ts *topo.Server) {
 		DBConfigs:           &dbconfigs.DBConfigs{},
 		QueryServiceControl: tabletservermock.NewController(),
 		VREngine:            vreplication.NewTestEngine(ts, ft.Tablet.Alias.Cell, ft.FakeMysqlDaemon, binlogplayer.NewFakeDBClient, binlogplayer.NewFakeDBClient, topoproto.TabletDbName(ft.Tablet), nil),
+		SemiSyncMonitor:     semisyncmonitor.CreateTestSemiSyncMonitor(ft.FakeMysqlDaemon.DB(), exporter),
 		Env:                 vtenv.NewTestEnv(),
 	}
 	if err := ft.TM.Start(ft.Tablet, nil); err != nil {

diff --git a/go/vt/mysqlctl/fakemysqldaemon.go b/go/vt/mysqlctl/fakemysqldaemon.go
@@ -217,6 +217,11 @@ func NewFakeMysqlDaemon(db *fakesqldb.DB) *FakeMysqlDaemon {
 	return result
 }
 
+// DB returns the fakesqldb.DB object.
+func (fmd *FakeMysqlDaemon) DB() *fakesqldb.DB {
+	return fmd.db
+}
+
 // Start is part of the MysqlDaemon interface.
 func (fmd *FakeMysqlDaemon) Start(ctx context.Context, cnf *Mycnf, mysqldArgs ...string) error {
 	if fmd.Running {

diff --git a/go/vt/vttablet/tabletmanager/rpc_replication.go b/go/vt/vttablet/tabletmanager/rpc_replication.go
@@ -1052,18 +1052,24 @@ func (tm *TabletManager) fixSemiSync(ctx context.Context, tabletType topodatapb.
 	case SemiSyncActionNone:
 		return nil
 	case SemiSyncActionSet:
-		// We want to enable the semi-sync monitor only if the tablet is going to start
-		// expecting semi-sync ACKs.
-		if tabletType == topodatapb.TabletType_PRIMARY {
-			tm.SemiSyncMonitor.Open()
-		} else {
-			tm.SemiSyncMonitor.Close()
+		if tm.SemiSyncMonitor != nil {
+			// We want to enable the semi-sync monitor only if the tablet is going to start
+			// expecting semi-sync ACKs.
+			if tabletType == topodatapb.TabletType_PRIMARY {
+				tm.SemiSyncMonitor.Open()
+			} else {
+				tm.SemiSyncMonitor.Close()
+			}
 		}
 		// Always enable replica-side since it doesn't hurt to keep it on for a primary.
 		// The primary-side needs to be off for a replica, or else it will get stuck.
 		return tm.MysqlDaemon.SetSemiSyncEnabled(ctx, tabletType == topodatapb.TabletType_PRIMARY, true)
 	case SemiSyncActionUnset:
-		tm.SemiSyncMonitor.Close()
+		// The nil check is required for vtcombo, which doesn't run the semi-sync monitor
+		// but does try to turn off semi-sync.
+		if tm.SemiSyncMonitor != nil {
+			tm.SemiSyncMonitor.Close()
+		}
 		return tm.MysqlDaemon.SetSemiSyncEnabled(ctx, false, false)
 	default:
 		return vterrors.Errorf(vtrpc.Code_INTERNAL, "Unknown SemiSyncAction - %v", semiSync)

diff --git a/go/vt/vttablet/tabletmanager/rpc_replication_test.go b/go/vt/vttablet/tabletmanager/rpc_replication_test.go
@@ -26,6 +26,7 @@ import (
 	"golang.org/x/sync/semaphore"
 
 	"vitess.io/vitess/go/vt/topo"
+	"vitess.io/vitess/go/vt/vttablet/tabletmanager/semisyncmonitor"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver"
 )
 
@@ -77,15 +78,17 @@ func TestDemotePrimaryStalled(t *testing.T) {
 		waitTime: 2 * time.Second,
 	}
 	// Create a tablet manager with a replica type tablet.
+	fakeDb := newTestMysqlDaemon(t, 1)
 	tm := &TabletManager{
 		actionSema:  semaphore.NewWeighted(1),
-		MysqlDaemon: newTestMysqlDaemon(t, 1),
+		MysqlDaemon: fakeDb,
 		tmState: &tmState{
 			displayState: displayState{
 				tablet: newTestTablet(t, 100, "ks", "-", map[string]string{}),
 			},
 		},
 		QueryServiceControl: qsc,
+		SemiSyncMonitor:     semisyncmonitor.CreateTestSemiSyncMonitor(fakeDb.DB(), exporter),
 	}
 
 	// We make IsServing stall for over 2 seconds, which is longer than 10 * remote operation timeout.

diff --git a/go/vt/vttablet/tabletmanager/semisyncmonitor/monitor.go b/go/vt/vttablet/tabletmanager/semisyncmonitor/monitor.go
@@ -23,8 +23,10 @@ import (
 	"time"
 
 	"vitess.io/vitess/go/constants/sidecar"
+	"vitess.io/vitess/go/mysql/fakesqldb"
 	"vitess.io/vitess/go/stats"
 	"vitess.io/vitess/go/timer"
+	"vitess.io/vitess/go/vt/dbconfigs"
 	"vitess.io/vitess/go/vt/dbconnpool"
 	"vitess.io/vitess/go/vt/log"
 	"vitess.io/vitess/go/vt/mysqlctl"
@@ -97,11 +99,29 @@ func NewMonitor(config *tabletenv.TabletConfig, exporter *servenv.Exporter) *Mon
 	}
 }
 
+// CreateTestSemiSyncMonitor created a monitor for testing.
+// It takes an optional fake db.
+func CreateTestSemiSyncMonitor(db *fakesqldb.DB, exporter *servenv.Exporter) *Monitor {
+	var dbc *dbconfigs.DBConfigs
+	if db != nil {
+		params := db.ConnParams()
+		cp := *params
+		dbc = dbconfigs.NewTestDBConfigs(cp, cp, "")
+	}
+	return NewMonitor(&tabletenv.TabletConfig{
+		DB: dbc,
+		SemiSyncMonitor: tabletenv.SemiSyncMonitorConfig{
+			Interval: 10 * time.Second,
+		},
+	}, exporter)
+}
+
 // Open starts the monitor.
 func (m *Monitor) Open() {
 	m.mu.Lock()
 	defer m.mu.Unlock()
-	if m.isOpen {
+	// The check for config being nil is only requried for tests.
+	if m.isOpen || m.config == nil || m.config.DB == nil {
 		// If we are already open, then there is nothing to do
 		return
 	}

diff --git a/go/vt/vttablet/tabletmanager/tm_init_test.go b/go/vt/vttablet/tabletmanager/tm_init_test.go
@@ -39,6 +39,7 @@ import (
 	"vitess.io/vitess/go/vt/topo"
 	"vitess.io/vitess/go/vt/topo/memorytopo"
 	"vitess.io/vitess/go/vt/topotools"
+	"vitess.io/vitess/go/vt/vttablet/tabletmanager/semisyncmonitor"
 	"vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv"
 	"vitess.io/vitess/go/vt/vttablet/tabletservermock"
 	"vitess.io/vitess/go/vt/vttest"
@@ -683,6 +684,10 @@ func newTestMysqlDaemon(t *testing.T, port int32) *mysqlctl.FakeMysqlDaemon {
 	return mysqld
 }
 
+var (
+	exporter = servenv.NewExporter("TestTabletManager", "")
+)
+
 func newTestTM(t *testing.T, ts *topo.Server, uid int, keyspace, shard string, tags map[string]string) *TabletManager {
 	// reset stats
 	statsTabletTags.ResetAll()
@@ -691,11 +696,13 @@ func newTestTM(t *testing.T, ts *topo.Server, uid int, keyspace, shard string, t
 	t.Helper()
 	ctx := context.Background()
 	tablet := newTestTablet(t, uid, keyspace, shard, tags)
+	fakeDb := newTestMysqlDaemon(t, 1)
 	tm := &TabletManager{
 		BatchCtx:            ctx,
 		TopoServer:          ts,
-		MysqlDaemon:         newTestMysqlDaemon(t, 1),
+		MysqlDaemon:         fakeDb,
 		DBConfigs:           &dbconfigs.DBConfigs{},
+		SemiSyncMonitor:     semisyncmonitor.CreateTestSemiSyncMonitor(fakeDb.DB(), exporter),
 		QueryServiceControl: tabletservermock.NewController(),
 	}
 	err := tm.Start(tablet, nil)

diff --git a/go/vt/wrangler/fake_tablet_test.go b/go/vt/wrangler/fake_tablet_test.go
@@ -33,6 +33,7 @@ import (
 	"vitess.io/vitess/go/vt/mysqlctl"
 	querypb "vitess.io/vitess/go/vt/proto/query"
 	topodatapb "vitess.io/vitess/go/vt/proto/topodata"
+	"vitess.io/vitess/go/vt/servenv"
 	"vitess.io/vitess/go/vt/sqlparser"
 	"vitess.io/vitess/go/vt/topo"
 	"vitess.io/vitess/go/vt/vtenv"
@@ -41,6 +42,7 @@ import (
 	"vitess.io/vitess/go/vt/vttablet/queryservice/fakes"
 	"vitess.io/vitess/go/vt/vttablet/tabletconntest"
 	"vitess.io/vitess/go/vt/vttablet/tabletmanager"
+	"vitess.io/vitess/go/vt/vttablet/tabletmanager/semisyncmonitor"
 	vdiff2 "vitess.io/vitess/go/vt/vttablet/tabletmanager/vdiff"
 	"vitess.io/vitess/go/vt/vttablet/tabletservermock"
 	"vitess.io/vitess/go/vt/vttablet/tmclient"
@@ -158,6 +160,10 @@ func newFakeTablet(t *testing.T, wr *Wrangler, cell string, uid uint32, tabletTy
 	}
 }
 
+var (
+	exporter = servenv.NewExporter("TestWrangler", "")
+)
+
 // StartActionLoop will start the action loop for a fake tablet,
 // using ft.FakeMysqlDaemon as the backing mysqld.
 func (ft *fakeTablet) StartActionLoop(t *testing.T, wr *Wrangler) {
@@ -199,6 +205,7 @@ func (ft *fakeTablet) StartActionLoop(t *testing.T, wr *Wrangler) {
 		DBConfigs:           &dbconfigs.DBConfigs{},
 		QueryServiceControl: tabletservermock.NewController(),
 		VDiffEngine:         vdiff2.NewEngine(wr.TopoServer(), ft.Tablet, collations.MySQL8(), sqlparser.NewTestParser()),
+		SemiSyncMonitor:     semisyncmonitor.CreateTestSemiSyncMonitor(ft.FakeMysqlDaemon.DB(), exporter),
 		Env:                 vtenv.NewTestEnv(),
 	}
 	if err := ft.TM.Start(ft.Tablet, nil); err != nil {

diff --git a/go/vt/wrangler/testlib/fake_tablet.go b/go/vt/wrangler/testlib/fake_tablet.go
@@ -33,12 +33,14 @@ import (
 	"vitess.io/vitess/go/vt/binlog/binlogplayer"
 	"vitess.io/vitess/go/vt/dbconfigs"
 	"vitess.io/vitess/go/vt/mysqlctl"
+	"vitess.io/vitess/go/vt/servenv"
 	"vitess.io/vitess/go/vt/topo"
 	"vitess.io/vitess/go/vt/topo/topoproto"
 	"vitess.io/vitess/go/vt/vtenv"
 	"vitess.io/vitess/go/vt/vttablet/grpctmserver"
 	"vitess.io/vitess/go/vt/vttablet/tabletconntest"
 	"vitess.io/vitess/go/vt/vttablet/tabletmanager"
+	"vitess.io/vitess/go/vt/vttablet/tabletmanager/semisyncmonitor"
 	"vitess.io/vitess/go/vt/vttablet/tabletmanager/vreplication"
 	"vitess.io/vitess/go/vt/vttablet/tabletservermock"
 	"vitess.io/vitess/go/vt/vttablet/tmclient"
@@ -159,6 +161,10 @@ func NewFakeTablet(t *testing.T, wr *wrangler.Wrangler, cell string, uid uint32,
 	}
 }
 
+var (
+	exporter = servenv.NewExporter("TestWranglerTestLib", "")
+)
+
 // StartActionLoop will start the action loop for a fake tablet,
 // using ft.FakeMysqlDaemon as the backing mysqld.
 func (ft *FakeTablet) StartActionLoop(t *testing.T, wr *wrangler.Wrangler) {
@@ -202,6 +208,7 @@ func (ft *FakeTablet) StartActionLoop(t *testing.T, wr *wrangler.Wrangler) {
 		DBConfigs:           &dbconfigs.DBConfigs{},
 		QueryServiceControl: tabletservermock.NewController(),
 		VREngine:            vreplication.NewTestEngine(wr.TopoServer(), ft.Tablet.Alias.Cell, ft.FakeMysqlDaemon, binlogplayer.NewFakeDBClient, binlogplayer.NewFakeDBClient, topoproto.TabletDbName(ft.Tablet), nil),
+		SemiSyncMonitor:     semisyncmonitor.CreateTestSemiSyncMonitor(ft.FakeMysqlDaemon.DB(), exporter),
 		Env:                 vtenv.NewTestEnv(),
 	}
 	if err := ft.TM.Start(ft.Tablet, nil); err != nil {