From d42e5826372f5565a2172aa0bcaf582d99a15f1d Mon Sep 17 00:00:00 2001 From: Manan Gupta Date: Tue, 28 Nov 2023 15:31:45 +0530 Subject: [PATCH] feat: add wait for reading mycnf Signed-off-by: Manan Gupta --- go/flags/endtoend/vtbackup.txt | 1 + go/flags/endtoend/vtcombo.txt | 1 + go/flags/endtoend/vttablet.txt | 1 + go/vt/mysqlctl/cmd.go | 2 +- go/vt/mysqlctl/mycnf.go | 20 +++++++++- go/vt/mysqlctl/mycnf_flag.go | 8 +++- go/vt/mysqlctl/mycnf_test.go | 68 +++++++++++++++++++++------------- 7 files changed, 72 insertions(+), 29 deletions(-) diff --git a/go/flags/endtoend/vtbackup.txt b/go/flags/endtoend/vtbackup.txt index 8610606eca2..e43b324673b 100644 --- a/go/flags/endtoend/vtbackup.txt +++ b/go/flags/endtoend/vtbackup.txt @@ -157,6 +157,7 @@ Flags: --min_retention_count int Always keep at least this many of the most recent backups in this backup storage location, even if some are older than the min_retention_time. This must be at least 1 since a backup must always exist to allow new backups to be made (default 1) --min_retention_time duration Keep each old backup for at least this long before removing it. Set to 0 to disable pruning of old backups. --mycnf-file string path to my.cnf, if reading all config params from there + --mycnf-wait-time duration time to wait for my.cnf to be created by mysqlctld externally (default 10s) --mycnf_bin_log_path string mysql binlog path --mycnf_data_dir string data directory for mysql --mycnf_error_log_path string mysql error log path diff --git a/go/flags/endtoend/vtcombo.txt b/go/flags/endtoend/vtcombo.txt index 40720e702b7..b0c77b7b168 100644 --- a/go/flags/endtoend/vtcombo.txt +++ b/go/flags/endtoend/vtcombo.txt @@ -203,6 +203,7 @@ Flags: --message_stream_grace_period duration the amount of time to give for a vttablet to resume if it ends a message stream, usually because of a reparent. (default 30s) --migration_check_interval duration Interval between migration checks (default 1m0s) --mycnf-file string path to my.cnf, if reading all config params from there + --mycnf-wait-time duration time to wait for my.cnf to be created by mysqlctld externally (default 10s) --mycnf_bin_log_path string mysql binlog path --mycnf_data_dir string data directory for mysql --mycnf_error_log_path string mysql error log path diff --git a/go/flags/endtoend/vttablet.txt b/go/flags/endtoend/vttablet.txt index 7dc29a140b1..413659edc44 100644 --- a/go/flags/endtoend/vttablet.txt +++ b/go/flags/endtoend/vttablet.txt @@ -225,6 +225,7 @@ Flags: --max_concurrent_online_ddl int Maximum number of online DDL changes that may run concurrently (default 256) --migration_check_interval duration Interval between migration checks (default 1m0s) --mycnf-file string path to my.cnf, if reading all config params from there + --mycnf-wait-time duration time to wait for my.cnf to be created by mysqlctld externally (default 10s) --mycnf_bin_log_path string mysql binlog path --mycnf_data_dir string data directory for mysql --mycnf_error_log_path string mysql error log path diff --git a/go/vt/mysqlctl/cmd.go b/go/vt/mysqlctl/cmd.go index 5c3bda11437..222a39e26ee 100644 --- a/go/vt/mysqlctl/cmd.go +++ b/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) } diff --git a/go/vt/mysqlctl/mycnf.go b/go/vt/mysqlctl/mycnf.go index 3af6b8e8607..056c81bf663 100644 --- a/go/vt/mysqlctl/mycnf.go +++ b/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,21 @@ 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) + ticker := time.NewTicker(myCnfWaitRetryTime) + defer ticker.Stop() + for err != nil { + select { + case <-timer.C: + return nil, err + case <-ticker.C: + f, err = os.Open(mycnf.Path) + } + } + } if err != nil { return nil, err } diff --git a/go/vt/mysqlctl/mycnf_flag.go b/go/vt/mysqlctl/mycnf_flag.go index 33a18d69940..f03765b3053 100644 --- a/go/vt/mysqlctl/mycnf_flag.go +++ b/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" @@ -49,6 +51,9 @@ var ( // the file to use to specify them all flagMycnfFile string + + // flagWaitForMyCnf is the amount of time to wait for the My.cnf file to be created externally. + flagWaitForMyCnf time.Duration ) // RegisterFlags registers the command line flags for @@ -75,6 +80,7 @@ func RegisterFlags() { fs.StringVar(&flagSecureFilePriv, "mycnf_secure_file_priv", flagSecureFilePriv, "mysql path for loading secure files") fs.StringVar(&flagMycnfFile, "mycnf-file", flagMycnfFile, "path to my.cnf, if reading all config params from there") + fs.DurationVar(&flagWaitForMyCnf, "mycnf-wait-time", 10*time.Second, "time to wait for my.cnf to be created by mysqlctld externally") }) } @@ -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, flagWaitForMyCnf) } diff --git a/go/vt/mysqlctl/mycnf_test.go b/go/vt/mysqlctl/mycnf_test.go index d422ed899c4..fc54f063618 100644 --- a/go/vt/mysqlctl/mycnf_test.go +++ b/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 {