Skip to content

Commit 109fe37

Browse files
author
Paul C
committed
wolfdisk v2.11.6: followers self-heal divergent state instead of getting stuck (wabil)
A bulk delete of 250k files followed by a restart left the cluster wedged: the leader had 226 files, followers had 262k, yet all reported "v0 — in sync" and deletions never propagated. Three compounding causes, all fixed: 1. The replication index_version + changelog were in-memory only, so a restart reset the leader to v0 while the on-disk file index persisted. Every node then falsely agreed on v0 and never reconciled. Now persisted to replication_state.json (atomic temp+rename) and reloaded on startup. 2. Follower sync was ADDITIVE — it inserted the leader's entries and removed only changelog-listed deletions, so a follower holding stale extras never dropped them. New is_full flag on SyncResponse marks a full index; the follower then reconciles AUTHORITATIVELY, deleting any local path the leader no longer has (batched, lock-friendly; guarded so an empty/failed-load leader index can never wipe a follower). This is the self-heal: a stuck follower now converges on its own within a resync cycle — no manual re-seed. 3. changes_since() returned Some((empty,empty)) for an empty changelog while behind ("nothing changed"); now returns None to force a full sync. WIRE CHANGE (is_full appended to SyncResponseMsg, bincode positional): upgrade the LEADER first, then followers — or all nodes together. Tests cover the restart trap, persistence round-trip, and the authoritative delete selection. Independent code review: no critical findings; data-loss/deadlock paths clear.
1 parent 47fbea5 commit 109fe37

5 files changed

Lines changed: 268 additions & 8 deletions

File tree

wolfdisk/Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

wolfdisk/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "wolfdisk"
3-
version = "2.11.5"
3+
version = "2.11.6"
44
edition = "2021"
55
authors = ["Wolf Software Systems Ltd"]
66
description = "Distributed file system with replicated and shared storage modes"

wolfdisk/src/cluster/state.rs

Lines changed: 152 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,18 @@ pub struct PeerInfo {
3535
pub last_seen: Instant,
3636
}
3737

