Add wait for reading mycnf to prevent race (vitessio#14626)

Signed-off-by: Manan Gupta <[email protected]>

go/vt/mysqlctl/cmd.go

@@ -55,7 +55,7 @@ func CreateMysqldAndMycnf(tabletUID uint32, mysqlSocket string, mysqlPort int) (
 // of the MySQL instance.
 func OpenMysqldAndMycnf(tabletUID uint32) (*Mysqld, *Mycnf, error) {
 	// We pass a port of 0, this will be read and overwritten from the path on disk
-	mycnf, err := ReadMycnf(NewMycnf(tabletUID, 0))
+	mycnf, err := ReadMycnf(NewMycnf(tabletUID, 0), 0)
 	if err != nil {
 		return nil, nil, fmt.Errorf("couldn't read my.cnf file: %v", err)
 	}

go/vt/mysqlctl/mycnf.go

@@ -28,6 +28,7 @@ import (
 	"os"
 	"path"
 	"strconv"
+	"time"
 )
 
 // Mycnf is a memory structure that contains a bunch of interesting
@@ -112,6 +113,10 @@ type Mycnf struct {
 	Path     string // the actual path that represents this mycnf
 }
 
+const (
+	myCnfWaitRetryTime = 100 * time.Millisecond
+)
+
 // TabletDir returns the tablet directory.
 func (cnf *Mycnf) TabletDir() string {
 	return path.Dir(cnf.DataDir)
@@ -153,8 +158,20 @@ func normKey(bkey []byte) string {
 
 // ReadMycnf will read an existing my.cnf from disk, and update the passed in Mycnf object
 // with values from the my.cnf on disk.
-func ReadMycnf(mycnf *Mycnf) (*Mycnf, error) {
+func ReadMycnf(mycnf *Mycnf, waitTime time.Duration) (*Mycnf, error) {
 	f, err := os.Open(mycnf.Path)
+	if waitTime != 0 {
+		timer := time.NewTimer(waitTime)
+		for err != nil {
+			select {
+			case <-timer.C:
+				return nil, err
+			default:
+				time.Sleep(myCnfWaitRetryTime)
+				f, err = os.Open(mycnf.Path)
+			}
+		}
+	}
 	if err != nil {
 		return nil, err
 	}

go/vt/mysqlctl/mycnf_flag.go

@@ -17,6 +17,8 @@ limitations under the License.
 package mysqlctl
 
 import (
+	"time"
+
 	"github.com/spf13/pflag"
 
 	"vitess.io/vitess/go/vt/log"
@@ -51,6 +53,10 @@ var (
 	flagMycnfFile string
 )
 
+const (
+	waitForMyCnf = 10 * time.Second
+)
+
 // RegisterFlags registers the command line flags for
 // specifying the values of a mycnf config file. See NewMycnfFromFlags
 // to get the supported modes.
@@ -129,5 +135,5 @@ func NewMycnfFromFlags(uid uint32) (mycnf *Mycnf, err error) {
 	}
 	mycnf = NewMycnf(uid, 0)
 	mycnf.Path = flagMycnfFile
-	return ReadMycnf(mycnf)
+	return ReadMycnf(mycnf, waitForMyCnf)
 }

go/vt/mysqlctl/mycnf_test.go

@@ -20,7 +20,11 @@ import (
 	"bytes"
 	"os"
 	"strings"
+	"sync"
 	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
 
 	"vitess.io/vitess/go/vt/dbconfigs"
 	"vitess.io/vitess/go/vt/servenv"
@@ -29,6 +33,9 @@ import (
 var MycnfPath = "/tmp/my.cnf"
 
 func TestMycnf(t *testing.T) {
+	// Remove any my.cnf file if it already exists.
+	os.Remove(MycnfPath)
+
 	uid := uint32(11111)
 	cnf := NewMycnf(uid, 6802)
 	myTemplateSource := new(bytes.Buffer)
@@ -39,36 +46,45 @@ func TestMycnf(t *testing.T) {
 	f, _ := os.ReadFile("../../../config/mycnf/default.cnf")
 	myTemplateSource.Write(f)
 	data, err := cnf.makeMycnf(myTemplateSource.String())
-	if err != nil {
-		t.Errorf("err: %v", err)
-	} else {
-		t.Logf("data: %v", data)
-	}
-	err = os.WriteFile(MycnfPath, []byte(data), 0666)
-	if err != nil {
-		t.Errorf("failed creating my.cnf %v", err)
-	}
-	_, err = os.ReadFile(MycnfPath)
-	if err != nil {
-		t.Errorf("failed reading, err %v", err)
-		return
-	}
+	require.NoError(t, err)
+	t.Logf("data: %v", data)
+
+	// Since there is no my.cnf file, reading it should fail with a no such file error.
 	mycnf := NewMycnf(uid, 0)
 	mycnf.Path = MycnfPath
-	mycnf, err = ReadMycnf(mycnf)
-	if err != nil {
-		t.Errorf("failed reading, err %v", err)
-	} else {
+	_, err = ReadMycnf(mycnf, 0)
+	require.ErrorContains(t, err, "no such file or directory")
+
+	// Next up we will spawn a go-routine to try and read the cnf file with a timeout.
+	// We will create the cnf file after some delay and verify that ReadMycnf does wait that long
+	// and ends up succeeding in reading the my.cnf file.
+	waitTime := 1 * time.Second
+	wg := sync.WaitGroup{}
+	wg.Add(1)
+
+	go func() {
+		defer wg.Done()
+		startTime := time.Now()
+		var readErr error
+		mycnf, readErr = ReadMycnf(mycnf, 1*time.Minute)
+		require.NoError(t, readErr, "failed reading")
 		t.Logf("socket file %v", mycnf.SocketFile)
-	}
+		totalTimeSpent := time.Since(startTime)
+		require.GreaterOrEqual(t, totalTimeSpent, waitTime)
+	}()
+
+	time.Sleep(waitTime)
+	err = os.WriteFile(MycnfPath, []byte(data), 0666)
+	require.NoError(t, err, "failed creating my.cnf")
+	_, err = os.ReadFile(MycnfPath)
+	require.NoError(t, err, "failed reading")
+
+	// Wait for ReadMycnf to finish and then verify that the data read is correct.
+	wg.Wait()
 	// Tablet UID should be 11111, which determines tablet/data dir.
-	if got, want := mycnf.DataDir, "/vt_0000011111/"; !strings.Contains(got, want) {
-		t.Errorf("mycnf.DataDir = %v, want *%v*", got, want)
-	}
+	require.Contains(t, mycnf.DataDir, "/vt_0000011111/")
 	// MySQL server-id should be 22222, different from Tablet UID.
-	if got, want := mycnf.ServerID, uint32(22222); got != want {
-		t.Errorf("mycnf.ServerID = %v, want %v", got, want)
-	}
+	require.EqualValues(t, uint32(22222), mycnf.ServerID)
 }
 
 // Run this test if any changes are made to hook handling / make_mycnf hook
@@ -112,7 +128,7 @@ func NoTestMycnfHook(t *testing.T) {
 	}
 	mycnf := NewMycnf(uid, 0)
 	mycnf.Path = cnf.Path
-	mycnf, err = ReadMycnf(mycnf)
+	mycnf, err = ReadMycnf(mycnf, 0)
 	if err != nil {
 		t.Errorf("failed reading, err %v", err)
 	} else {