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

raftstore: add tick registry #4632

Merged
merged 5 commits into from May 6, 2019

Conversation

Projects
None yet
4 participants
@BusyJay
Copy link
Contributor

commented May 6, 2019

What have you changed? (mandatory)

Tick registry is used to avoid schedule a tick twice. It's very useful
when try to schedule tick lazily.

What are the type of the changes? (mandatory)

  • New feature

How has this PR been tested? (mandatory)

unit tests

Does this PR affect documentation (docs) or release note? (mandatory)

No.

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

No.

Refer to a related PR or issue link (optional)

tikv/rfcs#23, #4591

raftstore: add tick registry
Tick registry is used to avoid schedule a tick twice. It's very useful
when try to schedule tick lazily.

Signed-off-by: Jay Lee <busyjaylee@gmail.com>

@BusyJay BusyJay added the C: Raft label May 6, 2019

@BusyJay BusyJay requested review from overvenus, hicqu and Connor1996 May 6, 2019

@Connor1996

This comment has been minimized.

Copy link
Member

commented May 6, 2019

wrong RFC link

@BusyJay

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

wrong RFC link

Fixed.

address comment
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
@@ -65,6 +65,8 @@ pub struct DestroyPeerJob {

pub struct PeerFsm {
peer: Peer,
/// A registry for all scheduled ticks. This can avoid schedule ticks twice accidentally.

This comment has been minimized.

Copy link
@siddontang

siddontang May 6, 2019

Contributor

avoid scheduling ticks

fix comment
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
@@ -608,7 +632,7 @@ impl<'a, T: Transport, C: PdClient> PeerFsmDelegate<'a, T, C> {
.map(move |_| {
fail_point!(
"on_raft_log_gc_tick_1",
peer_id == 1 && tick == PeerTick::RaftLogGc,
peer_id == 1 && tick == PeerTicks::RAFT_LOG_GC,
|_| unreachable!()
);
if let Err(e) = mb.force_send(PeerMsg::Tick(tick)) {

This comment has been minimized.

Copy link
@Connor1996

Connor1996 May 6, 2019

Member

maybe add a comment to indicate that it's fine to not clean registry here cause the error happens when the node is shutting down

address comment
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
@Connor1996
Copy link
Member

left a comment

LGTM

@hicqu

This comment has been minimized.

Copy link
Contributor

commented May 6, 2019

LGTM

@hicqu

hicqu approved these changes May 6, 2019

@BusyJay

This comment has been minimized.

Copy link
Contributor Author

commented May 6, 2019

/run-integration-tests

@BusyJay BusyJay merged commit 9e02b2d into tikv:master May 6, 2019

5 checks passed

DCO All commits are signed off!
Details
idc-jenkins-ci-tikv/integration-common-test Jenkins job succeeded.
Details
idc-jenkins-ci-tikv/integration-compatibility-test Jenkins job succeeded.
Details
idc-jenkins-ci-tikv/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci/test Jenkins job succeeded.
Details

@BusyJay BusyJay deleted the BusyJay:add-tick-registry branch May 6, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.