Support deploy TiFlash on multi-disks with "storage" configurations s…

…ince v4.0.9 (#931)

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/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/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()
 }
 

pkg/cluster/spec/spec_test.go

@@ -19,6 +19,7 @@ import (
 
 	"github.com/BurntSushi/toml"
 	. "github.com/pingcap/check"
+	"github.com/pingcap/tiup/pkg/cluster/template/scripts"
 	"gopkg.in/yaml.v2"
 )
 
@@ -557,3 +558,128 @@ pd_servers:
 	_, err = spec.LocationLabels()
 	c.Assert(err, NotNil)
 }
+
+func (s *metaSuiteTopo) TestTiFlashStorageSection(c *C) {
+	spec := &Specification{}
+	err := yaml.Unmarshal([]byte(`
+tiflash_servers:
+  - host: 172.16.5.138
+    data_dir: /hdd0/tiflash,/hdd1/tiflash
+    config:
+      storage.main.dir: [/ssd0/tiflash, /ssd1/tiflash]
+      storage.latest.dir: [/ssd0/tiflash]
+`), spec)
+	c.Assert(err, IsNil)
+
+	flashComp := FindComponent(spec, ComponentTiFlash)
+	instances := flashComp.Instances()
+	c.Assert(len(instances), Equals, 1)
+	// parse using clusterVersion<"v4.0.9"
+	{
+		ins := instances[0]
+		// This should be the same with tiflash_server instance's "data_dir"
+		dataDir := "/hdd0/tiflash,/hdd1/tiflash"
+		cfg := scripts.NewTiFlashScript(ins.GetHost(), "", dataDir, "", "", "")
+		conf, err := ins.(*TiFlashInstance).initTiFlashConfig(cfg, "v4.0.8", spec.ServerConfigs.TiFlash)
+		c.Assert(err, IsNil)
+
+		path, ok := conf["path"]
+		c.Assert(ok, IsTrue)
+		c.Assert(path, Equals, dataDir)
+	}
+	// parse using clusterVersion>="v4.0.9"
+	checkWithVersion := func(ver string) {
+		ins := instances[0].(*TiFlashInstance)
+		dataDir := "/ssd0/tiflash"
+		cfg := scripts.NewTiFlashScript(ins.GetHost(), "", dataDir, "", "", "")
+		conf, err := ins.initTiFlashConfig(cfg, ver, spec.ServerConfigs.TiFlash)
+		c.Assert(err, IsNil)
+
+		_, ok := conf["path"]
+		c.Assert(ok, IsTrue)
+
+		// After merging instance configurations with "storgae", the "path" property should be removed.
+		conf, err = ins.mergeTiFlashInstanceConfig(ver, conf, ins.InstanceSpec.(TiFlashSpec).Config)
+		c.Assert(err, IsNil)
+		_, ok = conf["path"]
+		c.Assert(ok, IsFalse)
+
+		if storageSection, ok := conf["storage"]; ok {
+			if mainSection, ok := storageSection.(map[string]interface{})["main"]; ok {
+				if mainDirsSection, ok := mainSection.(map[string]interface{})["dir"]; ok {
+					var mainDirs []interface{} = mainDirsSection.([]interface{})
+					c.Assert(len(mainDirs), Equals, 2)
+					c.Assert(mainDirs[0].(string), Equals, "/ssd0/tiflash")
+					c.Assert(mainDirs[1].(string), Equals, "/ssd1/tiflash")
+				} else {
+					c.Error("Can not get storage.main.dir section")
+				}
+			} else {
+				c.Error("Can not get storage.main section")
+			}
+			if latestSection, ok := storageSection.(map[string]interface{})["latest"]; ok {
+				if latestDirsSection, ok := latestSection.(map[string]interface{})["dir"]; ok {
+					var latestDirs []interface{} = latestDirsSection.([]interface{})
+					c.Assert(len(latestDirs), Equals, 1)
+					c.Assert(latestDirs[0].(string), Equals, "/ssd0/tiflash")
+				} else {
+					c.Error("Can not get storage.main.dir section")
+				}
+
+			} else {
+				c.Error("Can not get storage.main section")
+			}
+		} else {
+			c.Error("Can not get storage section")
+		}
+	}
+	checkWithVersion("v4.0.9")
+	checkWithVersion("nightly")
+}
+
+func (s *metaSuiteTopo) TestTiFlashInvalidStorageSection(c *C) {
+	spec := &Specification{}
+
+	testCases := [][]byte{
+		[]byte(`
+tiflash_servers:
+  - host: 172.16.5.138
+    data_dir: /hdd0/tiflash,/hdd1/tiflash
+    config:
+      # storage.main.dir is not defined
+      storage.latest.dir: ["/ssd0/tiflash"]
+`),
+		[]byte(`
+tiflash_servers:
+  - host: 172.16.5.138
+    data_dir: /hdd0/tiflash,/hdd1/tiflash
+    config:
+      # storage.main.dir is empty string array
+      storage.main.dir: []
+      storage.latest.dir: ["/ssd0/tiflash"]
+`),
+		[]byte(`
+tiflash_servers:
+  - host: 172.16.5.138
+    data_dir: /hdd0/tiflash,/hdd1/tiflash
+    config:
+      # storage.main.dir is not a string array
+      storage.main.dir: /hdd0/tiflash,/hdd1/tiflash
+      storage.latest.dir: ["/ssd0/tiflash"]
+`),
+		[]byte(`
+tiflash_servers:
+  - host: 172.16.5.138
+    data_dir: /hdd0/tiflash,/hdd1/tiflash
+    config:
+      # storage.main.dir is not a string array
+      storage.main.dir: [0, 1]
+      storage.latest.dir: ["/ssd0/tiflash"]
+`),
+	}
+
+	for _, testCase := range testCases {
+		err := yaml.Unmarshal([]byte(testCase), spec)
+		c.Check(err, NotNil)
+	}
+}