fix(indexing): make reindex survivable and batch graph writes - #42
Merged
Merged
Conversation
added 4 commits
September 26, 2026 09:34
Resolve the reindex crash-loop found on 2026-09-25 (py-spy + ledger evidence):
- deadlock: run() held the write RLock for the whole run while _bounded_link
ran bulk_write on a new thread that re-acquired it. Split the locks: run()
now holds begin_run() (a separate, non-reentrant Lock); _table_write_lock is
acquired per DB operation.
- concurrency: auto-index and a manual trigger could run two indexers in
parallel. begin_run() is now single-flight.
- embedder: idle-unload during a long parse let another workspace steal the
fixed port. Added a cross-process embedder lease; the idle watchdog skips
unload while a lease is fresh.
- observability: failures were swallowed ("Exception suppressed") and a job
could stay "running" with no worker. Added a durable reindex ledger
(reindex_ledger.jsonl), a terminal-status guarantee in finally, a stall
watchdog, and full tracebacks.
- path contract: normalise rel paths to POSIX at every index-feeding point
(full reindex wrote "\", incremental "/" -> ~2x index bloat).
- graph: thread-safe local RLock before the cross-process mutex.
Live-verified: full reindex completed (858s); index 19653 -> 10103 rows,
path-duplication 668 -> 0.
PropertyGraph.add_node/add_edge/delete_node each took a named mutex plus a BEGIN/commit per row (~194 entities/s measured, E18). Add PropertyGraph.batch(): one cross-process lock and one transaction for a whole block, reusing the connection; nested batches reuse the same transaction and errors roll back the block atomically. indexer wraps the per-file symbol-graph update (remove + add_*) in it. Measured with exp_graph_write_throughput: A (per-call) 194 ent/s vs D (batch, the real API) 26 826 ent/s -> 138x; negative control counts equal. Guard: tests/test_graph_batch.py (counts match non-batch, atomic rollback on error, nested batch = one tx, commit persists).
check_known_issues (E4.8 R4) failed at 475 lines. Moved the auto-synced E13/E14/E16 entries appended since the last archive into docs/archive/KNOWN_ISSUES_2026_09.md; the live file is back to 296.
The 4 parse workers shared one batch transaction, so the second BEGIN IMMEDIATE raised "cannot start a transaction within a transaction" during finalize (GraphSymbolResolver.resolve_all). batch() now records the owning thread id; a foreign thread blocks on the locks and opens its own batch, and the batched write paths verify ownership before reusing the shared connection. Guard: test_concurrent_batches_do_not_share_a_transaction (Barrier, 2 threads, no OperationalError, exact counts).
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Makes a full reindex survive concurrent/duplicate runs and removes the graph-write bottleneck.
fix(indexing): make reindex survivable, single-flight and recorded— split run-lock vs write-lock (removed the bounded-write deadlock), single-flight viabegin_run(), durablereindex_ledger, terminal status + stall watchdog, cross-processembedder_lease.perf(indexing): batch per-file graph writes (138x)—PropertyGraph.batch().fix(indexing): make PropertyGraph.batch thread-owned— the 4 parse workers shared one batch transaction (cannot start a transaction within a transactionat finalize); the batch is now owned per thread.docs(known-issues): archive auto-synced tail under 300-line limit.Evidence
completedin 603s, 10106/10106 rows; path-duplication 0.verify_clean_state.sh --no-cloneon this branch: 1852 passed, 0 failed.Tests
tests/test_graph_batch.py, incl.test_concurrent_batches_do_not_share_a_transaction.