Skip to content

fix(storage): wait out a busy graph DB instead of discarding it - #27

Merged
anvanster merged 2 commits into
mainfrom
fix/graph-db-live-lock
Oct 6, 2026
Merged

anvanster merged 2 commits into
mainfrom
fix/graph-db-live-lock

Conversation

@anvanster

Copy link
Copy Markdown
Member

Intent

The developer wanted to commit the graph-database lock fix on the fix/graph-db-live-lock branch and push it through the no-mistakes gate. The fix addresses two pre-existing bugs: a busy graph.db held by another live process was treated as corruption, which moved the healthy database aside and wiped every project's persisted graph, and on Linux the stale-lock probe could delete a live database's LOCK file. The developer had earlier approved adding libc as a direct dependency for a Linux lock probe. The final fix made that unnecessary: opening the database now waits up to 10s for a lock holder, never deletes LOCK, and only quarantines the database on real damage. Saved file hashes are also ignored when a project's graph loads empty, so files get reindexed, and fs2 is dropped. Standing constraints apply: push only through no-mistakes rather than directly to origin, use conventional commit messages with the Claude co-author line, never use em dashes, and keep the project sole-maintainer with no external PRs merged.

What Changed

  • Opening graph.db while another process holds it now returns a new GraphError::Locked instead of being treated as corruption. A new RocksDBBackend::open_waiting_for_lock(_with) retries with backoff for up to 10s, and every shared-DB open path in the server (MCP load and persist, LSP backend, vector-set claims, cross-project impact) uses it through memory::open_shared_graph_db. On the MCP server, a lock that outlasts the wait is reported as contention, so the database is no longer redirected or moved aside. Cross-project impact now reuses a single handle for the registry scan and every per-project load.
  • Removed open_with_stale_lock_recovery and its probe that deleted the LOCK file, and dropped the fs2 dependency. A LOCK file left behind by a crashed process no longer blocks the next open, so the file is never deleted. The graph.loading.<pid> sentinel is removed while the open waits for another holder, so a session killed during the wait leaves no false crash evidence.
  • When a project's graph loads empty, the saved file hashes are now ignored (MCP) or cleared (LSP), so every file gets reindexed instead of skipped. User-facing warnings were reworded to say the session starts without the saved graph. Added unit tests for lock classification and the wait/hook behaviour, plus a new graph_db_recovery_test.rs integration test.

🤖 Generated with Claude Code

Risk Assessment

✅ Low: The fix round does what both prior findings asked for. The sentinel is now armed for every DB open attempt and through the load. It is removed after each refused attempt, so it never exists during a backoff sleep, including on the final Locked error. Every non-lock failure and every success ends in a disarm. The crate tests check the hook pairing by calling the API, not by reading source. The only remaining note is a latency trade-off the user chose.

Testing

I built the server and ran the new end-to-end recovery test, which spawns the real MCP binary for both the contention case and the corrupt-DB redirect case. I also ran the focused RocksDB backend unit tests. Then I drove three live scenarios against the real binary, with a separate OS process holding graph.db: a 3s hold, a 15s hold (longer than the 10s wait), and a SIGKILL during the lock wait. Every scenario passed. No redirect happened under contention, no sentinel was ever present during a wait or after the kill, LOCK and the holder's data survived, and every session's symbol search found the indexed symbol. This change has no UI, so the evidence is CLI transcripts.

  • Live validation: ✅ go - 4 of 5 scenarios driven live against the product
