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
*: refactor split check #2440
*: refactor split check #2440
Conversation
tests/config/test-custom-latest.toml
Outdated
@@ -73,6 +71,10 @@ raft-store-max-leader-lease = "12s" | |||
right-derive-when-split = false | |||
allow-remove-leader = true | |||
|
|||
[coprocessor] | |||
region-max-size = "12MB" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to update ansible?
No need to support compatible config file. |
f5c4927
to
341b9c0
Compare
0e694ac
to
9a54c40
Compare
etc/config-template.toml
Outdated
[coprocessor] | ||
# When it is true, it will try to split a region with table prefix if | ||
# that region crosses tables. | ||
# split-region-on-table = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stale comment.
# When the region's size exceeds region-max-size, we will split the region | ||
# into two which the left region's size will be region-split-size or a little | ||
# bit smaller. | ||
# region-max-size = "144MB" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is set, then coprocessor.region-max-size
should be updated too.
a083df3
to
ac89022
Compare
src/bin/tikv-server.rs
Outdated
@@ -463,6 +463,7 @@ fn main() { | |||
if let Err(e) = config.validate() { | |||
fatal!("invalid configuration: {:?}", e); | |||
} | |||
config.compatible_adjust(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should adjust before validating.
acd7c0a
to
52211d4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rest LGTM
src/raftstore/store/config.rs
Outdated
@@ -202,6 +200,15 @@ impl Config { | |||
)); | |||
} | |||
|
|||
// Compatibility check. | |||
if self.region_max_size.0 != 0 { | |||
warn!("raftstore.region-max-size has been moved to coprocessor"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unreachable.
src/raftstore/store/keys.rs
Outdated
@@ -195,7 +195,7 @@ pub fn data_key(key: &[u8]) -> Vec<u8> { | |||
} | |||
|
|||
pub fn origin_key(key: &[u8]) -> &[u8] { | |||
assert!(validate_data_key(key)); | |||
assert!(validate_data_key(key), "invalid data key {:?}", key); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about util::escape
?
LGTM |
b442760
to
ac48aef
Compare
/run-all-test |
LGTM |
b8fa35f
to
19afb85
Compare
/run-all-test |
@@ -138,6 +133,13 @@ | |||
# Interval (s) to check region whether the data are consistent. | |||
# consistency-check-interval = 0 | |||
|
|||
[coprocessor] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will confuse ops.
Move size split check from SplitCheckRunner to CoprocessorHost. It also move region-max-size and region-split-size to coprocessor configuration. Ref tikv#2378
Move size split check from SplitCheckRunner to CoprocessorHost. It also move
region-max-size
andregion-split-size
to coprocessor configuration.Ref #2378