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

add option: restrict rocksdb log file num #3259

Merged
merged 6 commits into from
Jul 3, 2018

Conversation

DorianZheng
Copy link
Contributor

What have you changed? (mandatory)

Add an option to DBConfig & RaftDBConfig to restrict the info log file num

What are the type of the changes? (mandatory)

  • Improvement

How has this PR been tested? (mandatory)

Write a test case

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

No

Does this PR affect tidb-ansible update? (mandatory)

No

@DorianZheng DorianZheng requested a review from huachaohuang June 29, 2018 03:03
@@ -394,6 +394,7 @@ pub struct DbConfig {
pub compaction_readahead_size: ReadableSize,
pub info_log_max_size: ReadableSize,
pub info_log_roll_time: ReadableDuration,
pub info_log_keep_log_file_num: u64,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can default to 10 * 1GB log files.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@@ -171,6 +171,7 @@ fn test_serde_custom_tikv_config() {
compaction_readahead_size: ReadableSize::kb(1),
info_log_max_size: ReadableSize::kb(1),
info_log_roll_time: ReadableDuration::secs(12),
info_log_keep_log_file_num: 1000,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may want to use a different value than the default for the test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor

@huachaohuang huachaohuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to change the config-template.toml too.
Rest LGTM.

# If specified with non-zero value, log file will be rolled
# if it has been active longer than `log_file_time_to_roll`.
# Default: 0 (disabled)
# info-log-roll-time = "1s"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be the same as the default value.

@DorianZheng
Copy link
Contributor Author

/run-all-tests

Copy link
Contributor

@huachaohuang huachaohuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@breezewish
Copy link
Member

LGTM. Please also remember to file a PR in tidb-ansible repository, like this.

@DorianZheng DorianZheng merged commit f21722a into tikv:master Jul 3, 2018
@DorianZheng DorianZheng deleted the retrict_rocksdb_log_file_num branch July 3, 2018 02:44
DorianZheng added a commit to DorianZheng/tikv that referenced this pull request Jul 25, 2018
* add option: restrict rocksdb log file num

* add test case

* tune default log size and num to 1GB * 10

* add rocksdb info log options to conf template

* modify initial value in conf template
huachaohuang pushed a commit that referenced this pull request Jul 26, 2018
* add option: restrict rocksdb log file num

* add test case

* tune default log size and num to 1GB * 10

* add rocksdb info log options to conf template

* modify initial value in conf template
sticnarf pushed a commit to sticnarf/tikv that referenced this pull request Oct 27, 2019
* add option: restrict rocksdb log file num

* add test case

* tune default log size and num to 1GB * 10

* add rocksdb info log options to conf template

* modify initial value in conf template
@sre-bot sre-bot added the contribution This PR is from a community contributor. label Dec 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution This PR is from a community contributor.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants