Enhance default config behavior
Nobody has claimed this yet.
- Dominant language
- C++
- Stars
- 1k
- Forks
- 423
- Avg merge
- 1d 15h
- Merged PRs (30d)
- 24
Description
Enhancement
Currently many TiFlash config default values are evaluated in a wrong way. For example, given the following two config files, the evaluated final config are different!
Config-A: Final enable_ps_v3 = false
tcp_port = 9000
tmp_path = "/tmp/tiflash/data/tmp"
path = "/tmp/tiflash/data/db"
[storage]
[logger]
console = 1
level = "trace"
Config-B: Final enable_ps_v3 = true
tcp_port = 9000
tmp_path = "/tmp/tiflash/data/tmp"
path = "/tmp/tiflash/data/db"
# [storage] <---- No storage section
[logger]
console = 1
level = "trace"
The root cause of this specific issue is that, enable_ps_v3 is evaluated in the following way:
if (config.has("storage"))
{
storage_config.parseMisc(config.getString("storage"), log);
}
// In parseMisc:
if (table->contains("enable_ps_v3"))
{
...
}
else
{
...
enable_ps_v3 = true;
}
Thus, if [storage] is not defined, the default value of enable_ps_v3 will be evaluated different.
Throughout the whole code base, not only enable_ps_v3, but also other config items have similar problems.
We can improve the config architecture, to achieve these goals:
- In the code base, there should be only one place defining the default value of config item.
- The default value of each config is effective as long as this config item is not explicitly defined in the config file.
- Optional: Out-of-box -- It would be best if we don't need to even specify any config file, and all config items are filled with a good default values.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing the storage_config.parseMisc path and the config.has("storage") and config.getString("storage") checks shown in the issue. Compare Config-A and Config-B while reviewing how enable_ps_v3 and other defaults are assigned. Done means omitted settings receive the same defaults regardless of section presence, with defaults defined in one place.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- backend
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 30/100