Skip to content

Commit 6df2c64

Browse files
pradeepmouliclaude
andcommitted
fix: retry with backoff before wiping the graph on a non-lock-contention open failure
Infigraph::init() correctly avoids wiping the graph when Kuzu's open error is lock contention ("Could not set lock on file"), since that just means another live process (a watcher) already has it open. Any other open error -- including a transient short read from a concurrent writer mid-checkpoint -- fell straight through to wipe_graph with zero retry, on the assumption it must be genuine corruption. This destroyed a real repo's graph tonight: a manual `scip-import` ran concurrently with a background watcher/MCP-server touching the same graph file, producing "IO exception: Cannot read from file... 0 bytes" on the next open -- not a lock message, so it wiped tens of thousands of symbols that were sitting right there on disk moments earlier. Retry with backoff (200ms, 500ms, 1s) before giving up. This self-heals the transient-race case -- including slower writers than a single fixed retry would tolerate, e.g. a large SCIP import still mid-checkpoint -- while still correctly wiping-and-rebuilding graphs that are durably corrupt, since retries against those keep failing too. If a retry attempt comes back as lock contention specifically, stop retrying immediately and report that instead (now unambiguously another live process, not a checkpoint race). Investigated whether a more principled fix exists: the underlying `lbug` (kuzu) crate has no structured error type to match on instead of strings -- open failures cross the FFI boundary as an opaque `cxx::Exception`, so there's no error code to check. Kuzu's own concurrency docs don't cover retry/corruption-detection at all; their documented model is strict single-writer, with concurrent-write access meant to go through one dedicated writer process (the same pattern already used for the Neo4j backend, which has no retry/wipe logic of its own since Neo4j handles concurrency server-side). This fix is a pragmatic safety net given that constraint, not a substitute for routing writes through a single process -- that's a larger, separate architectural change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TAXDJyFdnA4BV1U2fxufdC
1 parent c76e2fb commit 6df2c64

1 file changed

Lines changed: 205 additions & 9 deletions

File tree

  • crates/infigraph-core/src

crates/infigraph-core/src/lib.rs

Lines changed: 205 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,10 @@ enum BackendKind {
9090
}
9191

