Skip to content

Commit 21684bb

Browse files
committed
Fix: potential inconsistency when installing snapshot
The conflicting logs that are before `snapshot_meta.last_log_id` should be deleted before installing a snapshot. Otherwise there is chance the snapshot is installed but conflicting logs are left in the store, when a node crashes.
1 parent 8d9b33a commit 21684bb

2 files changed

Lines changed: 29 additions & 2 deletions

File tree

openraft/src/core/append_entries.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ impl<D: AppData, R: AppDataResponse, N: RaftNetwork<D>, S: RaftStorage<D, R>> Ra
110110
}
111111

112112
#[tracing::instrument(level = "debug", skip(self))]
113-
async fn delete_conflict_logs_since(&mut self, start: LogId) -> Result<(), StorageError> {
113+
pub(crate) async fn delete_conflict_logs_since(&mut self, start: LogId) -> Result<(), StorageError> {
114114
// TODO(xp): add a StorageAdapter to provide auxiliary APIs.
115115
// e.g.:
116116
// - extract and manage membership config.

openraft/src/core/install_snapshot.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use crate::AppData;
1616
use crate::AppDataResponse;
1717
use crate::ErrorSubject;
1818
use crate::ErrorVerb;
19+
use crate::LogIdOptionExt;
1920
use crate::MessageSummary;
2021
use crate::RaftNetwork;
2122
use crate::RaftStorage;
@@ -237,6 +238,32 @@ impl<D: AppData, R: AppDataResponse, N: RaftNetwork<D>, S: RaftStorage<D, R>> Ra
237238
return Ok(());
238239
}
239240

241+
if let Some(last) = req.meta.last_log_id {
242+
let idx = last.index;
243+
let matches = {
244+
let logs = self.storage.try_get_log_entries(idx..=idx).await?;
245+
if let Some(ent) = logs.first() {
246+
Some(ent.log_id) == req.meta.last_log_id
247+
} else {
248+
// no log entry, consider it unmatched.
249+
false
250+
}
251+
};
252+
253+
// The log entry at snapshot_meta.last_log_id.index conflicts with the leaders'
254+
// We have to delete all conflicting logs before installing snapshot.
255+
// See: [snapshot-replication](https://datafuselabs.github.io/openraft/replication.html#snapshot-replication)
256+
if !matches {
257+
// Delete all non-committed log entries.
258+
// It is safe:
259+
let x = StorageHelper::new(&self.storage).get_log_id(self.last_applied.next_index()).await;
260+
if let Ok(log_id) = x {
261+
self.delete_conflict_logs_since(log_id).await?;
262+
}
263+
// else: no next log id, ignore
264+
}
265+
}
266+
240267
let changes = self.storage.install_snapshot(&req.meta, snapshot).await?;
241268

242269
tracing::info!("update after install-snapshot: {:?}", changes);
@@ -247,7 +274,7 @@ impl<D: AppData, R: AppDataResponse, N: RaftNetwork<D>, S: RaftStorage<D, R>> Ra
247274

248275
let last_applied = changes.last_applied;
249276

250-
// Applied logs are not needed.
277+
// Applied logs are not needed. Purge them or there may be a hole in the log.
251278
if let Some(last) = &last_applied {
252279
purge_applied_logs(self.storage.clone(), last, self.config.max_applied_log_to_keep).await?;
253280
}

0 commit comments

Comments
 (0)