Scenario Result Live Evidence
A session started while another process holds graph.db waits for it, loads the persisted graph and finds the indexed symbol, with no redirect ✅ pass live live_lock_scenarios.txt S1, plus graph_db_recovery_test::a_session_waits_for_a_sibling_holding_the_db_instead_of_abandoning_it
Adversarial: a holder keeps graph.db past the 10s wait. The session logs 'in use by another process', re-indexes and still finds the symbol, does not redirect, and leaves the holder's LOCK and data in… ✅ pass live live_lock_scenarios.txt S2/S2b
Adversarial: a session SIGKILLed during the lock wait leaves no graph.loading sentinel, and the next startup does not redirect (bump the generation) and still finds the symbol ✅ pass live live_lock_scenarios.txt S3
An actually corrupt graph.db is redirected to a fresh generation, and the sessions on it re-index the files the old DB held instead of trusting the saved hashes ✅ pass live cargo test -p codegraph-server --test graph_db_recovery_test a_session_after_a_redirect_reindexes_files_the_old_db_held
On Linux, the live-lock probe no longer deletes a held database's LOCK file ⏸️ untested no No Linux host was used in this run. The SLES build VM (root@192.168.254.113) would be needed to drive it live there.
Evidence: Live lock-contention scenario transcript (real codegraph-server, separate holder process)
== seed session
  [seed] exit=0 symbol_search finds marker=True elapsed=0.2s
  [seed] files in ~/.codegraph: ['graph.db', 'projects']
  [seed] redirected(graph.generation exists)=False, sentinels=[], LOCK exists=True

== S1: another PROCESS holds graph.db for 3s while a session starts
  [s1] exit=0 symbol_search finds marker=True elapsed=3.6s
  [s1] files in ~/.codegraph: ['graph.db', 'projects']
  [s1] redirected(graph.generation exists)=False, sentinels=[], LOCK exists=True

