Skip to content

Commit

Permalink
Fix: save leader_id if a higher term is seen when handling append-ent…
Browse files Browse the repository at this point in the history
…ries RPC

Problem:

A follower saves hard state `(term=msg.term, voted_for=None)`
when a `msg.term > local.term` when handling append-entries RPC.

This is quite enough to be correct but not perfect. Correct because:

- In one term, only an established leader will send append-entries;

- Thus, there is a quorum voted for this leader;

- Thus, no matter what `voted_for` is saved, it is still correct. E.g.
  when handling append-entries, a follower node could save hard state
  `(term=msg.term, voted_for=Some(ANY_VALUE))`.

The problem is that a follower already knows the legal leader for a term
but still does not save it. This leads to an unstable cluster state: The
test sometimes fails.

Solution:

A follower always save hard state with the id of a known legal leader.
  • Loading branch information
drmingdrmer committed Jan 10, 2022
1 parent a0d7829 commit 8651625
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion openraft/src/core/append_entries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ impl<D: AppData, R: AppDataResponse, N: RaftNetwork<D>, S: RaftStorage<D, R>> Ra
let mut report_metrics = false;

if msg.term > self.current_term {
self.update_current_term(msg.term, None);
self.update_current_term(msg.term, Some(msg.leader_id));
self.save_hard_state().await?;
report_metrics = true;
}
Expand Down
2 changes: 1 addition & 1 deletion openraft/src/core/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ impl<D: AppData, R: AppDataResponse, N: RaftNetwork<D>, S: RaftStorage<D, R>> Ra
}

/// Encapsulate the process of updating the current term, as updating the `voted_for` state must also be updated.
#[tracing::instrument(level = "trace", skip(self))]
#[tracing::instrument(level = "debug", skip(self))]
fn update_current_term(&mut self, new_term: u64, voted_for: Option<NodeId>) {
if new_term > self.current_term {
self.current_term = new_term;
Expand Down

0 comments on commit 8651625

Please # to comment.