Skip to content

Commit

Permalink
Improve: loosen validity check with RaftState.snapshot_last_log_id()
Browse files Browse the repository at this point in the history
A application may not persist snapshot. And when it restarted, the
last-purged-log-id is not `None` but `snapshot_last_log_id()` is None.
This is a valid state and should not emit error.
  • Loading branch information
drmingdrmer committed Mar 10, 2023
1 parent 0f80be9 commit 664635e
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 1 deletion.
1 change: 1 addition & 0 deletions openraft/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ mod runtime;
mod try_as_ref;

#[cfg(test)] mod raft_state_test;
#[cfg(test)] mod rust_state_validate_test;

pub use anyerror;
pub use anyerror::AnyError;
Expand Down
9 changes: 8 additions & 1 deletion openraft/src/raft_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,14 @@ where
}

less_equal!(self.last_purged_log_id(), self.purge_upto());
less_equal!(self.purge_upto(), self.snapshot_last_log_id());
if self.snapshot_last_log_id().is_none() {
// There is no snapshot, it is possible the application does not store snapshot, and
// just restarted. it is just ok.
// In such a case, we assert the monotonic relation without snapshot-last-log-id
less_equal!(self.purge_upto(), self.committed());
} else {
less_equal!(self.purge_upto(), self.snapshot_last_log_id());
}
less_equal!(self.snapshot_last_log_id(), self.committed());
less_equal!(self.committed(), self.last_log_id());

Expand Down
32 changes: 32 additions & 0 deletions openraft/src/rust_state_validate_test.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
use crate::engine::LogIdList;
use crate::validate::Validate;
use crate::CommittedLeaderId;
use crate::LogId;
use crate::RaftState;
use crate::SnapshotMeta;

fn log_id(term: u64, index: u64) -> LogId<u64> {
LogId::<u64> {
leader_id: CommittedLeaderId::new(term, 0),
index,
}
}

#[test]
fn test_raft_state_validate_snapshot_is_none() -> anyhow::Result<()> {
// Some app does not persist snapshot, when restarted, purged is not None but snapshot_last_log_id
// is None. This is a valid state and should not emit error.

let rs = RaftState::<u64, ()> {
log_ids: LogIdList::new(vec![log_id(1, 1), log_id(3, 4)]),
purged_next: 2,
purge_upto: Some(log_id(1, 1)),
committed: Some(log_id(1, 1)),
snapshot_meta: SnapshotMeta::default(),
..Default::default()
};

assert!(rs.validate().is_ok());

Ok(())
}

0 comments on commit 664635e

Please sign in to comment.