Repository navigation
fix(storage_engine): end the HNSW build deadlock that hung .index create and bound index builds by the request deadline - #382
Merged
Merged
Conversation
…and build indexes under the request deadline without blocking the graph (#377) A rare `.index create` never replied. Thread dumps of a looped build show the cause in hnsw_rs 0.3.4: its reverse-link update held a read lock on the new point's neighbour list while it write-locked each neighbour's list, so two parallel inserts whose new points listed each other waited for each other forever. Every parallel build could hang this way: `.index create`, `.index rebuild`, tombstone compaction inside a delete, and the rebuild of indexes when a knowledge graph loads. A standalone loop of the scenario's build (100 vectors, dimension 8, cosine) on 4 pinned cores hung 2 runs in 3 within a minute; with the fix, about 270,000 builds never hung. hnsw_rs is now vendored under vendor/hnsw_rs with [patch.crates-io] and three marked patches (vendor/hnsw_rs/PATCHES.md): 1. copy the neighbour list and release its lock before locking any neighbour, so an insert never holds one point's lock while waiting for another's (upstream master still has the deadlock); 2. store a reverse link at the layer it links on instead of the new point's top layer (upstream's unreleased fix). Layer 0 lacked the reverse links of every point above it and could split apart: this is the cause of the recall flake #302 (recall@10 fell to 0.59 in about 1 build in 200; over 200 builds the median goes from 0.984 to 0.996 and the minimum from 0.59 to 0.994); 3. log the every-50,000-points line instead of printing it to stdout. Serial insertion would also avoid the deadlock, but it is 3.5x slower on 4 threads and 14x on 16 (50k x 128: 145 s against 11 s). `.index create` and `.index rebuild` are now bounded by the request's deadline and cancellable, like queries. They used to enter the commit before building and hold the knowledge graph's write lock through the build, so a build could not be stopped and froze every read and write of the graph while it ran. Now the rows are taken under a short read lock, the index is built without any lock, checking the request's stop before each vector, and only then does the request enter its commit: under the write lock it applies the rows written during the build, checks that no other create or redefinition won meanwhile, and installs the index. A stopped build installs nothing and answers deadline_exceeded or cancelled. The first vector is inserted alone so that parallel inserts never meet an empty graph, where hnsw_rs leaves them unlinked. The Dockerfile copies vendor/ and CI's Rust filter includes it. The indexing guide says builds no longer block the graph and run under the deadline (query_timeout_ms, 30 s by default, to be raised for big builds). Fixes #302
jsam
force-pushed
the
fm/fix-index-hang
branch
from
October 5, 2026 22:18
b77f4bd to
69e8244
Compare
jsam
changed the title
jsam
marked this pull request as ready for review
October 5, 2026 22:20
This was referenced Oct 5, 2026
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
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.
Intent
I want you to really hyper focus on making the symbolic engine, you know, crisp and really truly production ready. (Context: #377 : a rare hang where .index create (HNSW, cosine) never replies; also related flake #302 in the HNSW recall test.)
Issue #377, "Rare hang: .index create (HNSW, cosine) never replies under load": the scenario harness test harness::shop_pack_vector_serves_the_near_rule failed 1 of 9 full debug runs on 4 pinned cores because the engine never answered
.index create emb_idx on embedding(vec) metric cosinewithin 60 s. It was not reproduced alone, in 16 parallel copies or in 120 overloaded runs, and is likely related to the process-wide rayon pool / HNSW build path (#302). A request that hangs forever is a production bug: find the cause, fix it, and make index creation bounded (deadline + cancellation) like queries. The scenario is quarantined to the nightly tier with that issue number until fixed.Issue #302, "hnsw recall@10 test flakes on CI (0.58-0.65 vs ~0.98) - parallel_insert makes the graph nondeterministic": hnsw_index::tests::test_hnsw_recall_at_10_on_3000_random_vectors (3000 rows of dimension 32, m=16, ef_construction=200, ef_search=64, cosine) sometimes fails on CI with recall far below its 0.95 bar (0.654 and 0.577 seen; usual recall about 0.98), and each run's recall varies even though the data seeds are fixed. Options named there: build the test index deterministically and keep the 0.95 bar, or find out why some parallel builds come out degenerate, since a 0.58 recall at ef_search=64 may point to a real build-quality problem rather than test noise.
What Changed
vendor/hnsw_rsand loads it through[patch.crates-io], with three patches listed inPATCHES.md. Patch 1 fixes the parallel-insert deadlock behind the.index createhang (Rare hang: .index create (HNSW, cosine) never replies under load #377):reverse_update_neighborhood_simpleheld a read lock on one point's list while waiting for a write lock on another's. Patch 2 stores reverse links on the correct layer, which fixes the degenerate graphs behind the recall flake (test: hnsw recall@10 test flakes on CI (0.58-0.65 vs ~0.98) — parallel_insert makes the graph nondeterministic #302). Patch 3 sends a progress line to the log instead of stdout. The Dockerfile and the CI path filter now includevendor/..index createand.index rebuildnow build the index without holding the knowledge-graph lock, in a dedicatedindex-buildrayon pool sized to half the global pool, so queries and writes continue during the build.HnswIndex::build_unlessinserts the first vector alone, then the rest in parallel, and checks a stop callback before each insert. The build stops at the request's deadline or oncancel. A stopped build registers nothing; otherwise the request takes the write lock, applies the writes made during the build, and only then enters its commit to install the index. The statement gate no longer puts these two commands into the commit up front, and the indexing guide documents the new behaviour.vector_index_tests.rs,hnsw_index_tests.rs) and for cancelling a WebSocket request during an index build (ws_cancel_tests.rs). Lifts the Rare hang: .index create (HNSW, cosine) never replies under load #377 quarantine fromharness::shop_pack_vector_serves_the_near_ruleand updatesTESTING.mdto match.Risk Assessment
Testing
I stood up the real server on its own port with a throwaway data directory and drove it over WebSocket. The scenario that failed in round 1 now passes. During a 19 s build of a 15k-row index, another connection ran 58 reads with a 1 s deadline: the slowest took 69 ms (7 ms with no build running) and none hit the deadline. In round 1 the same read timed out, and a read with no short deadline took 8.8 s. The build takes about twice as long now, because the build pool has half the threads. The other live checks also passed: the issue's exact
.index createreplied 50 of 50 times in a row and 8 of 8 when sent at once on separate connections; a 150 ms deadline and a cancel each stopped a build and left nothing registered; a rebuild stopped by its deadline kept the old index; a write and read during a build were answered at once and the written row ended up in the index. One driver check failed:hnsw_nearestfor row 42 on a dataset where every vector appears six times. I compared this with a base-commit server built in a throwaway directory. The base misses 10 to 64 of 200 such queries; the change misses 25 to 48. On distinct random data the change finds every query's own row as top-1 (300 of 300). So this is a pre-existing weakness of HNSW on exact duplicates, not a regression. Targeted tests also pass: the hnsw, vector_index and residency unit tests (including the never-hang stress test), the #302 recall test 20 of 20 times, the ws deadline/cancel/during-build tests, and the un-quarantined shop_pack_vector scenario on the plain PR tier (a test-harness run, not a live one). I stopped the servers and removed the temp directories; the worktree is clean..index create emb_idx ... metric cosinealways replies (50 in a row, 8 at once on separate connections).index createwith a short timeout_ms returns deadline_exceeded promptly and registers nothing.index createstops it and registers nothingEvidence: Reads during an index build stay fast (round-1 failure, now passing)
Evidence: Live WS driver: create replies, deadline, cancel, rebuild, writes during build
Evidence: Recall on distinct random vectors (live)
Evidence: Duplicate-heavy recall, base commit
Evidence: Duplicate-heavy recall, this change
Evidence: ws index tests + un-quarantined scenario
Evidence: #302 recall test looped 20x
Evidence: Server log for round-2 live runs
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
tests/scenarios/harness.rs:68- The intent says the scenario 'is quarantined to the nightly tier with that issue number until fixed'. This change claims the durable fix for Rare hang: .index create (HNSW, cosine) never replies under load #377: vendor patch 1 removes the lock-order inversion in reverse_update_neighborhood_simple, and builds now run under the request's deadline and cancel. Butharness::shop_pack_vector_serves_the_near_ruleis still#[ignore = "quarantined: rare engine hang in .index create (#377); nightly tier runs it"], so the PR gate still does not run the regression scenario from the issue. Should the quarantine be lifted in this change (moving the test back to the PR tier) or left for a follow-up? That CI-tier decision is yours.src/storage_engine/vector_index.rs:503-catch_upruns insidewith_kg_mut(install_created_index:611, install_rebuilt_index:674). It hashes every row of both the before and after relations, then applies the upserts written during the build one at a time withHnswIndex::apply. All of this holds the KG write lock and never checks the stop signal. If a bulk load lands while a large build runs, that catch-up work blocks the graph and is not bounded by the deadline. It is still far less than the old behaviour of building the whole index under the lock, so this is only a tradeoff to note.🔧 Fix applied.
2 infos still open:
src/storage_engine/vector_index.rs:503-catch_upruns insidewith_kg_mut(install_created_index:611, install_rebuilt_index:674). It hashes every row of both the before and after relations, then applies the upserts written during the build one at a time withHnswIndex::apply. All of this holds the KG write lock and never checks the stop signal. If a bulk load lands while a large build runs, that catch-up work blocks the graph and is not bounded by the deadline. It is still far less than the old behaviour of building the whole index under the lock, so this is only a tradeoff to note.src/storage_engine/vector_index.rs:95-build_for_requestchecks for a stop withRequestControl::expire_if_due. Once the request has entered its commit (state COMMITTING),stop()returnsHalt::TooLate, so neither the deadline nor a cancel stops the build. Concrete case: a program whose second statement is also an index build, such as.index create a ...then.index create b ..., or.index drop xthen.index rebuild y. The first statement callsbegin_commitininstall_created_index(line 308) or as a durable meta command (supervise.rs:129). The second build then runs to completion whatever the deadline. This follows the existing rule that a committing request is not interrupted, and queries after a commit behave the same way. With vendor patch 1 the build is also finite, so this is a tradeoff to note, not a hang.🔧 **Test** - 3 issues found → auto-fixed ✅
src/storage_engine/residency_tests.rs:295- The change added a thirdcontrolparameter toStorageEngine::create_index_in, butresidency_tests.rs:295still called it with two arguments.cargo test --libfailed with E0061, so no lib unit test could run (CI would fail). Fixed in the worktree by passingNone; the residency, vector_index and hnsw_index tests now compile and pass.src/hnsw_index.rs:145- A build no longer blocks writes, but it still blocks reads.HnswIndex::build_unlessrunsinto_par_iteron the global rayon pool, which query evaluation also uses, so a read on another connection waits for the whole build. Measured live on the real server: 70 ms became 8.8 s during a 10 s build, and a read with a 1 s deadline failed withdeadline_exceeded. The docs added in this change say queries go on while the build runs (docs/content/docs/guides/indexing.mdx), and the ws testwrites_and_reads_are_served_while_an_index_buildspasses only because it checks ordering. This is not a regression: at the base commit the build held the KG write lock, which was worse. But a build longer thanquery_timeout_mswill time out every read that runs alongside it. Options: run builds in a dedicated rayon pool with fewer threads, or correct the docs and tighten the test. Which do you want?.index create emb_idx on embedding(v) metric cosinealways replies (50 create/drop rounds, plus 8 creates at once on 8 connections)cancelled, and no index is registeredtaskset -c 0-3 cargo test --test scenarios harness::shop_pack_vector_serves_the_near_rule(now un-ignored)240 runs of the scenario binaryharness::shop_pack_vector_serves_the_near_rule --exact, 4 at a time on 4 pinned corestaskset -c 0-3 cargo test --test ws_cancel_tests index(deadline, cancel, and writes/reads during a build)Live: realtarget/debug/inputlayer-serverwith its own config, data dir and port 18977, driven over WebSocket bylive-driver.mjs(50 create/drop rounds of the issue's cosine create; 8 creates at once on 8 connections; create and rebuild withtimeout_ms; cancel during a build; write and read on another connection during a build)Live:live-concurrency.mjsandlive-starvation.mjstiming another connection's writes and reads, with and without a build running (15k rows, 10 s build)Live:live-late-rows.mjs,hnsw_nearestfinds rows written during and after the buildHNSW_STRESS_SECS=60 taskset -c 0-3 cargo test --release --lib test_hnsw_concurrent_builds_never_hang -- --ignoredwith the vendor patchThe same stress test with[patch.crates-io]commented out (stock hnsw_rs 0.3.4) to reproduce #377, then Cargo.toml and Cargo.lock restored40 runs ofhnsw_index::tests::test_hnsw_recall_at_10_on_3000_random_vectorson 4 pinned cores, recall recordedcargo test --lib --no-run, which failed to compile; fixedsrc/storage_engine/residency_tests.rs:295, then ranstorage_engine::residency::tests,vector_index_testsandhnsw_index🔧 Fix applied.
✅ Re-checked - no issues remain.
.index create emb_idx ... metric cosinealways replies (50 in a row, 8 at once on separate connections).index createwith a short timeout_ms returns deadline_exceeded promptly and registers nothing.index createstops it and registers nothingLive: release-free debuginputlayer-serveron 127.0.0.1:18977 with a throwaway data dir, driven over WebSocket bynode live-starvation-r2.mjs(15k-row index build while another connection reads under timeout_ms=1000)Live:node live-driver-r2.mjs(S1 50 sequential + 8 concurrent.index create emb_idx ... metric cosine; S2 timeout_ms=150 deadline; S3 cancel; S5 rebuild with timeout_ms=100 keeps the old index; S4 write and read during a build, late row indexed)Live:node live-recall-probe.mjs(3000x32 distinct random vectors, self top-1 via hnsw_nearest)Live:node live-dup-probe.mjs,live-dup-probe-change.mjsvslive-dup-probe-base.mjsagainst a base-commit (9aa22753) server built in a throwaway dir on port 18978: duplicate-heavy data recall, compared with basecargo test --lib -- hnsw_index vector_index residency(plus--ignored: test_hnsw_concurrent_builds_never_hang)cargo test --lib -- hnsw_index::tests::test_hnsw_recall_at_10_on_3000_random_vectorslooped 20x (#302)cargo test --test ws_cancel_tests -- index(deadline, cancel, reads/writes during build)cargo test --test scenarios -- shop_pack_vector(un-quarantined #377 scenario, no --ignored)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.