Auto-scale efConstruction and the search-ef ceiling with graph size for 1M+ node HNSW graphs - #2181
Auto-scale efConstruction and the search-ef ceiling with graph size for 1M+ node HNSW graphs#2181kriszyp wants to merge 11 commits into
Conversation
…holds at 1M+ nodes At a constant efConstruction (100), edge-selection quality erodes as the corpus grows until true neighbours become unreachable at any search ef: recall@10 at 1M nodes plateaued at 0.94-0.97 across ef 512-1536, while rebuilding at efConstruction 200 restored 0.985/0.997 and made queries faster at the same ef (21% fewer nodes visited). Scale it with sqrt(nodes/250K) from the same memoized node count the search-side auto-scale resolves, capped at 512; an explicitly configured efConstruction stays authoritative. Closes #2180 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… a float pool can hold Two-pass generation from a dedicated RNG stream (independent of the Math.random the index draws levels from, so both passes replay identical rows): pass 1 collects evenly-spaced query source rows, pass 2 feeds each row to the index and folds it into the brute-force ground truth as it streams by. Peak memory is the graph plus one row, vs a 15GB pool at 5M x 768 dims. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces auto-scaling for the HNSW efConstruction parameter based on the graph size to prevent edge quality erosion and maintain high search recall as the corpus grows. It also adds a --stream option to the HNSW scale benchmark to support two-pass streaming generation for large datasets, and includes unit tests for the auto-scaling logic. The review feedback suggests optimizing the auto-scaling logic during rapid bulk imports by reading the live node count directly from this.idIncrementer via Atomics.load to avoid the 10-second TTL lag of approximateNodeCount(). A corresponding update to the mockNodeCount test helper was also recommended to support this optimization.
This comment has been minimized.
This comment has been minimized.
…ath, own the count caveats From the pre-push cross-model review: resolve the build-side node count with resolveNodeCount() (one atomic load — the counter is always initialized on the write path) instead of the memoized approximateNodeCount(), removing TTL lag during bulk loads; document that the count is a lifetime high-water mark (churn-table build-cost caveat, bounded at the cap), that a post-restart reindex repeats the ramp, and that pinning efConstruction also pins search ef; update the stale schema.graphql description; fix the sepRand diagnostic below 200 rows; trim comments that restated DESIGN.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nly at id allocation An update-only worker after a restart never allocates a node id, so the counter stayed uninitialized and the direct count resolution fell back to a reverse seek on every update. Extract the create-or-attach into ensureIdIncrementer() and call it from the connection pass: one seek on the first write after open, atomic loads after. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e query-time ef Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2M and 5M validation runs (streaming mode, this branch)Same harness and calibration as the 100K–1M sweep on #2125 (768-dim, int8,
What this says about the fix. The construction side is doing its job: at 5M, What binds next is the search side. With good graphs, recall at the auto-resolved Two caveats. The 5M query latencies carry memory-pressure noise (27.6 GB RSS on a 30 GB box — page-cache squeeze and poor locality inflate per-visit cost; visited only grew 1.2× while p50 grew 2.1× from 2M). And the printed Raw logs/JSON: Generated by Claude Fable 5. |
…arge graphs AUTO_EF_MAX was calibrated when layers above 0 were searched at the full ef, which made large efs cost seconds; after the greedy-descent fix the same headroom costs tens of milliseconds, and the measured decay at a pinned 512 on well-built graphs (set-recall 0.997 -> 0.955 -> 0.935 across 1M/2M/5M) is recall left on the table. Past 1M nodes the scale resumes from the plateau (512 * sqrt(nodes/1M)) up to AUTO_EF_CEILING (2048, binding ~16M); the 5M point resolves 1,145, bracketed by the measured ef-1024 sweep there (0.985 set-recall). Raise AUTO_EFC_MAX to 1024 on the same curve (validated to 447 at 5M), and re-derive LIMIT_EF_MAX from the new ceiling. Latency-first apps pin efConstructionSearch or a per-query ef; graphs past tens of millions should shard. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…he raised ef ceiling, guard the write-path count The filtered-traversal budget (maxVisits, #1241) no longer follows the second-regime search ef: each budgeted visit is a synchronous record load + predicate evaluation, so an auto-scaled ef contributes at most AUTO_EF_MAX to the budget (explicit per-query or schema ef still raises it — the caller owns that cost). Guard the connection pass's counter ensure/resolve so a write can never fail on an ef heuristic with a safe fallback. Debug-log the resolved efC once per value so replica-divergent build quality is diagnosable. Correct the stale 5.12x churn-cost figure in DESIGN.md (AUTO_EFC_MAX is now 1024: 10.24x ef, ~6-7x build time), fix a stale DESIGN.md heading pointer, and add a mechanism test proving the update-only path ensures the shared counter with one seek rather than seeking per write. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…h succeeds Assigning the private seed array to this.idIncrementer before attaching meant a thrown attach (previously fatal to the write, made silently reachable by the round-4 guard) left this process allocating ids from a counter no other worker can see — cross-worker id collisions and graph corruption. Install only the attached shared view; on failure the counter stays unset and the next write retries. Test simulates a failing attach: the write succeeds on the base-efC fallback, no counter is installed, and the next write recovers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Reviewed — |
| // `ef` candidates in a sorted array with an O(len) insert. Kept within a small multiple of the | ||
| // index's own auto-scaled ceiling so the worst case stays the same order; a caller who genuinely | ||
| // wants more sets an explicit `ef` and owns the cost. | ||
| const LIMIT_EF_MAX = 2 * AUTO_EF_CEILING; |
There was a problem hiding this comment.
Medium: LIMIT_EF_MAX doubles to 4096 for every graph, but the ceiling that justifies it only binds at ~16M nodes
minResults is offset + limit — unprivileged and set on every request (resources/search.ts:513 → line 1104-1105), with no clamp, rate limit, or per-query CPU budget anywhere in that chain. The unfiltered layer-0 path has no maxVisits at all (filterState is undefined without a filter), so ef alone sizes the work: one safeGetSync + a dims-wide distance per visited node, plus an O(len) results.splice() per admission into an array of up to 4096. The comment's "kept within a small multiple of the index's own auto-scaled ceiling" only holds once autoScaleEf actually reaches 2048 — at 1M nodes this is 8x the ef the index resolves for itself, at 10K nodes 40x. A schema-level efConstructionSearch pin does not block the widening either: line 1104 checks only !explicitEf.
Suggested fix: anchor the limit ceiling to what this index resolves rather than to a ceiling that binds at 16M — e.g. Math.min(2 * AUTO_EF_CEILING, 4 * resolvedEf) — and let a schema-configured efConstructionSearch cap the widening the way an explicit per-query ef does.
Separately, estimateCountAsSort() (line 1368) still estimates against this.efConstructionSearch (default 50). That gap is pre-existing, but raising the real ef to 1145/2048 widens the planner's under-estimate to ~40x on exactly the large tables this PR targets.
—
Generated by Barber AI
| """ | ||
| Construction effort/recall parameter (HNSW). Higher values improve recall at | ||
| the cost of build time and memory. | ||
| the cost of build time and memory. When omitted, it auto-scales with graph |
There was a problem hiding this comment.
Medium: two user-visible defaults change with no update to the public docs
The published reference (HarperFast/documentation → reference/database/schema.md) still lists efConstruction | 100 | Max nodes explored during index construction, which is false for any table past 250K nodes after this lands, and says nothing about the default query ef now scaling past 1M to 2048. That repo already has a <VersionBadge type="changed" version="..." /> convention, used for the previous ef auto-scale change on this same page. The harper repo has no CHANGELOG or release-note file, so the docs repo is the only operator-facing channel for a change that silently alters p50 latency on existing >1M-node tables.
Suggested fix: land a companion documentation PR — correct the efConstruction default cell to "auto-scaled (base 100)", add VersionBadge type="changed" entries for both the build-side scale and the second search-ef regime, and state the latency implication plus efConstructionSearch as the opt-out.
—
Generated by Barber AI
Add second-regime filtered-budget coverage, preserve construction scaling when a shared-counter attach fails, expose resolved efConstruction in the benchmark, and document the operator-facing cost and configuration tradeoffs. Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Preserve the construction scale when shared counter attachment fails, bound retries, pass the active write transaction through fallback seeks, and document the remaining large-graph cost caveats. Co-Authored-By: GPT-5 Codex <noreply@openai.com>
Retain the selectivity rationale for filterExpansion while keeping the revised auto-scale budget description concise. Co-Authored-By: GPT-5 Codex <noreply@openai.com>
| // `ef` candidates in a sorted array with an O(len) insert. Kept within a small multiple of the | ||
| // index's own auto-scaled ceiling so the worst case stays the same order; a caller who genuinely | ||
| // wants more sets an explicit `ef` and owns the cost. | ||
| const LIMIT_EF_MAX = 2 * AUTO_EF_CEILING; |
There was a problem hiding this comment.
Suggestion (non-blocking): This raises LIMIT_EF_MAX from 4 * AUTO_EF_MAX (2048) to 2 * AUTO_EF_CEILING (4096). The limit-driven widening at line 1128 (if (minResults !== undefined && !explicitEf && minResults > effectiveEf)) only checks !explicitEf — not efSearchConfigured — so a schema pin via efConstructionSearch doesn't stop an unprivileged offset + limit from doubling the synchronous candidate-list work (sorted-array admission, O(len) per insert) relative to before this PR, on exactly the large tables this change targets. The PR body's adjudicator's ledger already flags the 2048→4096 ratio for confirmation, but doesn't mention this schema-pin gap specifically.
Consider gating the widening on !explicitEf && !efSearchConfigured, or anchoring LIMIT_EF_MAX to a small multiple of the ef this index actually resolved for itself (resolvedEf) rather than the absolute ceiling, so a caller who pinned a lower search ef isn't silently widened past it.
|
Reviewed |
| // atomic load; a failed attach falls back to the memoized seek and retries after its TTL. | ||
| let efConstruction = this.efConstruction; | ||
| if (!this.efConstructionConfigured) { | ||
| efConstruction = autoScaleEfConstruction(this.resolveConstructionNodeCount(options)); |
There was a problem hiding this comment.
Low: the "a write must never fail on it" guard was removed; the invariant now rests on validation two methods away
The previous head wrapped the whole node-count resolution in try { ... } catch, with the comment stating the reason outright: the count is an ef heuristic with a safe fallback, so a write must never fail on it. This commit removes both the guard and that sentence, and resolveConstructionNodeCount() is now called bare.
Every other throw is still contained — ensureIdIncrementer is caught inside resolveConstructionNodeCount, and resolveNodeCount's seek has its own catch returning 0. The one uncontained path is the healthy fast path: line 751 if (this.idIncrementer) return this.resolveNodeCount() → line 804 Number(Atomics.load(this.idIncrementer, 0)), which throws RangeError if idIncrementer is ever a zero-length BigInt64Array. Today that state is unreachable only because of the new byteLength validation added to ensureIdIncrementer in this same commit — so a documented write-path invariant went from locally enforced to enforced at a distance by a check in a different method.
I found no reachable failure on this head. The ask is to keep the invariant enforced where it is relied on rather than inferred: restore the try/catch around this line (falling back to this.efConstruction), and keep the sentence that says why it is there.
—
Generated by Barber AI
| } finally { | ||
| store.getUserSharedBuffer = originalGetUserSharedBuffer; | ||
| store.getKeys = originalGetKeys; | ||
| customIndex.idIncrementerRetryAt = 0; |
There was a problem hiding this comment.
Low: this finally restores three things but not the five the test saved, so one assertion failure cascades through the rest of the file
The test saves five fields up front — idIncrementer, nodeCount, nodeCountAt, idIncrementerRetryAt, idIncrementerFailureLogged — but this first finally only restores getUserSharedBuffer, getKeys, and idIncrementerRetryAt. All five are restored in the second try's finally, which is never reached if any of the seven assertions above (lines 824-834) throws.
On a genuine failure the index is left with idIncrementer === undefined (the real shared counter dropped), nodeCountAt === 0, and idIncrementerFailureLogged === true, which leaks into every later test in the file — so the one test guarding the degraded attach path is the one whose failure is hardest to read. The sibling test at line 853 ('does not cache an unusable shared-counter buffer') gets this right with a single try/finally.
Suggested fix: wrap both phases in one try/finally that restores all five saved fields plus the two store methods, so a failing assertion reports itself and nothing else.
—
Generated by Barber AI
| } | ||
| } | ||
| } | ||
| return this.approximateNodeCount(options); |
There was a problem hiding this comment.
Nit: the attach-failure path does two reverse seeks where one would do, and the new test asserts the waste as intended behavior
When the attach fails, ensureIdIncrementer has already performed its reverse seek to find the largest node id before getUserSharedBuffer throws; that value is then discarded, and this line does a second identical seek via approximateNodeCount. The new test bakes it in: assert.strictEqual(seekTransactions.length, 2, 'the first update should seek once to attach and once to fall back').
Only two seeks per NODE_COUNT_TTL in a degraded state, so the cost is small — but the seek is the exact thing the memo exists to avoid, and asserting 2 will keep it at 2. Suggested fix: have ensureIdIncrementer hand back the id it already found (or seed the memo with it before throwing) so the fallback reuses it, and assert 1.
—
Generated by Barber AI
| // query — the same value, and read from the index rather than recomputed here, so this cannot | ||
| // drift when the auto-scale formula changes. | ||
| if (inSearch && level === 0) currentEf = ef; | ||
| else if (!inSearch && level === 0) currentEfConstruction = ef; |
There was a problem hiding this comment.
Nit: the new efConstruction column records the ramp's terminal value, unlabeled
currentEfConstruction is last-write-wins across the build, so on an auto run it ends up holding the efC of the final insert — the top of the ramp, not what most of the graph was built at. A default 1M run and a --ef-construction=200 run therefore both print efC= 200 and both write efConstruction: 200 into rows[]; only the header's efC=auto and the top-level JSON field separate them. Your own results comment had to hand-label the column "auto efC (final)" for exactly this reason.
This matters more than it would have before: DESIGN.md now rests on the ramp-vs-uniform comparison being absent rather than measured, which makes this harness the thing that would eventually close that gap.
Suggested fix: track the first and last level-0 efC seen during the build and emit both (e.g. efConstructionStart / efConstructionEnd), so a ramped run is self-describing in rows[] without cross-referencing the header.
—
Generated by Barber AI
Large HNSW graphs lose recall because a fixed construction candidate list eventually stops creating enough useful edges. This change auto-scales
efConstructionfrom a base of 100 asmin(1024, 100 × sqrt(nodes / 250K)), resumes the search-efscale above one million nodes up to 2048, and adds streaming benchmark support for multi-million-node validation.The write path reads the lifetime node-id high-water mark directly from the shared atomic counter. If an update-only worker cannot attach that counter, it warns once, uses a transaction-consistent memoized reverse seek, and retries after the memo TTL. New-id allocation still fails closed rather than installing a private counter.
Evidence and limits
The original 1M A/B rebuilt the same corpus uniformly at
efConstruction: 200: set-recall improved from 0.967 to 0.997 and the better graph also reduced nodes visited at the same searchef. That isolates the construction-quality problem, but it is not a direct comparison of the automatic ramp against a uniform-200 build.The automatic ramp has separate 2M/5M evidence. With search
ef: 1024, those graphs reached set-recall 0.988/0.985. That shows the measured neighbours remained reachable at those sizes; it does not prove that the ramp converges to the uniform-build result.Below 250K nodes the construction default remains 100. Nodes inserted before a scale threshold keep their existing edges. A live-process reindex starts near the lifetime high-water mark, while a reindex after restart repeats the ramp from the store's current largest id.
Search cost
Past 1M nodes, automatic search
efresumes from 512 as512 × sqrt(nodes / 1M), capped at 2048. At 5M nodes it resolves to 1,145; the measuredef: 1024point held 0.985 set-recall at about 45 ms p50 on the benchmark corpus. Applications that prefer a latency ceiling can pinefConstructionSearchor pass a per-queryef.Filtered traversal does not inherit that entire automatic increase. Its visit-budget contribution remains capped at the former automatic ceiling of 512 before multiplying by
filterExpansion; explicit schema or per-queryefstill raises the budget for callers that own the cost.Construction and delete cost
The node count is a lifetime id high-water mark, not a live count. Delete-heavy tables can therefore remain over-provisioned after the live graph shrinks. When
efConstructionexceeds the live graph size, an insert can traverse much of the graph without gaining recall. Connectivity repair can also synchronously reinsert an orphan plus up to 256 nodes from a severed island, and each repair uses the current scaled construction value.An explicit
efConstructionremains authoritative and structural, so changing it rebuilds the index. It also seeds the search default. To retain the former large-graph search default while opting out of build scaling, configure bothefConstruction: 100andefConstructionSearch: 512; there is no “pinned build, automatic search” mode.Benchmark tooling
benchmarks/hnsw-scale.jsgains a--streammode for corpora too large to hold as one float pool. It uses a dedicated replayable corpus RNG, gathers query rows in one pass, then streams the same rows through graph construction and brute-force ground truth in a second pass. Output now records the resolved construction value as well as the requested setting.For the human reviewer
LIMIT_EF_MAXrises from 2048 to 4096 with the search ceiling. Keeping 4096 avoids truncating larger requested result sets, but raises worst-case synchronous query CPU. A schema-levelefConstructionSearchpin does not prevent limit-derived widening; only an explicit per-queryefdoes. Alternatives are to retain 2048, have a schema pin cap widening, or tie widening to the graph’s resolved automaticef.Verification
npm run build— passed.npx mocha unitTests/resources/vectorIndex.test.js— 106 passing.npm run test:unit:resources— all HNSW coverage passed; suite total 1,567 passing, 15 pending, and 3 unrelated structure-format failures.npm run test:integration:all— 1,732 passing, 20 skipped, 0 asserted failures; 6 Ollama tests were cancelled after an unrelated Node JSON import-attribute error.npm run lint:requiredandnpm run format:check— passed.npm run lintreports 12 pre-existing warnings outside this diff.npm run test:unit:mainis locally blocked by the Mocha apiTests-exclusion preloader bug and an unrelated built-entrypoint shebang import failure.GitHub Actions on this head: 41 passed, 2 intentionally skipped, 0 failed.
Review coverage
Authored by Claude Fable 5; review-feedback commits by GPT-5 Codex. Cross-model review at
8e3b2cb3e: Claude (opposite-family graded review) passed and Gemini (default model) passed; Cursor and Harper-domain legs were pruned for the narrow delta. The final3792c84f7commit is comment-only and was not rerun under the trivial-change policy. The review retained four policy decisions for human confirmation rather than identifying a new implementation defect.Addresses HNSW: efConstruction does not scale with corpus size — recall plateaus at 1M vectors regardless of search ef. Follow-up: HNSW: graph-size resolution is a lifetime id high-water mark — churn-heavy tables permanently over-provision build/search ef.
Updated by GPT-5 Codex.
Human-Review-Need: 4 @ 3792c84