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: speed up conf change #6421

Merged
merged 5 commits into from Jan 7, 2020
Merged

raftstore: speed up conf change #6421

merged 5 commits into from Jan 7, 2020

Conversation

@BusyJay
Copy link
Contributor

BusyJay commented Jan 6, 2020

What have you changed?

It speeds up conf change by sending more heartbeats. The heartbeats are
not frequent and the overhead is quite small.

What is the type of the changes?

  • Bugfix

How is the PR tested?

  • Unit test
  • Integration test

Does this PR affect documentation (docs) or should it be mentioned in the release notes?

No.

Does this PR affect tidb-ansible?

No

Refer to a related PR or issue link (optional)

Close #6420.

It speeds up conf change by sending more heartbeats. The heartbeats are
not frequent and the overhead is quite small.

Close #6420

Signed-off-by: Jay Lee <busyjaylee@gmail.com>
@BusyJay BusyJay requested review from overvenus, hicqu and gengliqi Jan 6, 2020
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
Copy link
Contributor

overvenus left a comment

LGTM

@BusyJay

This comment has been minimized.

Copy link
Contributor Author

BusyJay commented Jan 6, 2020

@5kbpers do you have any idea why the test fail?

Copy link
Member

gengliqi left a comment

Rest LGTM

add timer
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
Copy link
Member

gengliqi left a comment

LGTM

@gengliqi gengliqi added the S: CanMerge label Jan 7, 2020
@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Jan 7, 2020

/run-all-tests

@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Jan 7, 2020

@BusyJay merge failed.

@overvenus

This comment has been minimized.

Copy link
Contributor

overvenus commented Jan 7, 2020

/merge

@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Jan 7, 2020

/run-all-tests

@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Jan 7, 2020

@BusyJay merge failed.

@@ -1472,6 +1477,9 @@ impl<'a, T: Transport, C: PdClient> PeerFsmDelegate<'a, T, C> {
let id = peer.get_id();
self.fsm.peer.peer_heartbeats.insert(id, now);
if self.fsm.peer.is_leader() {
// Speed up snapshot instead of waiting another heartbeat.
self.fsm.peer.ping();

This comment has been minimized.

Copy link
@Connor1996

Connor1996 Jan 7, 2020

Member

seems it's only needed by AddLearnerNode?

This comment has been minimized.

Copy link
@BusyJay

BusyJay Jan 7, 2020

Author Contributor

Just in case learner is disabled.

@5kbpers

This comment has been minimized.

Copy link
Contributor

5kbpers commented Jan 7, 2020

@5kbpers do you have any idea why the test fail?

Not sure, it seems the ReadIndex is dropped, I'm investigating that.

@BusyJay BusyJay added S: CanMerge and removed S: CanMerge labels Jan 7, 2020
@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Jan 7, 2020

/run-all-tests

@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Jan 7, 2020

@BusyJay merge failed.

@BusyJay

This comment has been minimized.

Copy link
Contributor Author

BusyJay commented Jan 7, 2020

/run-test

@BusyJay

This comment has been minimized.

Copy link
Contributor Author

BusyJay commented Jan 7, 2020

/test

@AndreMouche

This comment has been minimized.

Copy link
Member

AndreMouche commented Jan 7, 2020

/merge

@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Jan 7, 2020

/run-all-tests

@sre-bot sre-bot merged commit 80b5d60 into tikv:master Jan 7, 2020
6 checks passed
6 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-copr-test Jenkins job succeeded.
Details
idc-jenkins-ci-tikv/integration-ddl-test Jenkins job succeeded.
Details
idc-jenkins-ci/test Jenkins job succeeded.
Details
@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Jan 7, 2020

cherry pick to release-3.1 failed

@sre-bot

This comment has been minimized.

Copy link
Collaborator

sre-bot commented Jan 7, 2020

cherry pick to release-3.0 failed

BusyJay added a commit to BusyJay/tikv that referenced this pull request Jan 7, 2020
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
BusyJay added a commit to BusyJay/tikv that referenced this pull request Jan 7, 2020
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
BusyJay added a commit to BusyJay/tikv that referenced this pull request Jan 7, 2020
Signed-off-by: Jay Lee <busyjaylee@gmail.com>
@BusyJay BusyJay deleted the BusyJay:fast-conf-change branch Jan 7, 2020
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.