Support specify configuration file of TiDB/TiKV/PD when starting playground#80
Conversation
|
Thanks for your contribution. If your PR get merged, you will be rewarded 100 points. |
Codecov Report
@@ Coverage Diff @@
## master #80 +/- ##
==========================================
+ Coverage 29.33% 32.81% +3.48%
==========================================
Files 17 18 +1
Lines 958 902 -56
==========================================
+ Hits 281 296 +15
+ Misses 625 557 -68
+ Partials 52 49 -3
Continue to review full report at Codecov.
|
|
PTAL @lonng |
| $ tiup playground nightly # Start a TiDB nightly version local cluster | ||
| $ tiup playground v3.0.10 --db 3 --pd 3 --kv 3 # Start a local cluster with 10 nodes | ||
| $ tiup playground nightly --monitor # Start a local cluster with monitor system | ||
| $ tiup playground --pd.config ./pd.toml --db.config ./db.toml --kv.config ./kv.toml # Start a local cluster with specified configuration file`, |
There was a problem hiding this comment.
I think we don't need to pass all config flags in examples. e.g
tiup playground --pd.config ./pd.tom
And the playground component has a different work directory with the tiup. So I think the example won't work properly.
| fmt.Sprintf("--advertise-client-urls=http://%s:%d", inst.Host, inst.StatusPort), | ||
| fmt.Sprintf("--log-file=%s", filepath.Join(inst.Dir, "pd.log")), | ||
| } | ||
| if inst.ConfigPath != "" { |
There was a problem hiding this comment.
We can remove other flags even if we specify a configuration file. We should keep the original flags and append --config if the config flag exists.
| fmt.Sprintf("--path=%s", strings.Join(endpoints, ",")), | ||
| fmt.Sprintf("--log-file=%s", filepath.Join(inst.Dir, "tidb.log")), | ||
| } | ||
| if inst.ConfigPath != "" { |
| version = args[0] | ||
| } | ||
| return bootCluster(version, pdNum, tidbNum, tikvNum, host, monitor) | ||
| return bootCluster(version, pdConfigPath, dbConfigPath, kvConfigPath, pdNum, tidbNum, tikvNum, host, monitor) |
There was a problem hiding this comment.
The signature of function bootCluster too complex to hard to maintain, you can define a struct, eg.
type bootOptions {
version...
}There was a problem hiding this comment.
OK, bootOptions added.
| } | ||
|
|
||
| func bootCluster(version string, pdNum, tidbNum, tikvNum int, host string, monitor bool) error { | ||
| func bootCluster(version, pdConfigPath, dbConfigPath, kvConfigPath string, pdNum, tidbNum, tikvNum int, host string, monitor bool) error { |
There was a problem hiding this comment.
I think we should make further handling for the pdConfigPath/dbConfigPath/kvConfigPath and convert it to an absolute path.
According to https://github.com/pingcap-incubator/tiup/blob/e08d28e360c6aee5044a9308c74eb994281be4e4/cmd/run.go#L198. Them will be related to the playground work directory, which is not same as the working directory of the user running tiup playground.
We can achieve our intention via the following code snippet.
if !strings.HasPrefix(pdConfigPath, "/") && !strings.HasPrefix(pdConfigPath, "~") {
pdConfigPath = filepath.Join(os.Getenv(localdata.EnvNameWorkDir), pdConfigPath)
}
There was a problem hiding this comment.
@fredchenbj define a method getAbsolutePath in #82 , i think i can reuse that method here. But redefine that method in my pr seems redundant and could cause conflicts. Maybe it is better to fix this problem after #82 merged so that i can use getAbsolutePath here. Is that OK?
|
All fixed, @lonng PTAL |
5 participants
UCP #64
What problem does this PR solve?
Support specify configuration file of TiDB/TiKV/PD when starting playground. for #64
What is changed and how it works?
Add flags
db.configpd.configkv.configfor playground, and pass their values to TiKV/TiDB/pd command as configuration file path when starting playground. If these flags are not specified, use default config.Check List
Tests