Is it safe to clear all entries on `apply_snapshot`

[jorgeantonio21]

Recently, I looking into how raft handles applying snapshots to its raft log stream, and upon reading the logic of apply_snapshot in:

https://github.com/tikv/raft-rs/blob/master/src/storage.rs#L238-L260

I realized that the the raft node does indeed clear all its entries: self.entries.clear().

Is this a safe approach ?

From the docs for self.entries:

// entries[i] has raft log position i+snapshot.get_metadata().index
entries: Vec<Entry>,

So I presume self.entries can indeed have entries beyond the snapshot.take_metadata().index which will be effectively lost. In my own use case, where raft log indices correspond to business logic entries, I am worried my system will indeed lose state.

Should we instead drain self.entries until the snapshot metadata index ? Similar to what compact() does ?

let offset = compact_index - entry.index;
self.entries.drain(..offset as usize);

[BusyJay]

Normally leader will not send a snapshot that is before follower's last index unless there is conflict. In all cases, it's OK to just clear all entries.

[cschanhniem]

The maintainer is right that clear() is safe under the conventional Raft contract, but the reasoning deserves a more precise trace — and there is actually a subtle edge case worth examining.

Why clear() works in the synchronous-commit path:

Under the Raft log matching property (§5.3), a leader will never send a snapshot to a follower whose log already contains committed entries beyond the snapshot index. The handshake is: the leader computes nextIndex from the followers rejection hints (AppendEntries response), and only sends a snapshot when the followers log is so far behind that all entries after the snapshot have been truncated. Since MemStorage::entries is an in-memory cache of the uncompacted log tail (offset by snapshot_metadata.index), calling clear() discards entries that the raft node receiving the snapshot either (a) already has applied to its state machine (and thus subsumed by the snapshot), or (b) never had committed. Entries in category (b) are necessarily uncommitted: if they were committed, the current leader would hold them in its own log via Leader Completeness (§5.4.2), and the snapshot would include them. So clear() is safe.

Where it can break — the async-ready path:

The code comment at line 266 (// TODO: This is not true for async ready.) is the real signal. Under an async storage pipeline, hard_state.commit can lag behind the actual committed entries because the state machine apply is asynchronous. If a snapshot arrives between the time entries are committed in Raft and the time they are applied to the state machine, clear() will discard the committed but not yet applied entries from the cache. Those entries will not be in the snapshot (the snapshot was taken at a lower index), and they will not be re-replicated (the leader considers them already committed). The result is a linearizability violation: the state machine skips entries.

Concrete fix for the async path:

Before self.entries.clear(), flush committed entries up to self.raft_state.hard_state.commit into the snapshot. Alternatively, in async mode, defer the snapshot application until committed entries are flushed to the state machine. The simplest guard:

// Before clear, verify we arent discarding committed entries.
let applied = self.raft_state.hard_state.commit;
if applied > index {
    // Some committed entries are beyond the snapshot. In async mode,
    // these might not yet be applied. Refuse the snapshot or snapshot
    // at the applied index instead.
    return Err(Error::Store(StorageError::SnapshotOutOfDate));
}

This preserves the invariant that the snapshot always covers at least the commit index, which is the same invariant the Raft paper assumes in §5.3 ("The leader uses a new snapshot to install its own state on a slow follower").

[BusyJay]

You can ask AI to read the doc in https://docs.rs/raft/latest/raft/#processing-the-ready-state and don't overlook comments from other functions. Also use Opus 4.8 max is possible.