Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compaction-style setting is broken #11028

Closed
kamijin-fanta opened this issue Oct 11, 2021 · 5 comments · Fixed by #11068
Closed

compaction-style setting is broken #11028

kamijin-fanta opened this issue Oct 11, 2021 · 5 comments · Fixed by #11068
Assignees
Labels
component/configuration Component: Configuration difficulty/easy Difficulty: Easy. Ideal for beginners. type/bug Type: Issue - Confirmed a bug

Comments

@kamijin-fanta
Copy link
Contributor

Bug Report

What version of TiKV are you using?

using tiup for deployment.

TiKV
Release Version:   5.2.1
Edition:           Community
Git Commit Hash:   2c99f317d4ba125b772a8b94a6c3c0eb9d07ac59
Git Commit Branch: heads/refs/tags/v5.2.1
UTC Build Time:    2021-09-08 02:32:36
Rust Version:      rustc 1.56.0-nightly (2faabf579 2021-07-27)
Enable Features:   jemalloc mem-profiling portable sse protobuf-codec test-engines-rocksdb cloud-aws cloud-gcp
Profile:           dist_release

What operating system and CPU are you using?

It seems to be a machine-independent bug.

Steps to reproduce

The server will not start if the following settings are made.

# tiup-topology.yaml
server_configs:
  # tidb:
  tikv:
    rocksdb:
      defaultcf:
        compaction-style: universal
# /opt/tidb/deploy/tikv-20160/conf/tikv.toml
[rocksdb]
[rocksdb.defaultcf]
compaction-style = "universal"
Error: init config failed: 192.168.50.3:20160: executor.ssh.execute_failed: Failed to execute command over SSH for '********@********:22' {ssh_stderr: thread 'main' panicked at 'invalid auto generated configuration file /opt/tidb/deploy/tikv-20160/conf/tikv.toml, err invalid type: string "level", expected valid DBCompactionStyle for key `rocksdb.defaultcf.compaction-style` at line 10 column 20', src/config.rs:2836:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
, ssh_stdout: , ssh_command: export LANG=C; PATH=$PATH:/bin:/sbin:/usr/bin:/usr/sbin /opt/tidb/deploy/tikv-20160/bin/tikv-server --config-check --config=/opt/tidb/deploy/tikv-20160/conf/tikv.toml --pd "" --data-dir "/opt/tidb/data/tikv-20160"}, cause: Process exited with status 101: check config failed

What did you expect?

The reference looks like a correct description.
https://docs.pingcap.com/tidb/stable/tikv-configuration-file#compaction-style

What did happened?

If specify compaction-style: 1, it appears to start successfully.

@tabokie
Copy link
Member

tabokie commented Oct 11, 2021

/assign @yuqi1129

@ti-chi-bot
Copy link
Member

@tabokie: GitHub didn't allow me to assign the following users: yuqi1129.

Note that only tikv members, repo collaborators and people who have commented on this issue/PR can be assigned. Additionally, issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @yuqi1129

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 kubernetes/test-infra repository.

@yuqi1129
Copy link
Contributor

/assign @yuqi1129

@yuqi1129
Copy link
Contributor

@tabokie I have test this configuration and it seems that only numeric is supported like compaction-pri. Sounds like we need to update user docs.

numeric_enum_mod! {compaction_style_serde DBCompactionStyle {
    Level = 0,
    Universal = 1,
}}

@tabokie
Copy link
Member

tabokie commented Oct 12, 2021

@yuqi1129 I prefer to use descriptive words for these config items, just like blob-run-mode, grpc-compression-type, etc. So I think you should submit a code patch to make compaction-pri accept string literals.

As for compaction-pri, it is problematic too, but we probably shouldn't change it directly for backward compatibility.

@tabokie tabokie added component/configuration Component: Configuration difficulty/easy Difficulty: Easy. Ideal for beginners. type/bug Type: Issue - Confirmed a bug labels Oct 12, 2021
@github-actions github-actions bot added this to Need Triage in Question and Bug Reports Oct 12, 2021
@tabokie tabokie moved this from Need Triage to In Progress: handled by SIGs in Question and Bug Reports Oct 12, 2021
yuqi1129 added a commit to yuqi1129/tikv that referenced this issue Oct 26, 2021
Change macro `numeric_enum_mod`:
 1. serialize config value as string instead of int
 2. accept int and string as config value for backwards compatibility

Close tikv#11028

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>
yuqi1129 added a commit to yuqi1129/tikv that referenced this issue Oct 26, 2021
Change macro `numeric_enum_mod`:
 1. serialize config value as string instead of int
 2. accept int and string as config value for backwards compatibility

Close tikv#11028

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>
yuqi1129 added a commit to yuqi1129/tikv that referenced this issue Oct 26, 2021
Change macro `numeric_enum_mod`:
 1. serialize config value as string instead of int
 2. accept int and string as config value for backwards compatibility

Close tikv#11028

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>
Question and Bug Reports automation moved this from In Progress: handled by SIGs to Closed(This Week) Nov 11, 2021
ti-chi-bot pushed a commit that referenced this issue Nov 11, 2021
…11068)

* config: Fix compaction-style setting is broken

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* config: Fix compaction-style setting is broken, fix discussion

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* config: Fix compaction-style setting is broken, fix discussion again

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* config: Fix compaction-style setting is broken, fix test error

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* config: Fix compaction-style setting is broken

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* config: Fix compaction-style setting is broken

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* config: Fix compaction-style setting is broken

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* config: Fix compaction-style setting is broken

Change macro `numeric_enum_mod`:
 1. serialize config value as string instead of int
 2. accept int and string as config value for backwards compatibility

Close #11028

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* config: Fix compaction-style setting is broken

Change macro `numeric_enum_mod`:
 1. serialize config value as string instead of int
 2. accept int and string as config value for backwards compatibility

Close #11028

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* config: Fix compaction-style setting is broken

Change macro `numeric_enum_mod`:
 1. serialize config value as string instead of int
 2. accept int and string as config value for backwards compatibility

Close #11028

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* merge master and use case macros to replace some code

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* rename numeric_enum_mod and move mirror type to macros

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* fix format in toml

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* remove enums in macro and merge test

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* replace numeric value with string value about config in `config-template.toml` and `test-template.toml` respectively

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* resolve disscussion about rate-limiter-mode

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* fix unittest about config-template.toml

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* revert changes thats serialize config value as str

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>

* resolve discussion about comment content

Signed-off-by: yuqi1129 <yuqi4733@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/configuration Component: Configuration difficulty/easy Difficulty: Easy. Ideal for beginners. type/bug Type: Issue - Confirmed a bug
Projects
Question and Bug Reports
  
Closed(This Week)
Development

Successfully merging a pull request may close this issue.

4 participants