server: support command line output config info #12956

glorv · 2022-07-05T06:43:43Z

Signed-off-by: glorv glorvs@163.com

What is changed and how it works?

Issue Number: ref #12492

What's Changed:

Add a new command line parameter `--config-info json` to output all config infos. Currently, the command only support Name, DefaultValue and ValueInFile 3 fields.

This is a simplified implementation of #12517 that only support output "Name, DefaultValue and ValueInFile", so we can avoid using the complex proc-macro.

Related changes

PR to update pingcap/docs/pingcap/docs-cn:
Need to cherry-pick to the release branch

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No code

Side effects

Performance regression
- Consumes more CPU
- Consumes more MEM
Breaking backward compatibility

Release note

Add a new command line parameter `--config-info` to output some information of all configs.

Signed-off-by: glorv <glorvs@163.com>

ti-chi-bot · 2022-07-05T06:43:44Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

BusyJay
tabokie

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

Signed-off-by: glorv <glorvs@163.com>

… json-config-info

Signed-off-by: glorv <glorvs@163.com>

glorv · 2022-07-06T02:50:54Z

@BusyJay @tabokie PTAL, thank you

tabokie · 2022-07-06T04:44:21Z

components/server/src/server.rs

+    ) {
+        for (k, v) in default_obj.iter() {
+            let cfg_val = value_obj.get(k);
+            append_sub_key(key_buf, k);


These tiny functions don't make it easy to review. How about

Suggested change

append_sub_key(key_buf, k);

let old_len = key_buf.len();

if !k.is_empty() {

key.push_str(k);

key.push('.');

}

tabokie · 2022-07-06T04:44:51Z

components/server/src/server.rs

+            } else {
+                res.push(to_cfg_value(v, cfg_val, key_buf));
+            }
+            pop_sub_key(key_buf, k);


Suggested change

pop_sub_key(key_buf, k);

key_buf.truncate(old_len);

tabokie · 2022-07-06T04:51:13Z

components/server/src/server.rs

@@ -1851,3 +1854,118 @@ impl IOBudgetAdjustor for EnginesResourceInfo {
        (total_budgets as f32 * score) as usize
    }
 }