9292
impl Infigraph {
93+
/// Backoff schedule (ms) for retrying a non-lock-contention graph open
94+
/// failure before concluding it's genuine corruption. See `init()`.
95+
const OPEN_RETRY_BACKOFF_MS: [u64; 3] = [200, 500, 1000];
96+
9397
/// Open a project directory. Creates `.infigraph/` if it doesn't exist.
9498
pub fn open(root: &Path, registry: LanguageRegistry) -> Result<Self> {
9599
Self::open_shared(root, std::sync::Arc::new(registry))
@@ -148,15 +152,54 @@ impl Infigraph {
148152
})
149153
}
150154
Err(first_err) => {
151-
eprintln!(
152-
"[graph] open failed ({first_err}), wiping corrupt graph and rebuilding..."
153-
);
154-
Self::wipe_graph(&self.db_path);
155-
let kb = graph::KuzuBackend::open(&self.db_path).with_context(|| {
156-
format!("graph still unreadable after wipe (was: {first_err})")
157-
})?;
158-
self.backend_kind = BackendKind::Kuzu(kb);
159-
Ok(())
155+
// Some Kuzu IO errors (e.g. a short read while a concurrent
156+
// writer is mid-checkpoint) look identical to genuine
157+
// corruption at open time but resolve themselves once that
158+
// writer finishes. Retry with backoff before concluding the
159+
// graph is unrecoverable -- wiping destroys real data if the
160+
// first failure was just a transient race, and a single
161+
// fixed-delay retry isn't enough for a slower writer (e.g. a
162+
// large SCIP import mid-checkpoint).
163+
let mut last_err = first_err;
164+
let mut recovered = None;
165+
for delay_ms in Self::OPEN_RETRY_BACKOFF_MS {
166+
std::thread::sleep(std::time::Duration::from_millis(delay_ms));
167+
match graph::KuzuBackend::open(&self.db_path) {
168+
Ok(kb) => {
169+
recovered = Some(kb);
170+
break;
171+
}
172+
Err(e) if is_lock_contention_error(&e) => {
173+
// Now unambiguously another live process holding the
174+
// db open, not a transient checkpoint race -- stop
175+
// retrying and report it as such.
176+
return Err(e).with_context(|| {
177+
"graph is locked by another infigraph process (e.g. a \
178+
running `infigraph watch`) -- not corrupted, so it was \
179+
left untouched. Run `infigraph watch-status` or try \
180+
again in a moment."
181+
});
182+
}
183+
Err(e) => last_err = e,
184+
}
185+
}
186+
187+
if let Some(kb) = recovered {
188+
self.backend_kind = BackendKind::Kuzu(kb);
189+
Ok(())
190+
} else {
191+
eprintln!(
192+
"[graph] open failed after {} attempts ({last_err}), wiping \
193+
corrupt graph and rebuilding...",
194+
Self::OPEN_RETRY_BACKOFF_MS.len() + 1
195+
);
196+
Self::wipe_graph(&self.db_path);
197+
let kb = graph::KuzuBackend::open(&self.db_path).with_context(|| {
198+
format!("graph still unreadable after wipe (was: {last_err})")
199+
})?;
200+
self.backend_kind = BackendKind::Kuzu(kb);
201+
Ok(())
202+
}
160203
}
161204
},
162205
}
@@ -590,4 +633,157 @@ mod tests {
590633
"a genuine format/ID mismatch must still be treated as corruption and wiped"
591634
);
592635
}
636+
637+
/// Regression test for a second data-loss bug in the same area: a Kuzu
638+
/// open failure that isn't lock contention (e.g. a short read while a
639+
/// concurrent writer is mid-checkpoint) used to be wiped immediately with
640+
/// no retry, even though the underlying file becomes readable again the
641+
/// instant that writer finishes. `init()` destroyed a real repo's graph
642+
/// this way -- the open failed with "Cannot read from file... 0 bytes",
643+
/// not a lock message, so it fell straight through to `wipe_graph`.
644+
#[test]
645+
fn init_recovers_from_transient_open_failure_without_wiping() {
646+
let dir = tempfile::TempDir::new().unwrap();
647+
let root = dir.path();
648+
let db_path = root.join(".infigraph").join("graph");
649+
650+
// Build a real graph with a marker symbol, then close it.
651+
{
652+
let store = GraphStore::open(&db_path).unwrap();
653+
let conn = store.connection().unwrap();
654+
conn.query(
655+
"CREATE (:Symbol {id: 'marker::survived', name: 'survived', kind: 'function', \
656+
file: 'marker.rs', start_line: 0, end_line: 0, signature_hash: '', \
657+
language: 'rust', visibility: 'public', parent: '', docstring: '', \
658+
complexity: 0, parameters: '', return_type: ''})",
659+
)
660+
.unwrap();
661+
}
662+
let valid_bytes = std::fs::read(&db_path).unwrap();
663+
664+
// Corrupt the file so the first open attempt fails, then heal it
665+
// shortly after -- well within init()'s 300ms retry delay -- to
666+
// simulate a concurrent writer that was mid-checkpoint.
667+
std::fs::write(&db_path, b"not a valid kuzu database file at all").unwrap();
668+
let healer_path = db_path.clone();
669+
let healer = std::thread::spawn(move || {
670+
std::thread::sleep(std::time::Duration::from_millis(50));
671+
std::fs::write(&healer_path, &valid_bytes).unwrap();
672+
});
673+
674+
let registry = LanguageRegistry::new();
675+
let mut ig = Infigraph::open(root, registry).unwrap();
676+
let result = ig.init();
677+
healer.join().unwrap();
678+
679+
assert!(
680+
result.is_ok(),
681+
"init() should recover once the transient failure heals: {result:?}"
682+
);
683+
684+
let backend = ig.backend().unwrap();
685+
let rows = backend
686+
.raw_query("MATCH (s:Symbol) WHERE s.id = 'marker::survived' RETURN s.id")
687+
.unwrap();
688+
assert_eq!(
689+
rows.len(),
690+
1,
691+
"the pre-existing graph must survive a transient failure -- no wipe should occur"
692+
);
693+
}
694+
695+
/// A single fixed-delay retry isn't enough for a slower concurrent writer
696+
/// (e.g. a large SCIP import still mid-checkpoint). Heal at ~600ms --
697+
/// past where a lone 300ms retry would already have given up and wiped,
698+
/// but within OPEN_RETRY_BACKOFF_MS's cumulative 200+500=700ms window --
699+
/// to prove the backoff schedule covers slower recoveries too.
700+
#[test]
701+
fn init_recovers_from_slower_transient_failure_via_backoff() {
702+
let dir = tempfile::TempDir::new().unwrap();
703+
let root = dir.path();
704+
let db_path = root.join(".infigraph").join("graph");
705+
706+
{
707+
let store = GraphStore::open(&db_path).unwrap();
708+
let conn = store.connection().unwrap();
709+
conn.query(
710+
"CREATE (:Symbol {id: 'marker::slow-heal', name: 'slow_heal', kind: 'function', \
711+
file: 'marker.rs', start_line: 0, end_line: 0, signature_hash: '', \
712+
language: 'rust', visibility: 'public', parent: '', docstring: '', \
713+
complexity: 0, parameters: '', return_type: ''})",
714+
)
715+
.unwrap();
716+
}
717+
let valid_bytes = std::fs::read(&db_path).unwrap();
718+
719+
std::fs::write(&db_path, b"not a valid kuzu database file at all").unwrap();
720+
let healer_path = db_path.clone();
721+
let healer = std::thread::spawn(move || {
722+
std::thread::sleep(std::time::Duration::from_millis(600));
723+
std::fs::write(&healer_path, &valid_bytes).unwrap();
724+
});
725+
726+
let registry = LanguageRegistry::new();
727+
let mut ig = Infigraph::open(root, registry).unwrap();
728+
let result = ig.init();
729+
healer.join().unwrap();
730+
731+
assert!(
732+
result.is_ok(),
733+
"init() should recover via the backoff schedule: {result:?}"
734+
);
735+
736+
let backend = ig.backend().unwrap();
737+
let rows = backend
738+
.raw_query("MATCH (s:Symbol) WHERE s.id = 'marker::slow-heal' RETURN s.id")
739+
.unwrap();
740+
assert_eq!(
741+
rows.len(),
742+
1,
743+
"a slower-but-still-transient failure must not be wiped either"
744+
);
745+
}
746+
747+
/// Companion to the test above: if the open failure is *not* transient
748+
/// (the file is durably corrupt, not just briefly unreadable), init()
749+
/// must still recover by wiping and rebuilding rather than looping
750+
/// forever or erroring out permanently.
751+
#[test]
752+
fn init_wipes_and_rebuilds_on_persistent_corruption() {
753+
let dir = tempfile::TempDir::new().unwrap();
754+
let root = dir.path();
755+
let db_path = root.join(".infigraph").join("graph");
756+
757+
{
758+
let store = GraphStore::open(&db_path).unwrap();
759+
let conn = store.connection().unwrap();
760+
conn.query(
761+
"CREATE (:Symbol {id: 'marker::wiped', name: 'wiped', kind: 'function', \
762+
file: 'marker.rs', start_line: 0, end_line: 0, signature_hash: '', \
763+
language: 'rust', visibility: 'public', parent: '', docstring: '', \
764+
complexity: 0, parameters: '', return_type: ''})",
765+
)
766+
.unwrap();
767+
}
768+
// Corrupt permanently -- nothing heals this one.
769+
std::fs::write(&db_path, b"not a valid kuzu database file at all").unwrap();
770+
771+
let registry = LanguageRegistry::new();
772+
let mut ig = Infigraph::open(root, registry).unwrap();
773+
let result = ig.init();
774+
775+
assert!(
776+
result.is_ok(),
777+
"init() must recover from persistent corruption via wipe+rebuild: {result:?}"
778+
);
779+
let backend = ig.backend().unwrap();
780+
let rows = backend
781+
.raw_query("MATCH (s:Symbol) WHERE s.id = 'marker::wiped' RETURN s.id")
782+
.unwrap();
783+
assert_eq!(
784+
rows.len(),
785+
0,
786+
"persistent corruption should have been wiped, not silently ignored"
787+
);
788+
}
593789
}

0 commit comments

Comments
 (0)