executor: fix TempDir permissions bug (pingcap#16823)

config/config.go

@@ -22,6 +22,7 @@ import (
 	"io/ioutil"
 	"net/url"
 	"os"
+	"os/user"
 	"path/filepath"
 	"strings"
 	"sync/atomic"
@@ -48,8 +49,12 @@ const (
 	DefMaxOfMaxIndexLength = 3072 * 4
 	// DefPort is the default port of TiDB
 	DefPort = 4000
-	// DefStatusPort is the default status port of TiBD
+	// DefStatusPort is the default status port of TiDB
 	DefStatusPort = 10080
+	// DefHost is the default host of TiDB
+	DefHost = "0.0.0.0"
+	// DefStatusHost is the default status host of TiDB
+	DefStatusHost = "0.0.0.0"
 	// DefStoreLivenessTimeout is the default value for store liveness timeout.
 	DefStoreLivenessTimeout = "120s"
 )
@@ -65,7 +70,7 @@ var (
 	// checkBeforeDropLDFlag is a go build flag.
 	checkBeforeDropLDFlag = "None"
 	// tempStorageDirName is the default temporary storage dir name by base64 encoding a string `port/statusPort`
-	tempStorageDirName = encodeDefTempStorageDir(DefPort, DefStatusPort)
+	tempStorageDirName = encodeDefTempStorageDir(DefHost, DefStatusHost, DefPort, DefStatusPort)
 )
 
 // Config contains configuration options.
@@ -139,13 +144,20 @@ type Config struct {
 // and the `tmp-storage-path` was not specified in the conf.toml or was specified the same as the default value.
 func (c *Config) UpdateTempStoragePath() {
 	if c.TempStoragePath == tempStorageDirName {
-		c.TempStoragePath = encodeDefTempStorageDir(c.Port, c.Status.StatusPort)
+		c.TempStoragePath = encodeDefTempStorageDir(c.Host, c.Status.StatusHost, c.Port, c.Status.StatusPort)
 	}
 }
 
-func encodeDefTempStorageDir(port, statusPort uint) string {
-	dirName := base64.URLEncoding.EncodeToString([]byte(fmt.Sprintf("%v/%v", port, statusPort)))
-	return filepath.Join(os.TempDir(), "tidb", dirName, "tmp-storage")
+func encodeDefTempStorageDir(host, statusHost string, port, statusPort uint) string {
+	dirName := base64.URLEncoding.EncodeToString([]byte(fmt.Sprintf("%v:%v/%v:%v", host, port, statusHost, statusPort)))
+	var osUID string
+	currentUser, err := user.Current()
+	if err != nil {
+		osUID = ""
+	} else {
+		osUID = currentUser.Uid
+	}
+	return filepath.Join(os.TempDir(), osUID+"_tidb", dirName, "tmp-storage")
 }
 
 // nullableBool defaults unset bool options to unset instead of false, which enables us to know if the user has set 2
@@ -531,7 +543,7 @@ type Experimental struct {
 }
 
 var defaultConf = Config{
-	Host:                         "0.0.0.0",
+	Host:                         DefHost,
 	AdvertiseAddress:             "",
 	Port:                         DefPort,
 	Cors:                         "",
@@ -581,7 +593,7 @@ var defaultConf = Config{
 	},
 	Status: Status{
 		ReportStatus:    true,
-		StatusHost:      "0.0.0.0",
+		StatusHost:      DefStatusHost,
 		StatusPort:      DefStatusPort,
 		MetricsInterval: 15,
 		RecordQPSbyDB:   false,

config/config.toml.example

@@ -38,9 +38,9 @@ mem-quota-query = 1073741824
 oom-use-tmp-storage = true
 
 # Specifies the temporary storage path for some operators when a single SQL statement exceeds the memory quota specified by mem-quota-query.
-# It defaults to a generated directory in `<TMPDIR>/tidb/` if it is unset.
+# It defaults to a generated directory in `<TMPDIR>/<os/user.Current().Uid>_tidb/` if it is unset.
 # It only takes effect when `oom-use-tmp-storage` is `true`.
-# tmp-storage-path = "/tmp/tidb/NDAwMC8xMDA4MA==/tmp-storage"
+# tmp-storage-path = "/tmp/<os/user.Current().Uid>_tidb/MC4wLjAuMDo0MDAwLzAuMC4wLjA6MTAwODA=/tmp-storage"
 
 # Specifies the maximum use of temporary storage (bytes) for all active queries when `oom-use-tmp-storage` is enabled.
 # If the `tmp-storage-quota` exceeds the capacity of the temporary storage directory, tidb-server would return an error and exit.

config/config_test.go

@@ -16,6 +16,7 @@ package config
 import (
 	"encoding/json"
 	"os"
+	"os/user"
 	"path/filepath"
 	"runtime"
 	"testing"
@@ -439,3 +440,32 @@ func (s *testConfigSuite) TestParsePath(c *C) {
 	c.Assert(err, IsNil)
 	c.Assert(disableGC, IsTrue)
 }
+
+func (s *testConfigSuite) TestEncodeDefTempStorageDir(c *C) {
+
+	tests := []struct {
+		host       string
+		statusHost string
+		port       uint
+		statusPort uint
+		expect     string
+	}{
+		{"0.0.0.0", "0.0.0.0", 4000, 10080, "MC4wLjAuMDo0MDAwLzAuMC4wLjA6MTAwODA="},
+		{"127.0.0.1", "127.16.5.1", 4000, 10080, "MTI3LjAuMC4xOjQwMDAvMTI3LjE2LjUuMToxMDA4MA=="},
+		{"127.0.0.1", "127.16.5.1", 4000, 15532, "MTI3LjAuMC4xOjQwMDAvMTI3LjE2LjUuMToxNTUzMg=="},
+	}
+
+	var osUID string
+	currentUser, err := user.Current()
+	if err != nil {
+		osUID = ""
+	} else {
+		osUID = currentUser.Uid
+	}
+
+	dirPrefix := filepath.Join(os.TempDir(), osUID+"_tidb")
+	for _, test := range tests {
+		tempStorageDir := encodeDefTempStorageDir(test.host, test.statusHost, test.port, test.statusPort)
+		c.Assert(tempStorageDir, Equals, filepath.Join(dirPrefix, test.expect, "tmp-storage"))
+	}
+}

tidb-server/main.go

@@ -210,10 +210,10 @@ func initializeTempDir() {
 	if err != nil {
 		switch err {
 		case fslock.ErrLockHeld:
-			fmt.Fprintf(os.Stderr, "The current temporary storage dir(%s) has been occupied by another instance, "+
-				"check tmp-storage-path config and make sure they are different.", tempDir)
+			log.Error("The current temporary storage dir has been occupied by another instance, "+
+				"check tmp-storage-path config and make sure they are different.", zap.String("TempStoragePath", tempDir), zap.Error(err))
 		default:
-			fmt.Fprintf(os.Stderr, "Failed to acquire exclusive lock on the temporary storage dir(%s). Error detail: {%s}", tempDir, err)
+			log.Error("Failed to acquire exclusive lock on the temporary storage dir.", zap.String("TempStoragePath", tempDir), zap.Error(err))
 		}
 		os.Exit(1)
 	}

util/chunk/chunk_test.go

@@ -16,6 +16,7 @@ package chunk
 import (
 	"bytes"
 	"fmt"
+	"io/ioutil"
 	"math"
 	"os"
 	"strconv"
@@ -37,7 +38,7 @@ import (
 func TestT(t *testing.T) {
 	cfg := config.GetGlobalConfig()
 	conf := *cfg
-	conf.TempStoragePath = "/tmp/tidb/test-temp-storage"
+	conf.TempStoragePath, _ = ioutil.TempDir("", "oom-use-tmp-storage")
 	config.StoreGlobalConfig(&conf)
 	_ = os.RemoveAll(conf.TempStoragePath) // clean the uncleared temp file during the last run.
 	_ = os.MkdirAll(conf.TempStoragePath, 0755)

util/chunk/disk_test.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 	"math/rand"
 	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -72,7 +73,7 @@ func (s *testChunkSuite) TestListInDisk(c *check.C) {
 		err := l.Add(chk)
 		c.Check(err, check.IsNil)
 	}
-	c.Assert(strings.HasPrefix(l.disk.Name(), "/tmp/tidb/test-temp-storage"), check.Equals, true)
+	c.Assert(strings.HasPrefix(l.disk.Name(), filepath.Join(os.TempDir(), "oom-use-tmp-storage")), check.Equals, true)
 	c.Check(l.NumChunks(), check.Equals, numChk)
 	c.Check(l.GetDiskTracker().BytesConsumed() > 0, check.IsTrue)