[release-24.0] vtorc: fix data race in forgetAliases cache initialization (#19843) (#19847)

Signed-off-by: Arthur Schreiber <arthur@planetscale.com>
Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: vitess-bot[bot]

go/vt/vtorc/inst/instance_dao.go

@@ -26,7 +26,6 @@ import (
 	"strconv"
 	"strings"
 	"sync"
-	"sync/atomic"
 	"time"
 
 	"github.com/patrickmn/go-cache"
@@ -54,31 +53,37 @@ var (
 	instanceWriteSem = semaphore.NewWeighted(config.GetBackendWriteConcurrency())
 )
 
-var forgetAliases *cache.Cache
+var (
+	forgetAliases     *cache.Cache
+	forgetAliasesOnce sync.Once
+)
 
 var (
 	readTopologyInstanceCounter = stats.NewCounter("InstanceReadTopology", "Number of times an instance was read from the topology")
 	readInstanceCounter         = stats.NewCounter("InstanceRead", "Number of times an instance was read")
 	currentErrantGTIDCount      = stats.NewGaugesWithSingleLabel("CurrentErrantGTIDCount", "Number of errant GTIDs a vttablet currently has", "TabletAlias")
 )
 
-var (
-	emptyQuotesRegexp            = regexp.MustCompile(`^""$`)
-	cacheInitializationCompleted atomic.Bool
-)
+var emptyQuotesRegexp = regexp.MustCompile(`^""$`)
 
 func init() {
 	go initializeInstanceDao()
 }
 
 func initializeInstanceDao() {
 	config.WaitForConfigurationToBeLoaded()
-	InitializeForgetAliasesCache()
+	initForgetAliasesCache()
+}
+
+func initForgetAliasesCache() {
+	forgetAliasesOnce.Do(func() {
+		forgetAliases = cache.New(config.GetInstancePollTime()*3, time.Second)
+	})
 }
 
+// InitializeForgetAliasesCache ensures the forgetAliases cache is initialized.
 func InitializeForgetAliasesCache() {
-	forgetAliases = cache.New(config.GetInstancePollTime()*3, time.Second)
-	cacheInitializationCompleted.Store(true)
+	initForgetAliasesCache()
 }
 
 // ExecDBWriteFunc chooses how to execute a write onto the database: whether synchronously or not
@@ -1105,6 +1110,7 @@ func UpdateInstanceLastAttemptedCheck(tabletAlias *topodatapb.TabletAlias) error
 
 // InstanceIsForgotten returns true if an instance was forgotten.
 func InstanceIsForgotten(tabletAlias *topodatapb.TabletAlias) bool {
+	initForgetAliasesCache()
 	tabletAliasString := topoproto.TabletAliasString(tabletAlias)
 	_, found := forgetAliases.Get(tabletAliasString)
 	return found
@@ -1118,6 +1124,7 @@ func ForgetInstance(tabletAlias *topodatapb.TabletAlias) error {
 		log.Error(errMsg)
 		return errors.New(errMsg)
 	}
+	initForgetAliasesCache()
 	tabletAliasString := topoproto.TabletAliasString(tabletAlias)
 	forgetAliases.Set(tabletAliasString, true, cache.DefaultExpiration)
 	log.Info(fmt.Sprintf("Forgetting: %v", tabletAliasString))

go/vt/vtorc/inst/instance_dao_test.go

@@ -558,8 +558,8 @@ func TestReadOutdatedInstances(t *testing.T) {
 		},
 	}
 
-	// wait for the forgetAliases cache to be initialized to prevent data race.
-	waitForCacheInitialization()
+	// Ensure the forgetAliases cache is initialized before overriding it.
+	InitializeForgetAliasesCache()
 
 	// We are setting InstancePollSeconds to 59 minutes, just for the test.
 	oldVal := config.GetInstancePollTime()
@@ -747,8 +747,8 @@ func TestForgetInstanceAndInstanceIsForgotten(t *testing.T) {
 		},
 	}
 
-	// wait for the forgetAliases cache to be initialized to prevent data race.
-	waitForCacheInitialization()
+	// Ensure the forgetAliases cache is initialized before overriding it.
+	InitializeForgetAliasesCache()
 
 	oldCache := forgetAliases
 	// Clear the database after the test. The easiest way to do that is to run all the initialization commands again.
@@ -810,17 +810,6 @@ func TestSnapshotTopologies(t *testing.T) {
 	require.Equal(t, []string{"zone1-0000000100", "zone1-0000000101", "zone1-0000000112", "zone2-0000000200"}, tabletAliases)
 }
 
-// waitForCacheInitialization waits for the cache to be initialized to prevent data race in tests
-// that alter the cache or depend on its behaviour.
-func waitForCacheInitialization() {
-	for {
-		if cacheInitializationCompleted.Load() {
-			return
-		}
-		time.Sleep(100 * time.Millisecond)
-	}
-}
-
 func TestGetDatabaseState(t *testing.T) {
 	// Clear the database after the test. The easiest way to do that is to run all the initialization commands again.
 	defer func() {