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

store/tikv: make cache ttl configurable #12647

Merged
merged 3 commits into from
Oct 14, 2019
Merged

store/tikv: make cache ttl configurable #12647

merged 3 commits into from
Oct 14, 2019

Conversation

disksing
Copy link
Contributor

Signed-off-by: disksing i@disksing.com

What problem does this PR solve?

Fix #12646

What is changed and how it works?

Add item in config file.

Check List

Tests

  • Unit test

Related changes

  • Need to cherry-pick to the release branch

Release note

  • Support configuring RegionCacheTTL

Signed-off-by: disksing <i@disksing.com>
@codecov
Copy link

codecov bot commented Oct 12, 2019

Codecov Report

Merging #12647 into master will decrease coverage by 0.0437%.
The diff coverage is 50%.

@@               Coverage Diff                @@
##             master     #12647        +/-   ##
================================================
- Coverage   79.9038%   79.8601%   -0.0437%     
================================================
  Files           462        461         -1     
  Lines        104393     103804       -589     
================================================
- Hits          83414      82898       -516     
+ Misses        14889      14814        -75     
- Partials       6090       6092         +2

@@ -70,6 +71,7 @@ txn-total-size-limit=2000
[tikv-client]
commit-timeout="41s"
max-batch-size=128
region-cache-ttl=6000
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems that we don't check it. region-cache-ttl is written to configure file, but we don't check it.

@@ -283,6 +283,10 @@ batch-wait-size = 8
# Enable chunk encoded data.
enable-arrow = true

# If a Region has not been accessed for more than the given duration, it
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to add the unit of region-cache-ttl to the comment (second)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea 👍

Signed-off-by: disksing <i@disksing.com>
@disksing disksing requested review from lonng and zimulala October 12, 2019 09:10
Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

LGTM

@lonng lonng added the status/LGT1 Indicates that a PR has LGTM 1. label Oct 14, 2019
Copy link
Contributor

@zimulala zimulala left a comment

Choose a reason for hiding this comment

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

LGTM

@zimulala zimulala added status/LGT2 Indicates that a PR has LGTM 2. status/can-merge Indicates a PR has been approved by a committer. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Oct 14, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 14, 2019

/run-all-tests

@sre-bot sre-bot merged commit 8137c74 into pingcap:master Oct 14, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Oct 14, 2019

cherry pick to release-3.0 failed

@sre-bot
Copy link
Contributor

sre-bot commented Oct 14, 2019

cherry pick to release-3.1 failed

@sre-bot
Copy link
Contributor

sre-bot commented Apr 7, 2020

It seems that, not for sure, we failed to cherry-pick this commit to release-3.0. Please comment '/run-cherry-picker' to try to trigger the cherry-picker if we did fail to cherry-pick this commit before. @disksing PTAL.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make RegionCacheTTL configurable
4 participants