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

tikv: cleanup outdate regions's memory from region-cache in 2.1 #11931

Merged
merged 4 commits into from
Sep 2, 2019
Merged

tikv: cleanup outdate regions's memory from region-cache in 2.1 #11931

merged 4 commits into from
Sep 2, 2019

Conversation

lysu
Copy link
Contributor

@lysu lysu commented Aug 29, 2019

What problem does this PR solve?

current region-cache should not clean up regions out of TTL #4506

this takes the risk to leave many invalid regions in the region-cache, although LocateKey and getCachedRegion can be free lock, it will hurt the DropStoreOnFail's performance(hold longer lock), for a long-running cluster with many regions, send tikv fail will be slow.

What is changed and how it works?

to control the modification, just add clear up logic in DropStoreOnFail(they already hold the lock to loops all regions, so this will not introduce any other locks cost)

Check List

Tests

  • Unit test
  • Integration test

Code changes

  • impl change

Side effects

  • n/a

Related changes

  • n/a

Release note

  • cleanup outdate regions's memory from region-cache

This change is Reviewable

@lysu lysu added type/enhancement The issue or PR belongs to an enhancement. component/tikv labels Aug 29, 2019
@lysu lysu requested review from coocood and disksing August 29, 2019 05:51
@coocood
Copy link
Member

coocood commented Aug 30, 2019

LGTM

@lysu lysu requested a review from tiancaiamao August 30, 2019 06:24
@lysu lysu added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 30, 2019
@lysu lysu requested a review from jackysp September 2, 2019 02:57
@nolouch
Copy link
Member

nolouch commented Sep 2, 2019

The long-time is spent in locateing key? maybe we can also update the btree version to the last, there is an improvement in seek.

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp
Copy link
Member

jackysp commented Sep 2, 2019

Should we update btree in master and release-3.0?

@jackysp jackysp added the status/can-merge Indicates a PR has been approved by a committer. label Sep 2, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 2, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Sep 2, 2019

/run-all-tests

@sre-bot sre-bot merged commit cc6301e into pingcap:release-2.1 Sep 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/tikv status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants