Skip to content

Commit ae2bed1

Browse files
anvansterclaude
andauthored
fix(storage): wait out a busy graph DB instead of discarding it (#27)
* fix(storage): wait out a busy graph DB instead of discarding it A session that started while another process had graph.db open (any sibling session or daemon mid-load or mid-persist) treated the lock error as corruption: it set the healthy DB aside as graph.db.corrupt and redirected every project to a fresh, empty generation. On Linux the stale-LOCK cleanup could also delete a live holder's LOCK file, because its flock probe cannot see RocksDB's fcntl lock, letting two processes open the same DB. - RocksDBBackend::open reports lock contention as GraphError::Locked. - open_waiting_for_lock replaces open_with_stale_lock_recovery: it retries with backoff for a bounded time and never removes LOCK. The OS drops a lock when its holder exits, so a leftover LOCK file never blocked an open; removing one only ever risked a second writer. fs2 is no longer needed. - Every open of the shared DB goes through memory::open_shared_graph_db (10s wait), including vector claim/save/load and cross-project impact. - open_persistent_graph redirects only on a real load failure, never on Locked. - Saved file hashes are ignored when the project's graph loads empty. After any redirect they made the indexer skip every unchanged file, leaving the project with no symbols until a file changed. Verified end to end with the release binary (live holder for 4s and 20s, SIGKILLed holder, damaged CURRENT) and by new crate and integration tests on macOS, SLES 15-SP4 and Windows. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017rVbt7rENTwXkdHt3Bpgb5 * no-mistakes(review): Disarm load sentinel during lock waits; share cross-project handle --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 354889f commit ae2bed1

12 files changed

Lines changed: 448 additions & 212 deletions

File tree

‎Cargo.lock‎

Lines changed: 0 additions & 11 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ thiserror = "1.0"
7474
rocksdb = "0.22"
7575
# Pin libz-sys to 1.1.25 — v1.1.26 has broken vendored zlib build on macOS/Windows
7676
libz-sys = "=1.1.25"
77-
# Cross-platform advisory file locks (Linux fcntl / Windows LockFileEx)
78-
fs2 = "0.4"
7977
log = "0.4"
8078
uuid = { version = "1.0", features = ["v4", "serde"] }
8179

‎crates/codegraph-server/src/ai_query/engine.rs‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -971,15 +971,13 @@ impl QueryEngine {
971971
/// rebuild that follows are loadable if it is interrupted. Returns whether
972972
/// the project was taken - see [`claim_project`].
973973
fn claim_vector_set(slug: &str, stamp: &str) -> std::result::Result<bool, String> {
974-
use codegraph::RocksDBBackend;
975-
976974
let db_path = crate::memory::shared_graph_db_path().map_err(|e| format!("{e}"))?;
977975
if let Some(parent) = db_path.parent() {
978976
std::fs::create_dir_all(parent)
979977
.map_err(|e| format!("Failed to create ~/.codegraph: {e}"))?;
980978
}
981-
let rocks =
982-
RocksDBBackend::open(&db_path).map_err(|e| format!("Failed to open graph.db: {e}"))?;
979+
let rocks = crate::memory::open_shared_graph_db(&db_path)
980+
.map_err(|e| format!("Failed to open graph.db: {e}"))?;
983981
let mut namespaced = NamespacedBackend::new(Box::new(rocks), slug);
984982

985983
claim_project(&mut namespaced, slug, stamp)
@@ -1013,8 +1011,6 @@ impl QueryEngine {
10131011
complete: bool,
10141012
stamp: &str,
10151013
) -> std::result::Result<bool, String> {
1016-
use codegraph::RocksDBBackend;
1017-
10181014
if vecs.is_empty() {
10191015
return Ok(false);
10201016
}
@@ -1025,8 +1021,8 @@ impl QueryEngine {
10251021
.map_err(|e| format!("Failed to create ~/.codegraph: {e}"))?;
10261022
}
10271023

1028-
let rocks =
1029-
RocksDBBackend::open(&db_path).map_err(|e| format!("Failed to open graph.db: {e}"))?;
1024+
let rocks = crate::memory::open_shared_graph_db(&db_path)
1025+
.map_err(|e| format!("Failed to open graph.db: {e}"))?;
10301026
let mut namespaced = NamespacedBackend::new(Box::new(rocks), slug);
10311027

10321028
if !store_vectors(&mut namespaced, vecs, stamp)? {
@@ -1058,8 +1054,6 @@ impl QueryEngine {
10581054
/// engine is configured to build; a set built from other text is left
10591055
/// untouched for whoever wrote it. Returns the number of vectors loaded.
10601056
pub async fn load_symbol_vectors(&self, slug: &str) -> VectorLoad {
1061-
use codegraph::RocksDBBackend;
1062-
10631057
let Some(stamp) = self.embed_stamp().await else {
10641058
tracing::warn!("[QueryEngine] No vector engine attached - cannot load symbol vectors");
10651059
return VectorLoad::Unreadable;
@@ -1077,7 +1071,7 @@ impl QueryEngine {
10771071
return VectorLoad::Absent;
10781072
}
10791073

1080-
let rocks = match RocksDBBackend::open(&db_path) {
1074+
let rocks = match crate::memory::open_shared_graph_db(&db_path) {
10811075
Ok(r) => r,
10821076
Err(e) => {
10831077
tracing::warn!("[QueryEngine] Failed to open graph.db for vectors: {}", e);

‎crates/codegraph-server/src/backend.rs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -515,7 +515,7 @@ impl CodeGraphBackend {
515515
workspace: &std::path::Path,
516516
graph: &codegraph::CodeGraph,
517517
) -> std::result::Result<(), String> {
518-
use codegraph::{NamespacedBackend, RocksDBBackend, StorageBackend};
518+
use codegraph::{NamespacedBackend, StorageBackend};
519519

520520
// Ephemeral workspaces (test harness tempdirs) skip
521521
// persistence to the shared graph.db entirely. The in-memory
@@ -532,7 +532,7 @@ impl CodeGraphBackend {
532532
.map_err(|e| format!("Failed to create ~/.codegraph: {e}"))?;
533533
}
534534

535-
let mut rocks = RocksDBBackend::open_with_stale_lock_recovery(&db_path)
535+
let mut rocks = crate::memory::open_shared_graph_db(&db_path)
536536
.map_err(|e| format!("Failed to open graph.db: {e}"))?;
537537

538538
let registry_value = serde_json::json!({
@@ -1206,23 +1206,29 @@ impl LanguageServer for CodeGraphBackend {
12061206
}
12071207
Err(e) => {
12081208
tracing::error!(
1209-
"LSP: RocksDB graph.db open failed: {e} — running in-memory only \
1210-
this session. Changes will NOT persist across restarts."
1209+
"LSP: could not load the persisted graph ({e}) - starting without it."
12111210
);
12121211
self.client
12131212
.log_message(
12141213
MessageType::ERROR,
12151214
format!(
1216-
"CodeGraph: graph database open failed ({e}). \
1217-
Index is volatile this session — restart to retry; \
1218-
if the error persists, check ~/.codegraph/graph.db for a stale LOCK file."
1215+
"CodeGraph: could not load the saved index ({e}), so this session \
1216+
starts without it. If another CodeGraph process keeps \
1217+
~/.codegraph/graph.db busy, restart once it is idle."
12191218
),
12201219
)
12211220
.await;
12221221
}
12231222
}
12241223
}
12251224

1225+
// The saved hashes (loaded in `initialize`) vouch that files are already
1226+
// in the graph. Without a persisted graph they would make the indexer
1227+
// skip every unchanged file and leave the session empty.
1228+
if !loaded_from_persistence {
1229+
self.index_state.lock().await.clear();
1230+
}
1231+
12261232
// Run incremental indexing: hash-based dedup skips unchanged files.
12271233
// - Fresh start (no persisted graph): full index if indexOnStartup=true
12281234
// - Loaded from persistence: incremental pass to catch changed files

‎crates/codegraph-server/src/domain/impact.rs‎

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@
77
88
use crate::ai_query::QueryEngine;
99
use crate::domain::node_props;
10-
use codegraph::{
11-
CodeGraph, Direction, EdgeType, NamespacedBackend, NodeId, RocksDBBackend, StorageBackend,
12-
};
10+
use codegraph::{CodeGraph, Direction, EdgeType, NamespacedBackend, NodeId, StorageBackend};
1311
use serde::Serialize;
1412
use std::collections::HashSet;
1513
use tokio::sync::RwLock;
@@ -338,24 +336,22 @@ fn find_cross_project_consumers(
338336
}
339337
};
340338

341-
// Open RocksDB, scan registry, then DROP the connection before per-project
342-
// loading — RocksDB uses exclusive locks, so only one connection at a time.
343-
let entries = {
344-
let rocks = match RocksDBBackend::open(&db_path) {
345-
Ok(r) => r,
346-
Err(e) => {
347-
tracing::warn!("[cross-project] Failed to open graph.db: {}", e);
348-
return Vec::new();
349-
}
350-
};
351-
match StorageBackend::scan_prefix(&rocks, b"_registry:") {
352-
Ok(e) => e,
353-
Err(e) => {
354-
tracing::warn!("[cross-project] Failed to scan registry: {}", e);
355-
return Vec::new();
356-
}
339+
// Open RocksDB once for the registry scan and every per-project load, so
340+
// a busy DB costs this query at most one lock wait. The handle drops, and
341+
// the lock is released, when this function returns.
342+
let rocks = match crate::memory::open_shared_graph_db(&db_path) {
343+
Ok(r) => r,
344+
Err(e) => {
345+
tracing::warn!("[cross-project] Failed to open graph.db: {}", e);
346+
return Vec::new();
347+
}
348+
};
349+
let entries = match StorageBackend::scan_prefix(&rocks, b"_registry:") {
350+
Ok(e) => e,
351+
Err(e) => {
352+
tracing::warn!("[cross-project] Failed to scan registry: {}", e);
353+
return Vec::new();
357354
}
358-
// rocks dropped here — lock released
359355
};
360356

361357
tracing::debug!(
@@ -401,11 +397,7 @@ fn find_cross_project_consumers(
401397
})
402398
.unwrap_or_else(|| slug.clone());
403399

404-
let other_rocks = match RocksDBBackend::open(&db_path) {
405-
Ok(r) => r,
406-
Err(_) => continue,
407-
};
408-
let namespaced = NamespacedBackend::new(Box::new(other_rocks), &slug);
400+
let namespaced = NamespacedBackend::new(Box::new(rocks.clone()), &slug);
409401
let mut other_graph = match CodeGraph::with_backend(Box::new(namespaced)) {
410402
Ok(g) => g,
411403
Err(_) => continue,

‎crates/codegraph-server/src/main.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ fn classify_panic(payload: &str, location: &str) -> (&'static str, &'static str)
276276
/// Strategy: panic / SIGINT / SIGTERM all funnel into `process::exit`.
277277
/// At process exit the kernel releases all fcntl / LockFileEx grants,
278278
/// so the next launch sees only the `LOCK` *file* (no live holder),
279-
/// which `RocksDBBackend::open_with_stale_lock_recovery` clears. WAL
279+
/// which RocksDB reuses without complaint. WAL
280280
/// durability is per-write, so any in-flight batch is either fully
281281
/// applied or fully discarded on next open — `exit` skipping `Drop` is
282282
/// a safe tradeoff here.

‎crates/codegraph-server/src/mcp/server.rs‎

Lines changed: 58 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ use crate::index_state::IndexState;
9292
use crate::indexer::{IndexConfig, Indexer};
9393
use crate::memory::{self, MemoryManager};
9494
use crate::parser_registry::ParserRegistry;
95-
use codegraph::{CodeGraph, NamespacedBackend, RocksDBBackend, StorageBackend};
95+
use codegraph::{CodeGraph, NamespacedBackend, StorageBackend};
9696
use serde_json::Value;
9797
use std::path::PathBuf;
9898
use std::sync::Arc;
@@ -200,9 +200,9 @@ impl McpBackend {
200200
}
201201
Err(e) => {
202202
tracing::error!(
203-
"RocksDB graph.db open failed: {e} — running in-memory only this session. \
204-
Changes will NOT persist across restarts. Inspect ~/.codegraph/graph.db and \
205-
ensure no other codegraph-server process is running."
203+
"Could not load the persisted graph ({e}) - this session re-indexes the \
204+
workspace instead. If another codegraph process keeps ~/.codegraph/graph.db \
205+
busy, restart this session once it is idle."
206206
);
207207
Arc::new(RwLock::new(
208208
CodeGraph::in_memory().expect("Failed to create in-memory graph"),
@@ -327,31 +327,22 @@ impl McpBackend {
327327
}
328328
}
329329

330-
// Mark WHERE we are (telemetry) and arm this process's sentinel for
331-
// the load. The phase guard resets to `serving` on return; the
330+
// Mark WHERE we are (telemetry); the load arms this process's
331+
// sentinel. The phase guard resets to `serving` on return; the
332332
// sentinel only clears on a completed load (success or graceful
333-
// error), not on a native AV. The sentinel body carries this
334-
// process's start time so a recycled PID can't impersonate a live
335-
// loader (Windows reuses PIDs aggressively during crash-restart
336-
// churn).
333+
// error) or a lock wait, not on a native AV. The sentinel body
334+
// carries this process's start time so a recycled PID can't
335+
// impersonate a live loader (Windows reuses PIDs aggressively during
336+
// crash-restart churn).
337337
let _phase = crate::crash_phase::enter("graph_load");
338-
let sentinel = db_path
339-
.parent()
340-
.map(|p| p.join(format!("graph.loading.{}", std::process::id())));
341-
if let Some(s) = &sentinel {
342-
let body = Self::own_start_time()
343-
.map(|t| t.to_string())
344-
.unwrap_or_default();
345-
let _ = std::fs::write(s, body);
346-
}
347-
348338
let result = Self::load_persistent_graph_inner(&db_path, slug);
349-
if let Some(s) = &sentinel {
350-
let _ = std::fs::remove_file(s);
351-
}
352339

353340
match result {
354341
Ok(graph) => Ok(graph),
342+
// Another process kept the DB open past the wait. That is
343+
// contention, not damage: redirecting here would abandon every
344+
// project's graph because a sibling session was mid-persist.
345+
Err(e @ codegraph::GraphError::Locked { .. }) => Err(format!("graph.db: {e}")),
355346
Err(e) => {
356347
// RocksDB reported corruption gracefully (didn't AV).
357348
// Redirect to a fresh generation and retry once so the
@@ -363,6 +354,7 @@ impl McpBackend {
363354
Self::sweep_stale_graph_dbs(parent, &fresh);
364355
}
365356
Self::load_persistent_graph_inner(&fresh, slug)
357+
.map_err(|e| format!("graph.db load failed: {e}"))
366358
}
367359
}
368360
}
@@ -516,25 +508,43 @@ impl McpBackend {
516508
/// detach storage to release the lock. The fallible core of
517509
/// [`open_persistent_graph`], split out so the poison-pill wrapper can
518510
/// retry it on a fresh DB after quarantining a corrupt one.
511+
///
512+
/// This process's `graph.loading.<pid>` sentinel is armed for every open
513+
/// attempt (open itself can AV on a torn DB) and kept from a successful
514+
/// open through the load, but removed while waiting out another holder:
515+
/// a session killed mid-wait must not leave poison evidence behind.
519516
fn load_persistent_graph_inner(
520517
db_path: &std::path::Path,
521518
slug: &str,
522-
) -> Result<CodeGraph, String> {
523-
// Stale-LOCK recovery: a prior crash can leave LOCK in place. The
524-
// recovery variant only clobbers it after probing for a live holder —
525-
// a healthy concurrent process is still respected.
526-
let rocks = RocksDBBackend::open_with_stale_lock_recovery(db_path)
527-
.map_err(|e| format!("Failed to open graph.db: {e}"))?;
528-
let namespaced = NamespacedBackend::new(Box::new(rocks), slug);
529-
let mut graph = CodeGraph::with_backend(Box::new(namespaced))
530-
.map_err(|e| format!("Failed to load graph: {e}"))?;
519+
) -> Result<CodeGraph, codegraph::GraphError> {
520+
let sentinel = db_path
521+
.parent()
522+
.map(|p| p.join(format!("graph.loading.{}", std::process::id())));
523+
let body = Self::own_start_time()
524+
.map(|t| t.to_string())
525+
.unwrap_or_default();
526+
let arm = || {
527+
if let Some(s) = &sentinel {
528+
let _ = std::fs::write(s, &body);
529+
}
530+
};
531+
let disarm = || {
532+
if let Some(s) = &sentinel {
533+
let _ = std::fs::remove_file(s);
534+
}
535+
};
531536

532-
// Detach to release the RocksDB lock — all data is now in memory
533-
graph
534-
.detach_storage()
535-
.map_err(|e| format!("Failed to detach storage: {e}"))?;
537+
let result = memory::open_shared_graph_db_with(db_path, arm, disarm).and_then(|rocks| {
538+
let namespaced = NamespacedBackend::new(Box::new(rocks), slug);
539+
let mut graph = CodeGraph::with_backend(Box::new(namespaced))?;
540+
541+
// Detach to release the RocksDB lock — all data is now in memory
542+
graph.detach_storage()?;
536543

537-
Ok(graph)
544+
Ok(graph)
545+
});
546+
disarm();
547+
result
538548
}
539549

540550
/// Move a corrupt `graph.db` aside so the next open starts clean. RocksDB
@@ -580,7 +590,7 @@ impl McpBackend {
580590
.map_err(|e| format!("Failed to create ~/.codegraph: {e}"))?;
581591
}
582592

583-
let mut rocks = RocksDBBackend::open_with_stale_lock_recovery(&db_path)
593+
let mut rocks = memory::open_shared_graph_db(&db_path)
584594
.map_err(|e| format!("Failed to open graph.db for persist: {e}"))?;
585595

586596
// Write project registry entry (un-namespaced, global key)
@@ -633,7 +643,7 @@ impl McpBackend {
633643
return Ok(vec![]);
634644
}
635645

636-
let rocks = RocksDBBackend::open_with_stale_lock_recovery(&db_path)
646+
let rocks = memory::open_shared_graph_db(&db_path)
637647
.map_err(|e| format!("Failed to open graph.db: {e}"))?;
638648

639649
let entries = rocks
@@ -903,7 +913,16 @@ impl McpBackend {
903913
}
904914

905915
/// Load saved file hashes from disk. Returns true if state was loaded.
916+
///
917+
/// The hashes vouch that a file's symbols are already in the graph, so
918+
/// they only load alongside a graph that has some. Against an empty graph
919+
/// (a redirected or unloadable DB) they would make the indexer skip every
920+
/// unchanged file and leave the session with no symbols at all.
906921
pub async fn load_index_state(&self) -> bool {
922+
if self.graph.read().await.node_count() == 0 {
923+
tracing::info!("No persisted graph to resume from - indexing every file");
924+
return false;
925+
}
907926
let mut state = self.index_state.lock().await;
908927
let count = state.load();
909928
count > 0

0 commit comments

Comments
 (0)