Merge branch 'master' into feature/remove-public-key-after-destroyed

Makefile

@@ -113,7 +113,7 @@ fmt:
 	@echo "gofmt (simplify)"
 	@gofmt -s -l -w $(FILES) 2>&1
 	@echo "goimports (if installed)"
-	$(shell gimports -w $(FILES) 2>/dev/null)
+	$(shell goimports -w $(FILES) 2>/dev/null)
 
 tools/bin/errcheck: tools/check/go.mod
 	cd tools/check; \

cmd/mirror.go

@@ -352,7 +352,7 @@ func newMirrorPublishCmd() *cobra.Command {
 				publishInfo.Stand = &standalone
 				publishInfo.Hide = &hidden
 			} else if flagSet.Exist("standalone") || flagSet.Exist("hide") {
-				fmt.Println("This is not a new component, --standalone and --hide flag will be omited")
+				fmt.Println("This is not a new component, --standalone and --hide flag will be omitted")
 			}
 
 			m = repository.UpdateManifestForPublish(m, component, version, entry, goos, goarch, desc, v1manifest.FileHash{

components/bench/ycsb.go

@@ -16,15 +16,16 @@ package main
 import (
 	"context"
 	"fmt"
+	"runtime"
+	"strings"
+
 	"github.com/magiconair/properties"
 	_ "github.com/pingcap/go-ycsb/db/tikv"
 	"github.com/pingcap/go-ycsb/pkg/client"
 	"github.com/pingcap/go-ycsb/pkg/measurement"
 	_ "github.com/pingcap/go-ycsb/pkg/workload"
 	"github.com/pingcap/go-ycsb/pkg/ycsb"
 	"github.com/spf13/cobra"
-	"runtime"
-	"strings"
 )
 
 type ycsbConfig struct {

components/cluster/command/deploy.go

@@ -28,7 +28,6 @@ import (
 	"github.com/pingcap/tiup/pkg/errutil"
 	telemetry2 "github.com/pingcap/tiup/pkg/telemetry"
 	"github.com/pingcap/tiup/pkg/utils"
-	tiuputils "github.com/pingcap/tiup/pkg/utils"
 	"github.com/spf13/cobra"
 )
 
@@ -47,7 +46,7 @@ var (
 
 func newDeploy() *cobra.Command {
 	opt := cluster.DeployOptions{
-		IdentityFile: path.Join(tiuputils.UserHome(), ".ssh", "id_rsa"),
+		IdentityFile: path.Join(utils.UserHome(), ".ssh", "id_rsa"),
 	}
 	cmd := &cobra.Command{
 		Use:          "deploy <cluster-name> <version> <topology.yaml>",
@@ -92,7 +91,7 @@ func newDeploy() *cobra.Command {
 		},
 	}
 
-	cmd.Flags().StringVarP(&opt.User, "user", "u", tiuputils.CurrentUser(), "The user name to login via SSH. The user must has root (or sudo) privilege.")
+	cmd.Flags().StringVarP(&opt.User, "user", "u", utils.CurrentUser(), "The user name to login via SSH. The user must has root (or sudo) privilege.")
 	cmd.Flags().BoolVarP(&opt.SkipCreateUser, "skip-create-user", "", false, "Skip creating the user specified in topology (experimental).")
 	cmd.Flags().StringVarP(&opt.IdentityFile, "identity_file", "i", opt.IdentityFile, "The path of the SSH identity file. If specified, public key authentication will be used.")
 	cmd.Flags().BoolVarP(&opt.UsePassword, "password", "p", false, "Use password of target hosts. If specified, password authentication will be used.")

components/playground/instance/process.go

@@ -24,7 +24,7 @@ type Process interface {
 	Cmd() *exec.Cmd
 }
 
-// process implementes Process
+// process implements Process
 type process struct {
 	cmd       *exec.Cmd
 	startTime time.Time

examples/topology.example.yaml

@@ -418,8 +418,10 @@ server_configs:
     #      - key: "dc"
     #        value: "sha"
   tiflash:
-    # path_realtime_mode: false
     logger.level: "info"
+    ## Deprecated multi-disks storage path setting style since v4.0.9,
+    ## check storage.* configurations in host level for new style.
+    # path_realtime_mode: false
   tiflash-learner:
     log-level: "info"
     # raftstore.apply-pool-size: 4
@@ -488,37 +490,40 @@ tiflash_servers:
     # flash_proxy_status_port: 20292
     # metrics_port: 8234
     # deploy_dir: /tidb-deploy/tiflash-9000
+    ## With cluster version >= v4.0.9 and you want to deploy a multi-disk TiFlash node, it is recommended to
+    ## check config.storage.* for details. The data_dir will be ignored if you defined those configurations.
+    ## Setting data_dir to a ','-joined string is still supported but deprecated.
+    ## Check https://docs.pingcap.com/tidb/stable/tiflash-configuration#multi-disk-deployment for more details.
     # data_dir: /tidb-data/tiflash-9000
     # log_dir: /tidb-deploy/tiflash-9000/log
     # numa_node: "0,1"
-    # #
-    # # `path_realtime_mode` only works if `data_dir` is specified with multiple paths.
-    # #
-    # #  path_realtime_mode:
-    # #    "true" means only other paths instead of first path can be used to store older data.
-    # #    "false" means all paths can be used to store older data.
-    # #
-    # # TiFlash only uses the first path to store the latest data (i.e. "delta"). And others for the older data (i.e. "stable". which is the majority of data),
-    # #
-    # # E.g, if you intend to use an fast and smaller NVMe SSD (256GB) to speed up data ingestion speed in TiFlash,
-    # # and 4 extra normal SSDs (512GB each) for main storage. Then your configurations should be look like:
-    # #
-    # # data_dir: /nvme_ssd_256/data,/ssd1_512/data,/ssd2_512/data,/ssd3_512/data,/ssd4_512/data
-    # # config:
-    # #   path_realtime_mode: true
-    # #
-    # #
-    # # And if your first disk is big enough, to fully use the capacity of it, use configurations look like:
-    # #
-    # # data_dir: /nvme_ssd_512/data,/ssd1_512/data,/ssd2_512/data,/ssd3_512/data,/ssd4_512/data
-    # # config:
-    # #   path_realtime_mode: false
-    # #
-    # #
     # # The following configs are used to overwrite the `server_configs.tiflash` values.
     # config:
-    #   path_realtime_mode: false
     #   logger.level: "info"
+    #   ## Deprecated multi-disks storage path setting. Deprecated since v4.0.9,
+    #   ## check following storage.* configurations for new style.
+    #   # path_realtime_mode: false
+    #   #
+    #   ## Multi-disks storage paths settings. These will overwrite the `data_dir`.
+    #   ## If there are multiple SSD disks on the machine,
+    #   ## specify the path list on `storage.main.dir` can improve TiFlash performance.
+    #   # storage.main.dir: [ "/nvme_ssd0_512/tiflash", "/nvme_ssd1_512/tiflash" ]
+    #   ## Store capacity of each path, i.e. max data size allowed.
+    #   ## If it is not set, or is set to 0s, the actual disk capacity is used.
+    #   # storage.main.capacity = [ 536870912000, 536870912000 ]
+    #   #
+    #   ## If there are multiple disks with different IO metrics (e.g. one SSD and some HDDs)
+    #   ## on the machine, 
+    #   ## set `storage.latest.dir` to store the latest data on SSD (disks with higher IOPS metrics)
+    #   ## set `storage.main.dir` to store the main data on HDD (disks with lower IOPS metrics)
+    #   ## can improve TiFlash performance.
+    #   # storage.main.dir: [ "/hdd0_512/tiflash", "/hdd1_512/tiflash", "/hdd2_512/tiflash" ]
+    #   # storage.latest.dir: [ "/nvme_ssd0_100/tiflash" ]
+    #   ## Store capacity of each path, i.e. max data size allowed.
+    #   ## If it is not set, or is set to 0s, the actual disk capacity is used.
+    #   # storage.main.capacity = [ 536870912000, 536870912000 ]
+    #   # storage.latest.capacity = [ 107374182400 ]
+    #
     # # The following configs are used to overwrite the `server_configs.tiflash-learner` values.
     # learner_config:
     #   log-level: "info"

pkg/cluster/module/shell.go

@@ -22,7 +22,7 @@ import (
 // ShellModuleConfig is the configurations used to initialize a TiOpsModuleSystemd
 type ShellModuleConfig struct {
 	Command  string // the command to run
-	Sudo     bool   // whether use root priviledge to run the command
+	Sudo     bool   // whether use root privilege to run the command
 	Chdir    string // change working directory before running the command
 	UseShell bool   // whether use shell to invoke the command
 }

pkg/cluster/module/systemd.go

@@ -57,7 +57,7 @@ func NewSystemdModule(config SystemdModuleConfig) *SystemdModule {
 
 	switch config.Scope {
 	case SystemdScopeUser:
-		sudo = false // `--user` scope does not need root priviledge
+		sudo = false // `--user` scope does not need root privilege
 		fallthrough
 	case SystemdScopeGlobal:
 		systemctl = fmt.Sprintf("%s --%s", systemctl, config.Scope)

pkg/cluster/operation/operation.go

@@ -28,7 +28,7 @@ type Options struct {
 	Force             bool             // Option for upgrade subcommand
 	SSHTimeout        uint64           // timeout in seconds when connecting an SSH server
 	OptTimeout        uint64           // timeout in seconds for operations that support it, not to confuse with SSH timeout
-	APITimeout        uint64           // timeout in seconds for API operations that support it, like transfering store leader
+	APITimeout        uint64           // timeout in seconds for API operations that support it, like transferring store leader
 	IgnoreConfigCheck bool             // should we ignore the config check result after init config
 	NativeSSH         bool             // should use native ssh client or builtin easy ssh (deprecated, shoule use SSHType)
 	SSHType           executor.SSHType // the ssh type: 'builtin', 'system', 'none'

pkg/cluster/spec/parse_topology_test.go

@@ -268,6 +268,150 @@ tikv_servers:
 	})
 }
 
+func (s *topoSuite) TestTiFlashStorage(c *check.C) {
+	// test tiflash storage section, 'storage.main.dir' should not be defined in server_configs
+	withTempFile(`
+server_configs:
+  tiflash:
+    storage.main.dir: [/data1/tiflash]
+tiflash_servers:
+  - host: 172.16.5.140
+    data_dir: /ssd0/tiflash,/ssd1/tiflash,/ssd2/tiflash
+    config:
+      storage.main.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash]
+      storage.latest.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash]
+`, func(file string) {
+		topo := Specification{}
+		err := ParseTopologyYaml(file, &topo)
+		c.Assert(err, check.NotNil)
+	})
+
+	// test tiflash storage section, 'storage.latest.dir' should not be defined in server_configs
+	withTempFile(`
+server_configs:
+  tiflash:
+    storage.latest.dir: [/data1/tiflash]
+tiflash_servers:
+  - host: 172.16.5.140
+    data_dir: /ssd0/tiflash,/ssd1/tiflash,/ssd2/tiflash
+    config:
+      storage.main.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash]
+      storage.latest.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash]
+`, func(file string) {
+		topo := Specification{}
+		err := ParseTopologyYaml(file, &topo)
+		c.Assert(err, check.NotNil)
+	})
+
+	// test tiflash storage section defined data dir
+	// test for depreacated setting, for backward compatibility
+	withTempFile(`
+tiflash_servers:
+  - host: 172.16.5.140
+    data_dir: /ssd0/tiflash
+    config:
+`, func(file string) {
+		topo := Specification{}
+		err := ParseTopologyYaml(file, &topo)
+		c.Assert(err, check.IsNil)
+		ExpandRelativeDir(&topo)
+
+		c.Assert(topo.TiFlashServers[0].DeployDir, check.Equals, "/home/tidb/deploy/tiflash-9000")
+		c.Assert(topo.TiFlashServers[0].DataDir, check.Equals, "/ssd0/tiflash")
+		c.Assert(topo.TiFlashServers[0].LogDir, check.Equals, "")
+	})
+
+	// test tiflash storage section defined data dir
+	withTempFile(`
+tiflash_servers:
+  - host: 172.16.5.140
+    data_dir: /ssd0/tiflash,/ssd1/tiflash,/ssd2/tiflash
+    config:
+      storage.main.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash]
+      storage.latest.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash]
+`, func(file string) {
+		topo := Specification{}
+		err := ParseTopologyYaml(file, &topo)
+		c.Assert(err, check.IsNil)
+		ExpandRelativeDir(&topo)
+
+		c.Assert(topo.TiFlashServers[0].DeployDir, check.Equals, "/home/tidb/deploy/tiflash-9000")
+		c.Assert(topo.TiFlashServers[0].DataDir, check.Equals, "/ssd0/tiflash,/ssd1/tiflash,/ssd2/tiflash")
+		c.Assert(topo.TiFlashServers[0].LogDir, check.Equals, "")
+	})
+
+	// test tiflash storage section defined data dir, "data_dir" will be ignored
+	withTempFile(`
+tiflash_servers:
+  - host: 172.16.5.140
+    # if storage.main.dir is defined, data_dir will be ignored
+    data_dir: /hdd0/tiflash 
+    config:
+      storage.main.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash]
+`, func(file string) {
+		topo := Specification{}
+		err := ParseTopologyYaml(file, &topo)
+		c.Assert(err, check.IsNil)
+		ExpandRelativeDir(&topo)
+
+		c.Assert(topo.TiFlashServers[0].DeployDir, check.Equals, "/home/tidb/deploy/tiflash-9000")
+		c.Assert(topo.TiFlashServers[0].DataDir, check.Equals, "/ssd0/tiflash,/ssd1/tiflash,/ssd2/tiflash")
+		c.Assert(topo.TiFlashServers[0].LogDir, check.Equals, "")
+	})
+
+	// test tiflash storage section defined data dir
+	// if storage.latest.dir is not empty, the first path in
+	// storage.latest.dir will be the first path in 'DataDir'
+	// DataDir is the union set of storage.latest.dir and storage.main.dir
+	withTempFile(`
+tiflash_servers:
+  - host: 172.16.5.140
+    data_dir: /ssd0/tiflash
+    config:
+      storage.main.dir: [/hdd0/tiflash, /hdd1/tiflash, /hdd2/tiflash]
+      storage.latest.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash, /hdd0/tiflash]
+`, func(file string) {
+		topo := Specification{}
+		err := ParseTopologyYaml(file, &topo)
+		c.Assert(err, check.IsNil)
+		ExpandRelativeDir(&topo)
+
+		c.Assert(topo.TiFlashServers[0].DeployDir, check.Equals, "/home/tidb/deploy/tiflash-9000")
+		c.Assert(topo.TiFlashServers[0].DataDir, check.Equals, "/ssd0/tiflash,/hdd0/tiflash,/hdd1/tiflash,/hdd2/tiflash,/ssd1/tiflash,/ssd2/tiflash")
+		c.Assert(topo.TiFlashServers[0].LogDir, check.Equals, "")
+	})
+
+	// test tiflash storage section defined data dir
+	// should always define storage.main.dir if any 'storage.*' is defined
+	withTempFile(`
+tiflash_servers:
+  - host: 172.16.5.140
+    data_dir: /ssd0/tiflash
+    config:
+      #storage.main.dir: [/hdd0/tiflash, /hdd1/tiflash, /hdd2/tiflash]
+      storage.latest.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash, /hdd0/tiflash]
+`, func(file string) {
+		topo := Specification{}
+		err := ParseTopologyYaml(file, &topo)
+		c.Assert(err, check.NotNil)
+	})
+
+	// test tiflash storage section defined data dir
+	// storage.main.dir should always use absolute path
+	withTempFile(`
+tiflash_servers:
+  - host: 172.16.5.140
+    data_dir: /ssd0/tiflash
+    config:
+      storage.main.dir: [tiflash/data, ]
+      storage.latest.dir: [/ssd0/tiflash, /ssd1/tiflash, /ssd2/tiflash, /hdd0/tiflash]
+`, func(file string) {
+		topo := Specification{}
+		err := ParseTopologyYaml(file, &topo)
+		c.Assert(err, check.NotNil)
+	})
+}
+
 func merge4test(base, scale string) (*Specification, error) {
 	baseTopo := Specification{}
 	if err := ParseTopologyYaml(base, &baseTopo); err != nil {

pkg/cluster/spec/server_config.go

@@ -40,7 +40,7 @@ const (
 	migrateLockName = "tiup-migrate.lck"
 )
 
-// ErrorCheckConfig represent error occured in config check stage
+// ErrorCheckConfig represent error occurred in config check stage
 var ErrorCheckConfig = errors.New("check config failed")
 
 // strKeyMap tries to convert `map[interface{}]interface{}` to `map[string]interface{}`

pkg/cluster/spec/spec.go

@@ -25,6 +25,7 @@ import (
 	"github.com/pingcap/errors"
 	"github.com/pingcap/tiup/pkg/cluster/executor"
 	"github.com/pingcap/tiup/pkg/cluster/template/scripts"
+	"github.com/pingcap/tiup/pkg/logger/log"
 	"github.com/pingcap/tiup/pkg/meta"
 	"go.etcd.io/etcd/clientv3"
 )
@@ -309,6 +310,19 @@ func (s *Specification) UnmarshalYAML(unmarshal func(interface{}) error) error {
 		return err
 	}
 
+	// Rewrite TiFlashSpec.DataDir since we may override it with configurations.
+	// Should do it before validatation because we need to detect dir conflicts.
+	for i := 0; i < len(s.TiFlashServers); i++ {
+		dataDir, err := s.TiFlashServers[i].GetOverrideDataDir()
+		if err != nil {
+			return err
+		}
+		if s.TiFlashServers[i].DataDir != dataDir {
+			log.Infof("'tiflash_server:%s.data_dir' is overwritten by its storage configuration. Now the data_dir is %s", s.TiFlashServers[i].Host, dataDir)
+			s.TiFlashServers[i].DataDir = dataDir
+		}
+	}
+
 	return s.Validate()
 }