== S2 (adversarial): another process holds graph.db for 15s (> 10s wait)
  [s2] exit=0 symbol_search finds marker=True elapsed=15.6s
  [s2-while-held] files in ~/.codegraph: ['graph.db', 'projects']
  [s2-while-held] redirected(graph.generation exists)=False, sentinels=[], LOCK exists=True
    stderr: �[2m2026-10-04T05:19:38.619462Z�[0m �[31mERROR�[0m �[2mcodegraph_server::mcp::server�[0m�[2m:�[0m Could not load the persisted graph (graph.db: Database at "/var/folders/dk/62tyv1993n5dy5jqms827nlc0000gn/T/tmp2bjobgzc/home/.codegraph/graph.db" is in use by another process) - this session re-indexes 
  holder still had DB intact; holder output: RELEASED
== S2b: next session after holder releases
  [s2b] exit=0 symbol_search finds marker=True elapsed=0.1s
  [s2b] files in ~/.codegraph: ['graph.db', 'projects']
  [s2b] redirected(graph.generation exists)=False, sentinels=[], LOCK exists=True

== S3 (adversarial): session SIGKILLed while waiting on a held DB, then a fresh session
  [s3kill] 3s into startup, sentinels present while waiting: []
  [s3kill] SIGKILLed during lock wait
  [s3-after-kill] files in ~/.codegraph: ['graph.db', 'last-phase.85833.json', 'projects']
  [s3-after-kill] redirected(graph.generation exists)=False, sentinels=[], LOCK exists=True
  [s3next] exit=0 symbol_search finds marker=True elapsed=0.1s
  [s3next] files in ~/.codegraph: ['graph.db', 'last-phase.85833.json', 'projects']
  [s3next] redirected(graph.generation exists)=False, sentinels=[], LOCK exists=True
Evidence: End-to-end graph_db_recovery_test run
     Running tests/graph_db_recovery_test.rs (target/debug/deps/graph_db_recovery_test-92c82f1c31c1fe69)

running 2 tests
test a_session_after_a_redirect_reindexes_files_the_old_db_held ... ok
test a_session_waits_for_a_sibling_holding_the_db_instead_of_abandoning_it ... ok

test result: ok. 2 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 5.25s
Evidence: RocksDB backend lock-wait unit tests
test storage::rocksdb_backend::tests::test_put_and_get ... ok
test storage::rocksdb_backend::tests::test_exists ... ok
test storage::rocksdb_backend::tests::test_delete ... ok
test storage::rocksdb_backend::tests::test_open_creates_database ... ok
test storage::rocksdb_backend::tests::test_get_nonexistent_key ... ok
test storage::rocksdb_backend::tests::test_scan_prefix ... ok
test storage::rocksdb_backend::tests::test_flush ... ok
test storage::rocksdb_backend::tests::test_open_reports_a_held_database_as_locked ... ok
test storage::rocksdb_backend::tests::test_leftover_lock_file_does_not_block_open ... ok
test storage::rocksdb_backend::tests::test_secondary_reads_primary_writes_after_catch_up ... ok
test storage::rocksdb_backend::tests::test_write_batch_puts ... ok
test storage::rocksdb_backend::tests::test_persistence_across_reopens ... ok
test storage::rocksdb_backend::tests::test_write_batch_mixed_operations ... ok
test storage::rocksdb_backend::tests::test_waiting_open_hooks_leave_a_successful_attempt_armed ... ok
test storage::rocksdb_backend::tests::test_waiting_open_hooks_bracket_every_refused_attempt ... ok
test storage::rocksdb_backend::tests::test_waiting_open_gives_up_after_max_wait_and_keeps_the_lock_file ... ok
test storage::rocksdb_backend::tests::test_waiting_open_succeeds_once_the_holder_releases ... ok
test result: ok. 17 passed; 0 failed; 0 ignored; 0 measured; 40 filtered out; finished in 0.50s
Evidence: Key live excerpt
S1 3s hold: exit=0 finds marker=True elapsed=3.6s redirected=False sentinels=[]
S2 15s hold: exit=0 finds marker=True; stderr 'Could not load the persisted graph (graph.db: Database ... is in use by another process) - this session re-indexes'; redirected=False; holder RELEASED intact
S3 SIGKILL at 3s into wait: sentinels present while waiting: []; after kill redirected=False; next session finds marker=True

Pipeline

Updates from git push no-mistakes

✅ **intent** - passed

✅ No issues found.

✅ **Rebase** - passed

✅ No issues found.

⚠️ **Review** - 1 info
  • ⚠️ crates/codegraph-server/src/mcp/server.rs:348 - open_persistent_graph writes the per-PID graph.loading.&lt;pid&gt; sentinel before calling load_persistent_graph_inner. That call now waits up to 10s in open_shared_graph_db while a sibling holds the DB. The sentinel is only removed when the load returns, and the SIGINT/SIGTERM/panic paths in main.rs end in process::exit without removing it. Concrete sequence: session A is persisting. Session B starts, arms its sentinel and waits on the lock. The user closes the editor window, or the MCP client kills B, inside that wait. The next startup's classify_load_sentinels sees a sentinel whose PID is dead, treats it as a native crash on a poisoned DB, and calls bump_graph_generation. That abandons every project's persisted graph, the same data loss this change sets out to stop. Before this change the exposure was only the open/scan itself, measured in milliseconds. Now it includes the whole lock wait. Two ways to fix it: (1) acquire the lock before arming the sentinel, for example a waiting open followed by writing the sentinel and then the scan; the trade-off is that the sentinel would no longer cover an AV inside DB::open recovery. (2) Remove this process's sentinel from the exit and signal hook, so a clean kill never leaves poison evidence.
  • ℹ️ crates/codegraph-server/src/domain/impact.rs:402 - find_cross_project_consumers used to skip a project immediately when the shared DB was busy. Now it reopens the DB once per registered project, and each open can wait up to 10s. If a sibling holds the DB for a long persist, one impact query can stall for up to 10s times N projects. Contention normally clears within one wait, so this is a latency note, not a correctness defect. A single open for the registry scan and all per-project loads would avoid the repeated waits.

🔧 Fix applied.
1 info still open:

  • ℹ️ crates/codegraph-server/src/domain/impact.rs:342 - find_cross_project_consumers now keeps one shared handle open from the registry scan through every per-project load. This is what the user asked for, and it does cap the query at one lock wait. The trade-off is that the RocksDB lock is now held for the whole loop instead of being released between projects. If loading every registered project takes longer than SHARED_GRAPH_DB_LOCK_WAIT (10s), a sibling session that starts, or tries to persist, during the loop gets Locked. A starting session then runs without its persisted graph. This only happens with many large projects, and the fix-round instructions explicitly chose this design. No action is needed, but if it shows up in practice, holding the handle only across the scan plus the per-project load would cap the hold time.
✅ **Test** - passed

✅ No issues found.

  • Live validation: ✅ go - 4 of 5 scenarios driven live against the product
Scenario Result Live Evidence
A session started while another process holds graph.db waits for it, loads the persisted graph and finds the indexed symbol, with no redirect ✅ pass live live_lock_scenarios.txt S1, plus graph_db_recovery_test::a_session_waits_for_a_sibling_holding_the_db_instead_of_abandoning_it
Adversarial: a holder keeps graph.db past the 10s wait. The session logs 'in use by another process', re-indexes and still finds the symbol, does not redirect, and leaves the holder's LOCK and data in… ✅ pass live live_lock_scenarios.txt S2/S2b
Adversarial: a session SIGKILLed during the lock wait leaves no graph.loading sentinel, and the next startup does not redirect (bump the generation) and still finds the symbol ✅ pass live live_lock_scenarios.txt S3
An actually corrupt graph.db is redirected to a fresh generation, and the sessions on it re-index the files the old DB held instead of trusting the saved hashes ✅ pass live cargo test -p codegraph-server --test graph_db_recovery_test a_session_after_a_redirect_reindexes_files_the_old_db_held
On Linux, the live-lock probe no longer deletes a held database's LOCK file ⏸️ untested no No Linux host was used in this run. The SLES build VM (root@192.168.254.113) would be needed to drive it live there.
  • cargo build -p codegraph-server --bin codegraph-server
  • cargo test -p codegraph-server --test graph_db_recovery_test (spawns the real MCP binary against a temporary HOME and workspace)
  • cargo test -p codegraph --lib rocksdb_backend (Locked classification, waiting open, per-attempt hook bracketing)
  • Manual live driver (python, real codegraph-server --mcp, temporary HOME): seed session, then a separate holder process holding graph.db for 3s, then 15s (longer than the wait), then a session SIGKILLed 3s into a lock wait, checking symbol_search results, graph.generation, graph.loading.* sentinels, LOCK and holder data after each
✅ **Document** - passed

✅ No issues found.

⚠️ **Lint** - 1 info
  • ℹ️ crates/codegraph-server/src/backend.rs:1168 - Running cargo fmt --all --check shows formatting drift in about 60 files in crates this change does not touch (language parsers, codegraph-harness, codegraph-memory examples). Running cargo clippy shows warnings in codegraph-server (backend.rs:1168/1253 unused total_indexed, mcp/server.rs:1484/4108/4155/4254), none of them on lines this change touched. All of it predates this change, and CI enforces neither tool. The changed files themselves are fmt-clean and add no new clippy warnings. This is left for a separate cargo fmt --all plus clippy cleanup PR so this fix stays focused.
✅ **Push** - passed

✅ No issues found.

anvanster and others added 2 commits October 3, 2026 21:59
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
@github-actions

github-actions Bot commented Oct 4, 2026

Copy link
Copy Markdown

🔍 CodeGraph PR Review

12 files changed (+448/−212, 32 functions) · Risk: 🟡 medium

Blast radius

44 direct callers affected (18 breaking) across CodeGraph/CodeGraph/scripts, CodeGraph/jetbrains/scripts, codegraph-memory/src/embedding, codegraph-server/src/ai_query, codegraph-server/src/domain

⚠️ Test gaps (15 functions, 0 coverage)

  • save_vectors_map (crates/codegraph-server/src/ai_query/engine.rs) — body_changed
  • load_symbol_vectors (crates/codegraph-server/src/ai_query/engine.rs) — signature_changed
  • claim_vector_set (crates/codegraph-server/src/ai_query/engine.rs) — signature_changed
  • initialized (crates/codegraph-server/src/backend.rs) — body_changed
  • persist_graph_to_rocksdb (crates/codegraph-server/src/backend.rs) — body_changed
  • find_cross_project_consumers (crates/codegraph-server/src/domain/impact.rs) — body_changed
  • new (crates/codegraph-server/src/mcp/server.rs) — body_changed
  • open_persistent_graph (crates/codegraph-server/src/mcp/server.rs) — body_changed
  • load_persistent_graph_inner (crates/codegraph-server/src/mcp/server.rs) — signature_changed
  • persist_graph (crates/codegraph-server/src/mcp/server.rs) — body_changed
  • …and 5 more

Suggested reviewers

Andrey Vasilevsky (127 lines), anvanster (17 lines)

Suggested commit: feat(scripts): <describe the change> · 26 tests cover the changes
🤖 Generated by CodeGraph

@anvanster
anvanster merged commit ae2bed1 into main Oct 6, 2026
1 check passed
@anvanster
anvanster deleted the fix/graph-db-live-lock branch October 6, 2026 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant