From 7f60b3a836090852a4f616b13407614ea2f30ff2 Mon Sep 17 00:00:00 2001 From: "vitess-bot[bot]" <108069721+vitess-bot[bot]@users.noreply.github.com> Date: Thu, 1 Feb 2024 11:48:39 -0600 Subject: [PATCH] [release-18.0] Improve efficiency and accuracy of mysqld.GetVersionString (#15096) (#15111) Signed-off-by: Matt Lord Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> --- go/vt/env/env.go | 35 ++++++++++------- go/vt/env/env_test.go | 82 ++++++++++++++++++++++++++++++++++++++++ go/vt/mysqlctl/mysqld.go | 39 +++++++++++++------ 3 files changed, 130 insertions(+), 26 deletions(-) diff --git a/go/vt/env/env.go b/go/vt/env/env.go index 70feb43186c..186f81cd585 100644 --- a/go/vt/env/env.go +++ b/go/vt/env/env.go @@ -18,7 +18,6 @@ package env import ( "errors" - "fmt" "os" "os/exec" "path" @@ -30,9 +29,12 @@ const ( // DefaultVtDataRoot is the default value for VTROOT environment variable DefaultVtDataRoot = "/vt" // DefaultVtRoot is only required for hooks - DefaultVtRoot = "/usr/local/vitess" + DefaultVtRoot = "/usr/local/vitess" + mysqldSbinPath = "/usr/sbin/mysqld" ) +var errMysqldNotFound = errors.New("VT_MYSQL_ROOT is not set and no mysqld could be found in your PATH") + // VtRoot returns $VTROOT or tries to guess its value if it's not set. // This is the root for the 'vt' distribution, which contains bin/vttablet // for instance. @@ -64,25 +66,30 @@ func VtDataRoot() string { } // VtMysqlRoot returns the root for the mysql distribution, -// which contains bin/mysql CLI for instance. -// If it is not set, look for mysqld in the path. +// which contains the bin/mysql CLI for instance. +// If $VT_MYSQL_ROOT is not set, look for mysqld in the $PATH. func VtMysqlRoot() (string, error) { - // if the environment variable is set, use that + // If the environment variable is set, use that. if root := os.Getenv("VT_MYSQL_ROOT"); root != "" { return root, nil } - // otherwise let's look for mysqld in the PATH. - // ensure that /usr/sbin is included, as it might not be by default - // This is the default location for mysqld from packages. - newPath := fmt.Sprintf("/usr/sbin:%s", os.Getenv("PATH")) - os.Setenv("PATH", newPath) - path, err := exec.LookPath("mysqld") + getRoot := func(path string) string { + return filepath.Dir(filepath.Dir(path)) // Strip mysqld and [s]bin parts + } + binpath, err := exec.LookPath("mysqld") if err != nil { - return "", errors.New("VT_MYSQL_ROOT is not set and no mysqld could be found in your PATH") + // First see if /usr/sbin/mysqld exists as it might not be in + // the PATH by default and this is often the default location + // used by mysqld OS system packages (apt, dnf, etc). + fi, err := os.Stat(mysqldSbinPath) + if err == nil /* file exists */ && fi.Mode().IsRegular() /* not a DIR or other special file */ && + fi.Mode()&0111 != 0 /* executable by anyone */ { + return getRoot(mysqldSbinPath), nil + } + return "", errMysqldNotFound } - path = filepath.Dir(filepath.Dir(path)) // strip mysqld, and the sbin - return path, nil + return getRoot(binpath), nil } // VtMysqlBaseDir returns the Mysql base directory, which diff --git a/go/vt/env/env_test.go b/go/vt/env/env_test.go index 4aa53a25bed..f91cdf94673 100644 --- a/go/vt/env/env_test.go +++ b/go/vt/env/env_test.go @@ -18,7 +18,10 @@ package env import ( "os" + "path/filepath" "testing" + + "github.com/stretchr/testify/require" ) func TestVtDataRoot(t *testing.T) { @@ -43,3 +46,82 @@ func TestVtDataRoot(t *testing.T) { t.Errorf("The value of VtDataRoot should be %v, not %v.", passed, root) } } + +func TestVtMysqlRoot(t *testing.T) { + envVar := "VT_MYSQL_ROOT" + originalMySQLRoot := os.Getenv(envVar) + defer os.Setenv(envVar, originalMySQLRoot) + originalPATH := os.Getenv("PATH") + defer os.Setenv("PATH", originalPATH) + + // The test directory is used to create our fake mysqld binary. + testDir := t.TempDir() // This is automatically cleaned up + createExecutable := func(path string) error { + fullPath := testDir + path + err := os.MkdirAll(filepath.Dir(fullPath), 0755) + require.NoError(t, err) + return os.WriteFile(fullPath, []byte("test"), 0755) + } + + type testcase struct { + name string + preFunc func() error + vtMysqlRootEnvVal string + pathEnvVal string + expect string // The return value we expect from VtMysqlRoot() + expectErr string + } + testcases := []testcase{ + { + name: "VT_MYSQL_ROOT set", + vtMysqlRootEnvVal: "/home/mysql/binaries", + }, + { + name: "VT_MYSQL_ROOT empty; PATH set without /usr/sbin", + pathEnvVal: testDir + filepath.Dir(mysqldSbinPath) + + ":/usr/bin:/sbin:/bin:/usr/local/bin:/usr/local/sbin:/home/mysql/binaries", + preFunc: func() error { + return createExecutable(mysqldSbinPath) + }, + expect: testDir + "/usr", + }, + } + + // If /usr/sbin/mysqld exists, confirm that we find it even + // when /usr/sbin is not in the PATH. + _, err := os.Stat(mysqldSbinPath) + if err == nil { + t.Logf("Found %s, confirming auto detection behavior", mysqldSbinPath) + testcases = append(testcases, testcase{ + name: "VT_MYSQL_ROOT empty; PATH empty; mysqld in /usr/sbin", + expect: "/usr", + }) + } else { + testcases = append(testcases, testcase{ // Error expected + name: "VT_MYSQL_ROOT empty; PATH empty; mysqld not in /usr/sbin", + expectErr: errMysqldNotFound.Error(), + }) + } + + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + if tc.preFunc != nil { + err := tc.preFunc() + require.NoError(t, err) + } + os.Setenv(envVar, tc.vtMysqlRootEnvVal) + os.Setenv("PATH", tc.pathEnvVal) + path, err := VtMysqlRoot() + if tc.expectErr != "" { + require.EqualError(t, err, tc.expectErr) + } else { + require.NoError(t, err) + } + if tc.vtMysqlRootEnvVal != "" { + // This should always be returned. + tc.expect = tc.vtMysqlRootEnvVal + } + require.Equal(t, tc.expect, path) + }) + } +} diff --git a/go/vt/mysqlctl/mysqld.go b/go/vt/mysqlctl/mysqld.go index b8597735b9b..d2882453ab8 100644 --- a/go/vt/mysqlctl/mysqld.go +++ b/go/vt/mysqlctl/mysqld.go @@ -42,27 +42,32 @@ import ( "github.com/spf13/pflag" - "vitess.io/vitess/go/mysql/sqlerror" - "vitess.io/vitess/go/protoutil" - "vitess.io/vitess/config" "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/mysql/sqlerror" + "vitess.io/vitess/go/protoutil" "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/vt/dbconfigs" "vitess.io/vitess/go/vt/dbconnpool" + vtenv "vitess.io/vitess/go/vt/env" "vitess.io/vitess/go/vt/hook" "vitess.io/vitess/go/vt/log" "vitess.io/vitess/go/vt/mysqlctl/mysqlctlclient" "vitess.io/vitess/go/vt/servenv" "vitess.io/vitess/go/vt/vterrors" - vtenv "vitess.io/vitess/go/vt/env" mysqlctlpb "vitess.io/vitess/go/vt/proto/mysqlctl" - "vitess.io/vitess/go/vt/proto/vtrpc" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) -var ( +// The string we expect before the MySQL version number +// in strings containing MySQL version information. +const versionStringPrefix = "Ver " +// How many bytes from MySQL error log to sample for error messages +const maxLogFileSampleSize = 4096 + +var ( // DisableActiveReparents is a flag to disable active // reparents for safety reasons. It is used in three places: // 1. in this file to skip registering the commands. @@ -86,15 +91,18 @@ var ( replicationConnectRetry = 10 * time.Second - versionRegex = regexp.MustCompile(`Ver ([0-9]+)\.([0-9]+)\.([0-9]+)`) + versionRegex = regexp.MustCompile(fmt.Sprintf(`%s([0-9]+)\.([0-9]+)\.([0-9]+)`, versionStringPrefix)) + // versionSQLQuery will return a version string directly from + // a MySQL server that is compatible with what we expect from + // mysqld --version and matches the versionRegex. Example + // result: Ver 8.0.35 MySQL Community Server - GPL + versionSQLQuery = fmt.Sprintf("select concat('%s', @@global.version, ' ', @@global.version_comment) as version", + versionStringPrefix) binlogEntryCommittedTimestampRegex = regexp.MustCompile("original_committed_timestamp=([0-9]+)") binlogEntryTimestampGTIDRegexp = regexp.MustCompile(`^#(.+) server id.*\bGTID\b`) ) -// How many bytes from MySQL error log to sample for error messages -const maxLogFileSampleSize = 4096 - // Mysqld is the object that represents a mysqld daemon running on this server. type Mysqld struct { dbcfgs *dbconfigs.DBConfigs @@ -1136,7 +1144,13 @@ func buildLdPaths() ([]string, error) { // GetVersionString is part of the MysqlExecutor interface. func (mysqld *Mysqld) GetVersionString(ctx context.Context) (string, error) { - // Execute as remote action on mysqlctld to ensure we get the actual running MySQL version. + // Try to query the mysqld instance directly. + qr, err := mysqld.FetchSuperQuery(ctx, versionSQLQuery) + if err == nil && len(qr.Rows) == 1 { + return qr.Rows[0][0].ToString(), nil + } + // Execute as remote action on mysqlctld to use the actual running MySQL + // version. if socketFile != "" { client, err := mysqlctlclient.New("unix", socketFile) if err != nil { @@ -1145,6 +1159,7 @@ func (mysqld *Mysqld) GetVersionString(ctx context.Context) (string, error) { defer client.Close() return client.VersionString(ctx) } + // Fall back to the sys exec method using mysqld --version. return GetVersionString() } @@ -1379,7 +1394,7 @@ func (mysqld *Mysqld) scanBinlogTimestamp( // ReadBinlogFilesTimestamps reads all given binlog files via `mysqlbinlog` command and returns the first and last found transaction timestamps func (mysqld *Mysqld) ReadBinlogFilesTimestamps(ctx context.Context, req *mysqlctlpb.ReadBinlogFilesTimestampsRequest) (*mysqlctlpb.ReadBinlogFilesTimestampsResponse, error) { if len(req.BinlogFileNames) == 0 { - return nil, vterrors.Errorf(vtrpc.Code_INVALID_ARGUMENT, "empty binlog list in ReadBinlogFilesTimestampsRequest") + return nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "empty binlog list in ReadBinlogFilesTimestampsRequest") } if socketFile != "" { log.Infof("executing Mysqld.ReadBinlogFilesTimestamps() remotely via mysqlctld server: %v", socketFile)