pingcap / pingcap/tiflash

Enhance default config behavior

Open
#4,809 0 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

good first issue help wanted type/enhancement
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:

  1. In the code base, there should be only one place defining the default value of config item.
  2. The default value of each config is effective as long as this config item is not explicitly defined in the config file.
  3. 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

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.