38+
/// On-disk filename for the persisted replication version + changelog,
39+
/// stored alongside the file index.
40+
const REPL_STATE_FILENAME: &str = "replication_state.json";
41+
42+
/// Serializable snapshot of the replication state that must survive a restart:
43+
/// the monotonic index version and the recent changelog used for delta sync.
44+
#[derive(serde::Serialize, serde::Deserialize)]
45+
struct PersistedReplicationState {
46+
version: u64,
47+
changelog: Vec<(u64, std::path::PathBuf, bool)>,
48+
}
49+
3850
/// Cluster manager - handles leader election and state
3951
pub struct ClusterManager {
4052
config: Config,
@@ -184,13 +196,30 @@ impl ClusterManager {
184196
&self,
185197
from_version: u64,
186198
) -> Option<(Vec<std::path::PathBuf>, Vec<std::path::PathBuf>)> {
199+
let current = self.index_version();
200+
201+
// Already at (or somehow beyond) our version → genuinely nothing to send.
202+
if from_version >= current {
203+
return Some((Vec::new(), Vec::new()));
204+
}
205+
187206
let log = self.changelog.read().unwrap();
188207

189-
// Check if changelog goes back far enough
190-
if let Some((oldest_version, _, _)) = log.front() {
191-
if from_version < *oldest_version {
192-
// Changelog truncated, need full sync
193-
return None;
208+
// We're behind (from_version < current), so the changelog MUST contain
209+
// every change after from_version to answer with a delta. If it's empty,
210+
// or its oldest entry is newer than from_version+1, there's a gap we
211+
// can't fill → force a full sync. The empty case is the critical one:
212+
// after a restart the in-memory changelog is empty while `current` was
213+
// restored from disk, and the old code returned Some((empty, empty))
214+
// here — telling the follower "nothing changed" when in fact it must
215+
// re-reconcile against the leader's full index (wabil 2026-06-29).
216+
match log.front() {
217+
None => return None,
218+
Some((oldest_version, _, _)) => {
219+
if from_version.saturating_add(1) < *oldest_version {
220+
// Changelog truncated, need full sync
221+
return None;
222+
}
194223
}
195224
}
196225

@@ -222,6 +251,60 @@ impl ClusterManager {
222251
*self.index_version.write().unwrap() = version;
223252
}
224253

254+
/// Persist the replication index version + changelog next to the file index,
255+
/// written atomically (temp + rename). Without this the version is in-memory
256+
/// only, so a restart reset the leader to v0 — every node then falsely agreed
257+
/// "v0, in sync" while their file sets differed by a quarter-million entries,
258+
/// and deletions never propagated (wabil 2026-06-29).
259+
pub fn save_replication_state(&self, index_dir: &std::path::Path) -> std::io::Result<()> {
260+
// Snapshot version + changelog CONSISTENTLY: hold the version lock across
261+
// the changelog read, mirroring record_change's lock order (version then
262+
// changelog). Otherwise a concurrent record_change could bump the version
263+
// and append its entry between the two reads, persisting a version one
264+
// behind its own changelog.
265+
let (version, changelog) = {
266+
let v = self.index_version.read().unwrap();
267+
let log: Vec<(u64, std::path::PathBuf, bool)> =
268+
self.changelog.read().unwrap().iter().cloned().collect();
269+
(*v, log)
270+
};
271+
let state = PersistedReplicationState { version, changelog };
272+
let bytes = serde_json::to_vec(&state).map_err(std::io::Error::other)?;
273+
std::fs::create_dir_all(index_dir)?;
274+
let path = index_dir.join(REPL_STATE_FILENAME);
275+
let tmp = index_dir.join(format!("{}.tmp", REPL_STATE_FILENAME));
276+
std::fs::write(&tmp, &bytes)?;
277+
std::fs::rename(&tmp, &path)?;
278+
Ok(())
279+
}
280+
281+
/// Restore the replication version + changelog on startup. A missing or
282+
/// corrupt file simply leaves us at v0 (the prior behaviour) — which now
283+
/// self-heals via the authoritative full sync regardless.
284+
pub fn load_replication_state(&self, index_dir: &std::path::Path) {
285+
let path = index_dir.join(REPL_STATE_FILENAME);
286+
let bytes = match std::fs::read(&path) {
287+
Ok(b) => b,
288+
Err(_) => return,
289+
};
290+
let state: PersistedReplicationState = match serde_json::from_slice(&bytes) {
291+
Ok(s) => s,
292+
Err(e) => {
293+
warn!("Ignoring corrupt replication state ({}); starting at v0", e);
294+
return;
295+
}
296+
};
297+
*self.index_version.write().unwrap() = state.version;
298+
let mut log = self.changelog.write().unwrap();
299+
log.clear();
300+
log.extend(state.changelog);
301+
info!(
302+
"Restored replication state: version {}, {} changelog entries",
303+
state.version,
304+
log.len()
305+
);
306+
}
307+
225308
/// Mark initial sync as complete (allows election monitor to promote to leader)
226309
pub fn set_sync_complete(&self) {
227310
*self.initial_sync_complete.write().unwrap() = true;
@@ -571,3 +654,67 @@ impl ClusterManager {
571654
}
572655
}
573656
}
657+
658+
#[cfg(test)]
659+
mod replication_state_tests {
660+
use super::*;
661+
use std::path::PathBuf;
662+
663+
fn mgr() -> ClusterManager {
664+
ClusterManager::new(Config::default())
665+
}
666+
667+
#[test]
668+
fn changes_since_at_version_is_empty_not_none() {
669+
let m = mgr();
670+
// Genuinely caught up (from == current == 0) → "no changes", NOT a full sync.
671+
assert_eq!(m.changes_since(0), Some((Vec::new(), Vec::new())));
672+
m.increment_index_version(PathBuf::from("/a"));
673+
assert_eq!(m.changes_since(1), Some((Vec::new(), Vec::new())));
674+
}
675+
676+
#[test]
677+
fn changes_since_reports_changes_and_deletions() {
678+
let m = mgr();
679+
m.increment_index_version(PathBuf::from("/a")); // v1 change
680+
m.record_deletion(PathBuf::from("/b")); // v2 delete
681+
let (changed, deleted) = m.changes_since(0).expect("covered by changelog");
682+
assert_eq!(changed, vec![PathBuf::from("/a")]);
683+
assert_eq!(deleted, vec![PathBuf::from("/b")]);
684+
}
685+
686+
#[test]
687+
fn behind_with_empty_changelog_forces_full_sync() {
688+
// The restart trap: version was restored from disk (so current > 0) but
689+
// the in-memory changelog is empty. The old code returned Some((empty,
690+
// empty)) here — telling the follower "nothing changed" when it must in
691+
// fact reconcile against the full index. It MUST be None (full sync).
692+
let m = mgr();
693+
m.set_index_version(262_700);
694+
assert_eq!(m.changes_since(0), None);
695+
assert_eq!(m.changes_since(100), None);
696+
}
697+
698+
#[test]
699+
fn replication_state_survives_save_load() {
700+
let dir = std::env::temp_dir().join(format!("wolfdisk_rs_test_{}", std::process::id()));
701+
std::fs::create_dir_all(&dir).unwrap();
702+
703+
let m1 = mgr();
704+
m1.increment_index_version(PathBuf::from("/x")); // v1
705+
m1.increment_index_version(PathBuf::from("/y")); // v2
706+
m1.record_deletion(PathBuf::from("/x")); // v3
707+
m1.save_replication_state(&dir).unwrap();
708+
709+
let m2 = mgr();
710+
assert_eq!(m2.index_version(), 0);
711+
m2.load_replication_state(&dir);
712+
assert_eq!(m2.index_version(), 3);
713+
// Changelog restored → delta sync still works across the restart.
714+
let (changed, deleted) = m2.changes_since(1).expect("changelog restored");
715+
assert!(changed.contains(&PathBuf::from("/y")));
716+
assert!(deleted.contains(&PathBuf::from("/x")));
717+
718+
std::fs::remove_dir_all(&dir).ok();
719+
}
720+
}

wolfdisk/src/main.rs

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,20 @@ enum Commands {
6969
},
7070
}
7171

72+
/// Paths the follower must delete on a full (authoritative) sync: every local
73+
/// path the leader's complete index no longer contains. This is the
74+
/// data-deleting decision of the self-heal, so it's a pure function with a
75+
/// direct regression test (see tests at the bottom of this file).
76+
fn full_sync_stale_paths<'a>(
77+
leader_paths: &std::collections::HashSet<std::path::PathBuf>,
78+
local_paths: impl Iterator<Item = &'a std::path::PathBuf>,
79+
) -> Vec<std::path::PathBuf> {
80+
local_paths
81+
.filter(|p| !leader_paths.contains(*p))
82+
.cloned()
83+
.collect()
84+
}
85+
7286
fn main() {
7387
let cli = Cli::parse();
7488

@@ -124,6 +138,12 @@ fn main() {
124138
));
125139
let file_index_for_handler = file_index.clone();
126140

141+
// Restore the replication version + changelog from disk so a restart
142+
// doesn't reset us to v0 — which made every node falsely agree "v0, in
143+
// sync" while their file sets diverged, and forced a heavy full
144+
// reconcile on every restart (wabil 2026-06-29).
145+
cluster.load_replication_state(&config.index_dir());
146+
127147
// Create chunk store for replication (shared with WolfDiskFS). When an SSD
128148
// cache tier is configured it sits in front of the bulk data_dir.
129149
let chunk_store = std::sync::Arc::new(
@@ -1307,6 +1327,7 @@ fn main() {
13071327
current_version,
13081328
entries: Vec::new(),
13091329
deleted_paths: Vec::new(),
1330+
is_full: false,
13101331
}))
13111332
} else if sync_req.from_version > 0 {
13121333
// Try delta sync — only send files that changed since their version
@@ -1364,6 +1385,7 @@ fn main() {
13641385
current_version,
13651386
entries,
13661387
deleted_paths: deleted_strs,
1388+
is_full: false,
13671389
}))
13681390
}
13691391
None => {
@@ -1411,6 +1433,8 @@ fn main() {
14111433
current_version,
14121434
entries,
14131435
deleted_paths: Vec::new(),
1436+
// Full index → follower reconciles authoritatively.
1437+
is_full: true,
14141438
}))
14151439
}
14161440
}
@@ -1465,6 +1489,8 @@ fn main() {
14651489
current_version,
14661490
entries,
14671491
deleted_paths: Vec::new(),
1492+
// Full index (initial / from_version 0) → authoritative.
1493+
is_full: true,
14681494
}))
14691495
}
14701496
}
@@ -2292,6 +2318,41 @@ fn main() {
22922318
}
22932319
}
22942320

