From 7cc079fa6a0a19a6dd4f99f7b3b72620cd73fbbf Mon Sep 17 00:00:00 2001 From: lucasliang Date: Mon, 2 Dec 2024 23:47:49 +0800 Subject: [PATCH 1/3] TiKV: support clearing extra data dirs automatically when destroying cluster. Signed-off-by: lucasliang --- pkg/cluster/spec/parse_topology_test.go | 47 ++++++++++++++++++++++++ pkg/cluster/spec/tikv.go | 49 +++++++++++++++++++++++-- 2 files changed, 92 insertions(+), 4 deletions(-) diff --git a/pkg/cluster/spec/parse_topology_test.go b/pkg/cluster/spec/parse_topology_test.go index 4b544da329..45b1020753 100644 --- a/pkg/cluster/spec/parse_topology_test.go +++ b/pkg/cluster/spec/parse_topology_test.go @@ -17,6 +17,7 @@ import ( "fmt" "os" "path/filepath" + "sort" "testing" "github.com/pingcap/check" @@ -277,6 +278,52 @@ tikv_servers: }) } +func (s *topoSuite) TestTiKVServerConfigs(c *check.C) { + // test tikv server config section + withTempFile(` +global: + user: tidb + deploy_dir: my-global-deploy + data_dir: my-global-data + log_dir: my-global-log + +server_configs: + tikv: + storage.reserve-space: 1K + storage.data-dir: /data1/tikv + raft-engine.dir: /data1/data/raft + raft-engine.spill-dir: /data1/data/spill + rocksdb.wal-dir: /data2/data/wal + rocksdb.titan.dirname: /data2/data/titan + +tikv_servers: + - host: 172.16.5.140 + port: 20161 + status_port: 20181 +`, func(file string) { + topo := Specification{} + err := ParseTopologyYaml(file, &topo) + c.Assert(err, check.IsNil) + ExpandRelativeDir(&topo) + + c.Assert(topo.TiKVServers[0].DeployDir, check.Equals, "/home/tidb/my-global-deploy/tikv-20161") + c.Assert(topo.TiKVServers[0].DataDir, check.Equals, "/home/tidb/my-global-deploy/tikv-20161/my-global-data") + c.Assert(topo.TiKVServers[0].LogDir, check.Equals, "/home/tidb/my-global-deploy/tikv-20161/my-global-log") + + c.Assert(topo.ServerConfigs.TiKV["storage.data-dir"], check.Equals, "/data1/tikv") + c.Assert(topo.ServerConfigs.TiKV["raft-engine.dir"], check.Equals, "/data1/data/raft") + c.Assert(topo.ServerConfigs.TiKV["raft-engine.spill-dir"], check.Equals, "/data1/data/spill") + c.Assert(topo.ServerConfigs.TiKV["rocksdb.wal-dir"], check.Equals, "/data2/data/wal") + c.Assert(topo.ServerConfigs.TiKV["rocksdb.titan.dirname"], check.Equals, "/data2/data/titan") + + extraDirs := topo.TiKVServers[0].getExtraDeployDirs(topo.ServerConfigs.TiKV) + sort.Strings(extraDirs) + targetDirs := []string{"/data1/tikv", "/data1/data/raft", "/data1/data/spill", "/data2/data/wal", "/data2/data/titan"} + sort.Strings(targetDirs) + c.Assert(extraDirs, check.DeepEquals, targetDirs) + }) +} + func (s *topoSuite) TestTiFlashStorage(c *check.C) { // test tiflash storage section, 'storage.main.dir' should not be defined in server_configs withTempFile(` diff --git a/pkg/cluster/spec/tikv.go b/pkg/cluster/spec/tikv.go index 4c40fb1408..c3653f05a6 100644 --- a/pkg/cluster/spec/tikv.go +++ b/pkg/cluster/spec/tikv.go @@ -164,6 +164,48 @@ func (s *TiKVSpec) Labels() (map[string]string, error) { return lbs, nil } +// getExtraDeployDirs returns the extra directories that needs to be checked. +func (s *TiKVSpec) getExtraDeployDirs(globalConf map[string]any) []string { + var extractDir = func(dir any, rootPath string) string { + realDir := strings.TrimSpace(dir.(string)) + // Skip the relative path under the root path. + if realDir == "" || (strings.HasPrefix(realDir, "./") || strings.HasPrefix(realDir, rootPath)) { + return "" + } + elemStr := strings.TrimSuffix(realDir, "/") + return elemStr + } + + extraDirs := []string{} + rootPath := s.DataDir + candidates := []string{ + "storage.data-dir", + "rocksdb.wal-dir", + "rocksdb.titan.dirname", + "raft-engine.dir", + "raft-engine.spill-dir", + } + for _, candidate := range candidates { + // Check the global configuration first. + globalConfDir, ok := globalConf[candidate] + if ok { + elemStr := extractDir(globalConfDir, rootPath) + if elemStr != "" { + extraDirs = append(extraDirs, elemStr) + } + } + // Check the instance configuration. + dir, ok := s.Config[candidate] + if ok { + elemStr := extractDir(dir, rootPath) + if elemStr != "" { + extraDirs = append(extraDirs, elemStr) + } + } + } + return extraDirs +} + // TiKVComponent represents TiKV component. type TiKVComponent struct{ Topology *Specification } @@ -205,6 +247,8 @@ func (c *TiKVComponent) Instances() []Instance { ins := make([]Instance, 0, len(c.Topology.TiKVServers)) for _, s := range c.Topology.TiKVServers { s := s + dirs := []string{s.DeployDir, s.DataDir} + dirs = append(dirs, s.getExtraDeployDirs(c.Topology.ServerConfigs.TiKV)...) ins = append(ins, &TiKVInstance{BaseInstance{ InstanceSpec: s, Name: c.Name(), @@ -221,10 +265,7 @@ func (c *TiKVComponent) Instances() []Instance { s.Port, s.StatusPort, }, - Dirs: []string{ - s.DeployDir, - s.DataDir, - }, + Dirs: dirs, StatusFn: s.Status, UptimeFn: func(_ context.Context, timeout time.Duration, tlsCfg *tls.Config) time.Duration { return UptimeByHost(s.GetManageHost(), s.StatusPort, timeout, tlsCfg) From e768debef8cbb9ca3f0e491f2cc42a13bed3df89 Mon Sep 17 00:00:00 2001 From: lucasliang Date: Tue, 3 Dec 2024 11:20:01 +0800 Subject: [PATCH 2/3] Del paths if needs. Signed-off-by: lucasliang --- pkg/cluster/operation/destroy.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/pkg/cluster/operation/destroy.go b/pkg/cluster/operation/destroy.go index 08e53b9e8d..49985942d5 100644 --- a/pkg/cluster/operation/destroy.go +++ b/pkg/cluster/operation/destroy.go @@ -382,6 +382,15 @@ func DestroyComponent(ctx context.Context, instances []spec.Instance, cls spec.T } } + // For TiKV, we need to delete all extra data directories if the data is not retained. + if ins.ComponentName() == spec.ComponentTiKV { + for _, dir := range ins.UsedDirs() { + if !dataRetained { + delPaths.Insert(dir) + } + } + } + // For TiFlash, we need to delete storage.remote.cache.dir if ins.ComponentName() == spec.ComponentTiFlash { tiflashInstance := ins.(*spec.TiFlashInstance) From 3b7a0aca012729e43d59787cc82f052c66afc9ec Mon Sep 17 00:00:00 2001 From: lucasliang Date: Tue, 3 Dec 2024 18:06:36 +0800 Subject: [PATCH 3/3] Add extra check on nested configurations. Signed-off-by: lucasliang --- pkg/cluster/spec/parse_topology_test.go | 7 ++- pkg/cluster/spec/tikv.go | 58 +++++++++++++++++++------ 2 files changed, 48 insertions(+), 17 deletions(-) diff --git a/pkg/cluster/spec/parse_topology_test.go b/pkg/cluster/spec/parse_topology_test.go index 45b1020753..b880325bfb 100644 --- a/pkg/cluster/spec/parse_topology_test.go +++ b/pkg/cluster/spec/parse_topology_test.go @@ -291,8 +291,9 @@ server_configs: tikv: storage.reserve-space: 1K storage.data-dir: /data1/tikv - raft-engine.dir: /data1/data/raft - raft-engine.spill-dir: /data1/data/spill + raft-engine: + dir: /data1/data/raft + spill-dir: /data1/data/spill rocksdb.wal-dir: /data2/data/wal rocksdb.titan.dirname: /data2/data/titan @@ -311,8 +312,6 @@ tikv_servers: c.Assert(topo.TiKVServers[0].LogDir, check.Equals, "/home/tidb/my-global-deploy/tikv-20161/my-global-log") c.Assert(topo.ServerConfigs.TiKV["storage.data-dir"], check.Equals, "/data1/tikv") - c.Assert(topo.ServerConfigs.TiKV["raft-engine.dir"], check.Equals, "/data1/data/raft") - c.Assert(topo.ServerConfigs.TiKV["raft-engine.spill-dir"], check.Equals, "/data1/data/spill") c.Assert(topo.ServerConfigs.TiKV["rocksdb.wal-dir"], check.Equals, "/data2/data/wal") c.Assert(topo.ServerConfigs.TiKV["rocksdb.titan.dirname"], check.Equals, "/data2/data/titan") diff --git a/pkg/cluster/spec/tikv.go b/pkg/cluster/spec/tikv.go index c3653f05a6..fbb58019a9 100644 --- a/pkg/cluster/spec/tikv.go +++ b/pkg/cluster/spec/tikv.go @@ -166,6 +166,7 @@ func (s *TiKVSpec) Labels() (map[string]string, error) { // getExtraDeployDirs returns the extra directories that needs to be checked. func (s *TiKVSpec) getExtraDeployDirs(globalConf map[string]any) []string { + // Extract the directory from the configuration. var extractDir = func(dir any, rootPath string) string { realDir := strings.TrimSpace(dir.(string)) // Skip the relative path under the root path. @@ -175,6 +176,37 @@ func (s *TiKVSpec) getExtraDeployDirs(globalConf map[string]any) []string { elemStr := strings.TrimSuffix(realDir, "/") return elemStr } + // Search the directory from the nested configuration. + var searchDir = func(fieldName string, confs map[string]any) string { + if confs == nil { + return "" + } + steps := strings.Split(fieldName, ".") + if len(steps) == 0 { + return "" + } + // Search the directory from the nested configuration. + var tmpConfs map[any]any + for _, step := range steps { + if step == steps[0] { + tmpConfs, _ = confs[steps[0]].(map[any]any) + if tmpConfs == nil { + return "" + } + continue + } + nextTmpConfs, ok := tmpConfs[step].(map[any]any) + if !ok || nextTmpConfs == nil { + break + } + tmpConfs = nextTmpConfs + } + dir, ok := tmpConfs[steps[len(steps)-1]] + if !ok { + return "" + } + return strings.TrimSpace(dir.(string)) + } extraDirs := []string{} rootPath := s.DataDir @@ -186,20 +218,20 @@ func (s *TiKVSpec) getExtraDeployDirs(globalConf map[string]any) []string { "raft-engine.spill-dir", } for _, candidate := range candidates { - // Check the global configuration first. - globalConfDir, ok := globalConf[candidate] - if ok { - elemStr := extractDir(globalConfDir, rootPath) - if elemStr != "" { - extraDirs = append(extraDirs, elemStr) + // Check the global configurations and the instance configurations. + for _, conf := range []any{globalConf, s.Config} { + config := conf.(map[string]any) + dir, ok := config[candidate] + if ok { + elemStr := extractDir(dir, rootPath) + if elemStr != "" { + extraDirs = append(extraDirs, elemStr) + } } - } - // Check the instance configuration. - dir, ok := s.Config[candidate] - if ok { - elemStr := extractDir(dir, rootPath) - if elemStr != "" { - extraDirs = append(extraDirs, elemStr) + // Check the configuration with nested fields. + dataDir := searchDir(candidate, config) + if dataDir != "" { + extraDirs = append(extraDirs, dataDir) } } }