+
+// currently, only support return json result
+pub fn get_flatten_cfg_info(cfg: &TiKvConfig) -> Vec<Value> {


Put it near TiKvConfig?

Signed-off-by: glorv <glorvs@163.com>

glorv · 2022-07-12T03:26:11Z

@tabokie @BusyJay PTAL again, thanks~

tabokie

Could you paste a sample output? I'm not sure if deprecated items are printed.

glorv · 2022-07-12T04:30:21Z

Could you paste a sample output? I'm not sure if deprecated items are printed.

emm.. The currently implement outputs all serializable config fields. The configs included are the same as all the configs in the last_tikv.toml. So deprecated items are also included. Do you have any good ideas to excludes them? @tabokie

The current output is like:

[gl@ip-172-16-5-33 tikv]$ ./target/debug/tikv-server --config tests/integrations/config/test-custom.toml --config-info json
{
  "Component": "TiKV Server",
  "Version": "6.2.0-alpha",
  "Parameters": [
    {
      "Name": "log-level",
      "DefaultValue": "info"
    },
    {
      "Name": "log-file",
      "DefaultValue": ""
    },
    {
      "Name": "log-format",
      "DefaultValue": "text"
    },
    {
      "Name": "log-rotation-timespan",
      "DefaultValue": "0s",
      "ValueInFile": "1d"
    },
    {
      "Name": "log-rotation-size",
      "DefaultValue": "300MiB"
    },
    {
      "Name": "slow-log-file",
      "DefaultValue": "",
      "ValueInFile": "slow_foo"
    },
    {
      "Name": "slow-log-threshold",
      "DefaultValue": "1s"
    },
    {
      "Name": "panic-when-unexpected-key-or-data",
      "DefaultValue": false,
      "ValueInFile": true
    },
    {
      "Name": "abort-on-panic",
      "DefaultValue": false,
      "ValueInFile": true
    },
   ...
  ]
}

tabokie · 2022-07-12T04:35:23Z

@glorv deprecated items are all marked with #[online_config(skip)] and #[serde(skip_serializing)]. So I guess they won't appear in the list of ConfigValue?

You can verify it by checking a deprecated config e.g. xxcf.titan.gc-merge-rewrite.

If #[online_config(skip)] doesn't work, maybe try #[online_config(hidden)].

Signed-off-by: glorv <glorvs@163.com>

… json-config-info

glorv · 2022-07-12T06:20:30Z

@glorv deprecated items are all marked with #[online_config(skip)] and #[serde(skip_serializing)]. So I guess they won't appear in the list of ConfigValue?

You can verify it by checking a deprecated config e.g. xxcf.titan.gc-merge-rewrite.

If #[online_config(skip)] doesn't work, maybe try #[online_config(hidden)].

I have checked all the configs, there are only 8 commented with // deprecated. And there are 19 fields that are marked with #[serde(skip_serializing)]. These fields won't be included in the output.

glorv · 2022-07-15T01:57:03Z

@tabokie can we merge this pr now?

tabokie · 2022-07-15T02:05:13Z

Sure, I already approved it.

tabokie · 2022-07-15T02:55:56Z

/merge

ti-chi-bot · 2022-07-15T02:55:57Z

@tabokie: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2022-07-15T02:55:59Z

This pull request has been accepted and is ready to merge.

Commit hash: 8033514

add config info

d485d68

Signed-off-by: glorv <glorvs@163.com>

ti-chi-bot added the release-note label Jul 5, 2022

Merge branch 'master' into json-config-info

c493de8

ti-chi-bot added the size/L label Jul 5, 2022

glorv added 4 commits July 5, 2022 14:49

remove useless deps

5c9a63e

Signed-off-by: glorv <glorvs@163.com>

Merge branch 'json-config-info' of https://github.com/glorv/tikv into…

3828671

… json-config-info

fix clippy

d6ec393

Signed-off-by: glorv <glorvs@163.com>

Merge branch 'master' into json-config-info

6f8a450

glorv requested review from tabokie and BusyJay July 6, 2022 02:50

tabokie reviewed Jul 6, 2022

View reviewed changes

glorv added 2 commits July 6, 2022 20:22

resolve comments

0ec7ad5

Signed-off-by: glorv <glorvs@163.com>

fmt code

7c75e95

Signed-off-by: glorv <glorvs@163.com>

BusyJay approved these changes Jul 12, 2022

View reviewed changes

ti-chi-bot added the status/LGT1 Status: PR - There is already 1 approval label Jul 12, 2022

tabokie approved these changes Jul 12, 2022

View reviewed changes

ti-chi-bot added status/LGT2 Status: PR - There are already 2 approvals and removed status/LGT1 Status: PR - There is already 1 approval labels Jul 12, 2022

Merge branch 'master' into json-config-info

fba4ea7

glorv added 2 commits July 12, 2022 14:13

skip serialize enable_multi_batch_write

2176fb1

Signed-off-by: glorv <glorvs@163.com>

Merge branch 'json-config-info' of https://github.com/glorv/tikv into…

7b65e16

… json-config-info

Merge branch 'master' into json-config-info

35e4a3b

Merge branch 'master' into json-config-info

8033514

ti-chi-bot added the status/can-merge Status: Can merge to base branch label Jul 15, 2022

ti-chi-bot merged commit 956c219 into tikv:master Jul 15, 2022

ti-chi-bot added this to the Pool milestone Jul 15, 2022

glorv deleted the json-config-info branch July 15, 2022 03:09

Oreoxmt mentioned this pull request Jul 25, 2022

tikv: add command line flag --config-info pingcap/docs-cn#10615

Merged

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

server: support command line output config info #12956

server: support command line output config info #12956

glorv commented Jul 5, 2022

ti-chi-bot commented Jul 5, 2022 •

edited

glorv commented Jul 6, 2022

tabokie Jul 6, 2022

tabokie Jul 6, 2022

tabokie Jul 6, 2022

glorv commented Jul 12, 2022

tabokie left a comment

glorv commented Jul 12, 2022 •

edited

tabokie commented Jul 12, 2022 •

edited

glorv commented Jul 12, 2022

glorv commented Jul 15, 2022

tabokie commented Jul 15, 2022

tabokie commented Jul 15, 2022

ti-chi-bot commented Jul 15, 2022

ti-chi-bot commented Jul 15, 2022

-            append_sub_key(key_buf, k);
+            let old_len = key_buf.len();
+            if !k.is_empty() {
+              key.push_str(k);
+              key.push('.');
+            }

server: support command line output config info #12956

server: support command line output config info #12956

Conversation

glorv commented Jul 5, 2022

What is changed and how it works?

Related changes

Check List

Release note

ti-chi-bot commented Jul 5, 2022 • edited

glorv commented Jul 6, 2022

tabokie Jul 6, 2022

Choose a reason for hiding this comment

tabokie Jul 6, 2022

Choose a reason for hiding this comment

tabokie Jul 6, 2022

Choose a reason for hiding this comment

glorv commented Jul 12, 2022

tabokie left a comment

Choose a reason for hiding this comment

glorv commented Jul 12, 2022 • edited

tabokie commented Jul 12, 2022 • edited

glorv commented Jul 12, 2022

glorv commented Jul 15, 2022

tabokie commented Jul 15, 2022

tabokie commented Jul 15, 2022

ti-chi-bot commented Jul 15, 2022

ti-chi-bot commented Jul 15, 2022

ti-chi-bot commented Jul 5, 2022 •

edited

glorv commented Jul 12, 2022 •

edited

tabokie commented Jul 12, 2022 •

edited