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

transfer leader may not be safe for lease read #234

Open
BusyJay opened this issue May 9, 2019 · 4 comments
Open

transfer leader may not be safe for lease read #234

BusyJay opened this issue May 9, 2019 · 4 comments
Assignees
Labels
Bug Recognized misbehavior.

Comments

@BusyJay
Copy link
Member

BusyJay commented May 9, 2019

if messages are out of order and MsgTimeoutNow arrives after an election timeout, a new leader may be elected probably as MsgTimeoutNow will trigger election despite of the lease. So even if there is no clock drift, leader's lease can still be stale.

The problem is that when aborting transferring leader, leader just resets its internal field, which is not enough, because MsgTimeoutNow may still being transferring. So leader has to make sure the MsgTimeoutNow message becomes stale. It can either start a new election or add last index and term to MsgTimeoutNow and broadcast an empty entry. Former will make the election triggered by MsgTimeoutNow can't succeed, latter will either make the election can't succeed or MsgTimeoutNow be dropped.

/cc @siddontang

@BusyJay BusyJay added the Bug Recognized misbehavior. label May 9, 2019
@BusyJay
Copy link
Member Author

BusyJay commented May 9, 2019

/cc @tiancaiamao

@jayzhan211
Copy link
Contributor

add last index and term to MsgTimeoutNow and broadcast an empty entry

  1. what is the purpose of broadcast an empty entry
  2. if leader election timeout does not change index or term, but only set transferee to None, I dont think leader_transferee can check term or index in order to drop MsgTimeoutNow

raft-rs/src/raft.rs

Lines 838 to 848 in f145d5a

if self.election_elapsed >= self.election_timeout {
self.election_elapsed = 0;
if self.check_quorum {
let m = new_message(INVALID_ID, MessageType::MsgCheckQuorum, Some(self.id));
has_ready = true;
let _ = self.step(m);
}
if self.state == StateRole::Leader && self.lead_transferee.is_some() {
self.abort_leader_transfer()
}
}

@BusyJay
Copy link
Member Author

BusyJay commented May 28, 2020

When last index and term are recorded in MsgTimeoutNow, transferee can check if it still matches its log. If not, it can be dropped directly. Broadcast an empty entry when about to abort transferring is to make either quorum have more logs than transferee or transferee's log more up to date than the MsgTimeoutNow.

@jayzhan211
Copy link
Contributor

jayzhan211 commented May 29, 2020

how to deal with the situation that,
if empty entry that broadcast by leader is received after MsgRequestVote from transferee, then the follower will send MsgRequestVoteResponse to transferee, and transferee will become leader, which is not expected, (after that follower receive broadcast message and update the log but it is too late)

I have an answer,

leader election timeout will send empty entry MsgAppend and receive MsgAppendResponse from followers,
transferee will receive MsgTimeoutNow and send MsgRequestVote and receive MsgRequestVoteResponse,

leader will abort transferee only after receiving MsgAppendResponse, it means the followers has up -to-date logs, otherwise transfer leadership is considered success and transferee will get the follower's MsgRequestVoteResponse

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Recognized misbehavior.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants