raftstore: support consistency check #1344
Conversation
BusyJay
force-pushed
the
busyjay/consistency-check
branch
from
f3105d7
to
f9646e1
Compare
|
PTAL |
|
ping |
| pub static ref REGION_HASH_COUNTER_VEC: CounterVec = | ||
| register_counter_vec!( | ||
| "tikv_raftstore_hash_total", | ||
| "Total number of raftstore has computing.", |
| @@ -165,6 +171,13 @@ fn notify_region_removed(region_id: u64, peer_id: u64, mut cmd: PendingCmd) { | |||
| cmd.call(resp); | |||
| } | |||
|
|
|||
| pub struct ConsistencyState { | |||
| pub last_check_time: Instant, | |||
| // (computed_result_or_to_be_revified, index, hash) | |||
| return; | ||
| } | ||
|
|
||
| if state.index == expected_index { |
There was a problem hiding this comment.
I think we can extract a function for both here and L1706.
| state.index = index; | ||
| state.hash = hash; | ||
|
|
||
| let mut request = RaftCmdRequest::new(); |
There was a problem hiding this comment.
why not using a new_verify_hash_request like new_compute_hash_request below?
| timer.observe_duration(); | ||
|
|
||
| let mut checksum = Vec::with_capacity(4); | ||
| checksum.write_u32::<BigEndian>(sum).unwrap(); |
There was a problem hiding this comment.
Seem that we use CRC32 for checksum, no need to use Sha1?
There was a problem hiding this comment.
Yes, to reduce the overhead.
|
Rest LGTM |
|
We should add MVCC verify later. |
| @@ -99,6 +102,9 @@ pub struct Config { | |||
| pub max_leader_missing_duration: Duration, | |||
|
|
|||
| pub snap_apply_batch_size: usize, | |||
|
|
|||
| // Interval (s) to check region whether the data are consistent. | |||
| pub static ref REGION_HASH_COUNTER_VEC: CounterVec = | ||
| register_counter_vec!( | ||
| "tikv_raftstore_hash_total", | ||
| "Total number of raftstore has computed.", |
| leader_lease_expired_time: Option<Timespec>, | ||
| leader_missing_time: Option<Instant>, |
There was a problem hiding this comment.
The related fields should be grouped together, such as leader_lease_expired_time and election_timeout.
| Some(ExecResult::ComputeHash { | ||
| region: self.region().clone(), | ||
| index: ctx.index, | ||
| // This snapshot may be hold for a long time, which may cause too many |
| escape(&state.hash)); | ||
| } | ||
| REGION_HASH_COUNTER_VEC.with_label_values(&["verify", "matched"]).inc(); | ||
| state.hash = vec![]; |
There was a problem hiding this comment.
It would be clearer to set and reset state.index and state.hash together every time.
There was a problem hiding this comment.
state.index does not need to be updated here.
|
Rest LGTM |
|
LGTM |
BusyJay
changed the title
ref #1123 #1286 #1314
@siddontang @hhkbp2 PTAL