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

pd/raftstore: support region half-split without scan #3310

Merged
merged 11 commits into from
Jul 18, 2018

Conversation

Connor1996
Copy link
Member

What have you changed? (mandatory)

Now tikv support get region approximate middle key based on region properties, so make region half-split without scan. pingcap/kvproto#287 add a check-policy field for split-region request. And this PR makes the way of check-split to get split-key depend on policy. If SCAN, do as before, if APPROXIMATE, use approximate middle key.

limit: APPROXIMATE policy doesn't support for table-split-checker and size-split-checker.

What are the type of the changes? (mandatory)

Improvement (non-breaking change which is an improvement to an existing feature)

How has this PR been tested? (mandatory)

unit-test

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

N/A

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

N/A

Refer to a related PR or issue link (optional)

pingcap/kvproto#287

CheckPolicy::APPROXIMATE => {
let res = raftstore_util::get_region_approximate_middle(&self.engine, region);
if let Err(e) = res {
debug!("[region {}] failed to get approxiamte split key: {}", region_id, e);
Copy link
Member

Choose a reason for hiding this comment

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

Please use error! here.

host.split_key()
}
CheckPolicy::APPROXIMATE => {
let res = raftstore_util::get_region_approximate_middle(&self.engine, region);
Copy link
Member

Choose a reason for hiding this comment

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

Middle and approximate are not related, here looks strange.

@zhangjinpeng87
Copy link
Member

Good job, BTW have you test it with pd-ctl?

@@ -102,6 +102,11 @@ pub trait SplitChecker {

/// Get the desired split keys.
fn split_key(&mut self) -> Option<Vec<u8>>;

/// Get approximate split keys without scan.
fn approximate_split_key(&self, _: &Region, _: &DB) -> Result<Option<Vec<u8>>> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you combine this into split_key? We don't need to know how it gets the split key outside.

Copy link
Member Author

Choose a reason for hiding this comment

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

since to get not approximate split key, we must call on_kv() first. So outside caller it does know what the policy is.

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

Copy link
Member

@zhangjinpeng87 zhangjinpeng87 left a comment

Choose a reason for hiding this comment

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

LGTM

@Connor1996 Connor1996 merged commit 825aca7 into tikv:master Jul 18, 2018
@Connor1996 Connor1996 deleted the half-split branch July 18, 2018 06:53
sticnarf pushed a commit to sticnarf/tikv that referenced this pull request Oct 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants