Skip to content

Commit

Permalink
Fix: #424 wrong range when searching for membership entries: `[end-st…
Browse files Browse the repository at this point in the history
…ep, end)`.

The iterating range searching for membership log entries should be
`[end-step, end)`, not `[start, end)`.
With this bug it will return duplicated membership entries.

- Bug: #424
  • Loading branch information
drmingdrmer committed Jul 3, 2022
1 parent 1bc3ded commit 30058c0
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
3 changes: 2 additions & 1 deletion openraft/src/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ where
let step = 64;

while start < end {
let entries = self.try_get_log_entries(start..end).await?;
let step_start = std::cmp::max(start, end.saturating_sub(step));
let entries = self.try_get_log_entries(step_start..end).await?;

for ent in entries.iter().rev() {
if let EntryPayload::Membership(ref mem) = ent.payload {
Expand Down
43 changes: 43 additions & 0 deletions openraft/src/testing/suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ where
pub fn test_store(builder: &B) -> anyhow::Result<()> {
run_fut(Suite::last_membership_in_log_initial(builder))?;
run_fut(Suite::last_membership_in_log(builder))?;
run_fut(Suite::last_membership_in_log_multi_step(builder))?;
run_fut(Suite::get_membership_initial(builder))?;
run_fut(Suite::get_membership_from_log_and_sm(builder))?;
run_fut(Suite::get_initial_state_without_init(builder))?;
Expand Down Expand Up @@ -174,6 +175,44 @@ where
Ok(())
}

pub async fn last_membership_in_log_multi_step(builder: &B) -> anyhow::Result<()> {
let store = builder.build().await;

tracing::info!("--- find membership log entry backwards, multiple steps");
{
store
.append_to_log(&[
//
&Entry {
log_id: log_id(1, 1),
payload: EntryPayload::Membership(Membership::new_single(btreeset! {1,2,3})),
},
&Entry {
log_id: log_id(1, 2),
payload: EntryPayload::Membership(Membership::new_single(btreeset! {3,4,5})),
},
])
.await?;

for i in 3..100 {
store.append_to_log(&[&blank(1, i)]).await?;
}

store
.append_to_log(&[&Entry {
log_id: log_id(1, 100),
payload: EntryPayload::Membership(Membership::new_single(btreeset! {5,6,7})),
}])
.await?;

let mem = store.last_membership_in_log(0).await?;
assert!(mem.is_some());
assert_eq!(Membership::new_single(btreeset! {5,6,7}), mem.unwrap().membership,);
}

Ok(())
}

pub async fn get_membership_initial(builder: &B) -> anyhow::Result<()> {
let store = builder.build().await;

Expand Down Expand Up @@ -1467,6 +1506,10 @@ where
}
}

fn log_id(term: u64, index: u64) -> LogId {
LogId { term, index }
}

/// Create a blank log entry for test
fn blank<D: AppData>(term: u64, index: u64) -> Entry<D> {
Entry {
Expand Down

0 comments on commit 30058c0

Please sign in to comment.