2321+
// Authoritative full sync: the leader sent its COMPLETE index, so
2322+
// any local entry it did NOT send is stale and must go. This is what
2323+
// makes a follower actually CONVERGE after it falls out of changelog
2324+
// range or misses a bulk delete — the old code only ever added/updated
2325+
// and applied changelog deletions, so a follower holding 250k stale
2326+
// files re-received the leader's small index every cycle and kept the
2327+
// extras forever (wabil 2026-06-29). Snapshot local paths under a
2328+
// brief read lock, diff against the leader's set off-lock, then delete
2329+
// in small batches so FUSE ops aren't starved on a huge index.
2330+
// Guard: never authoritatively wipe to match an EMPTY leader index.
2331+
// A leader whose index failed to load reads as 0 entries; honouring
2332+
// that would nuke every follower. A genuine "delete everything" still
2333+
// propagates as normal changelog deletions, not a full-sync wipe.
2334+
if response.is_full && !response.entries.is_empty() {
2335+
let keep: std::collections::HashSet<std::path::PathBuf> = response
2336+
.entries
2337+
.iter()
2338+
.map(|e| std::path::PathBuf::from(&e.path))
2339+
.collect();
2340+
let stale: Vec<std::path::PathBuf> = {
2341+
let index = resync_file_index.read().unwrap();
2342+
full_sync_stale_paths(&keep, index.paths())
2343+
};
2344+
for del_batch in stale.chunks(50) {
2345+
let mut index = resync_file_index.write().unwrap();
2346+
let mut inode_tbl = resync_inode_table.write().unwrap();
2347+
for del_path in del_batch {
2348+
if index.remove(del_path).is_some() {
2349+
inode_tbl.remove_path(del_path);
2350+
removed += 1;
2351+
}
2352+
}
2353+
}
2354+
}
2355+
22952356
// Update our version to the leader's version, and publish it
22962357
// to the cluster so the health UI reflects real progress.
22972358
// Previously this stayed in this thread's local variable, so a
@@ -2383,11 +2444,17 @@ fn main() {
23832444
// the index directly without setting the FUSE dirty flag.
23842445
let persist_file_index = file_index.clone();
23852446
let persist_index_dir = config.index_dir().to_path_buf();
2447+
let persist_cluster = cluster.clone();
23862448
std::thread::spawn(move || loop {
23872449
std::thread::sleep(std::time::Duration::from_secs(5));
23882450
if std::sync::Arc::strong_count(&persist_file_index) <= 1 {
23892451
break;
23902452
}
2453+
// Persist the replication version + changelog so a restart resumes
2454+
// at the right version instead of resetting to v0 (wabil 2026-06-29).
2455+
if let Err(e) = persist_cluster.save_replication_state(&persist_index_dir) {
2456+
tracing::warn!("Failed to persist replication state: {}", e);
2457+
}
23912458
// Serialize under the read lock (CPU only), then write to disk
23922459
// AFTER releasing it. The old code held the read lock across the
23932460
// whole disk write of a 250k-entry index every 5s, blocking every
@@ -2689,3 +2756,30 @@ fn main() {
26892756
}
26902757
}
26912758
}
2759+
2760+
#[cfg(test)]
2761+
mod full_sync_tests {
2762+
use super::full_sync_stale_paths;
2763+
use std::collections::HashSet;
2764+
use std::path::PathBuf;
2765+
2766+
fn p(s: &str) -> PathBuf { PathBuf::from(s) }
2767+
2768+
#[test]
2769+
fn deletes_only_paths_absent_from_leader() {
2770+
// Leader's authoritative set (post bulk-delete): two files.
2771+
let leader: HashSet<PathBuf> = [p("/keep/a"), p("/keep/b")].into_iter().collect();
2772+
// Follower still holds the leader's two PLUS three stale extras.
2773+
let local = vec![p("/keep/a"), p("/keep/b"), p("/old/1"), p("/old/2"), p("/old/3")];
2774+
let mut stale = full_sync_stale_paths(&leader, local.iter());
2775+
stale.sort();
2776+
assert_eq!(stale, vec![p("/old/1"), p("/old/2"), p("/old/3")]);
2777+
}
2778+
2779+
#[test]
2780+
fn keeps_everything_when_follower_matches_leader() {
2781+
let leader: HashSet<PathBuf> = [p("/a"), p("/b")].into_iter().collect();
2782+
let local = vec![p("/a"), p("/b")];
2783+
assert!(full_sync_stale_paths(&leader, local.iter()).is_empty());
2784+
}
2785+
}

wolfdisk/src/network/protocol.rs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,21 @@ pub struct SyncResponseMsg {
197197
pub entries: Vec<IndexEntryMsg>,
198198
/// Paths deleted since the requested version (for delta sync)
199199
pub deleted_paths: Vec<String>,
200+
/// True when `entries` is the leader's COMPLETE index (a full sync), not a
201+
/// delta. The follower then reconciles authoritatively — it removes any
202+
/// local path absent from `entries`, instead of merely merging. Without
203+
/// this, a follower that fell out of changelog range (e.g. a bulk delete of
204+
/// 250k files, or a leader that restarted to v0) re-received the leader's
205+
/// small index every cycle but never dropped its stale extras, so it never
206+
/// converged (wabil 2026-06-29). Appended LAST (bincode is positional — see
207+
/// encode_message's WIRE-COMPAT note). Rolling-upgrade order for THIS field:
208+
/// **leader first, then followers**. A new follower cannot decode an old
209+
/// leader's (shorter) response and would loop on sync errors; an old follower
210+
/// harmlessly ignores this trailing field on a new leader's response (it just
211+
/// never runs the authoritative reconcile). Upgrading all nodes together is
212+
/// also safe.
213+
#[serde(default)] // no-op for the bincode transport (positional); kept for JSON/debug only
214+
pub is_full: bool,
200215
}
201216

202217
/// Index entry in sync response
@@ -400,7 +415,11 @@ pub struct CreateSymlinkMsg {
400415
/// JSON). Adding a field to a message struct is a breaking wire change: mixed
401416
/// versions during a rolling upgrade WILL mis-decode, and for a `Vec<…>` payload
402417
/// (e.g. SyncResponse.entries) a new→old decode corrupts every entry after the
403-
/// first. Upgrade order: all nodes together, or followers before the leader.
418+
/// first. Upgrade order is DIRECTION-DEPENDENT: upgrade the side that DECODES the
419+
/// changed message last. For a field added to a response (leader→follower, e.g.
420+
/// SyncResponseMsg.is_full) upgrade the LEADER first; for a field added to a
421+
/// request (follower→leader) upgrade FOLLOWERS first. Upgrading all nodes
422+
/// together is always safe.
404423
pub fn encode_message(msg: &Message) -> Result<Vec<u8>, bincode::Error> {
405424
let serialized = bincode::serialize(msg)?;
406425
Ok(lz4_flex::compress_prepend_size(&serialized))

0 commit comments

Comments
 (0)