Concurrency hardening for BfTree parallel workloads

[JordanMaples]

Problem

NeighborProvider::append_vector performs a read-modify-write cycle (read existing neighbors → append → write back) without synchronization. Under concurrent mutation, the last writer wins and silently drops the other's edges. Stress testing shows 11-51% edge loss under 8-thread contention on a single vertex.

Additionally, the dual-store SetElement wrote quant before full-precision, meaning if the full-precision write failed after quant succeeded, it would leave a quantized ghost with no backing data. And Delete did not clean up the quant store entry.

Solution

Striped RwLock on NeighborProvider

• 16,384 stripes with Fibonacci multiply-shift hashing (⌊2⁶⁴/φ⌋)
• Constant ~128 KB memory regardless of dataset size — suitable for billion-scale workloads
• RwLock allows concurrent readers (search) while only writers serialize
• Read lock on get_neighbors, write lock on set_neighbors/append_vector/delete_vector

Why not per-vertex locks?

BfTree's value proposition is spilling to disk for datasets exceeding RAM. At 1B vectors, per-vertex RwLock<()> would consume 8 GB of in-memory locks — defeating the purpose. Striped locks keep memory constant at the cost of rare false contention (0.006% collision rate at 16K stripes).

Dual-store write ordering

• Full-precision is now written first in SetElement
• Failure mode is benign: vector reachable in full-precision but not yet in quantized search

Quant store cleanup on delete

• DeleteQuant trait with real impl for QuantVectorProvider, no-op for NoStore
• Single generic Delete impl; neighbor topology cleanup remains in the upper DiskANNIndex layer

Testing

• 3 new concurrent stress tests: - test_concurrent_append_no_lost_edges — 8 threads × 10 edges to same vertex
• test_concurrent_append_independent_vertices — 8 threads to different vertices
• test_concurrent_read_write_consistency — mixed readers + writers, validates no torn reads

[Copilot]

Pull request overview

This PR hardens diskann-bftree for concurrent workloads by synchronizing neighbor-list mutations (preventing lost updates), correcting dual-store (full + quant) lifecycle behavior, and adding concurrent stress tests to validate the new behavior.

Changes:

• Add striped per-vertex RwLock synchronization to NeighborProvider (read locks for reads, write locks for mutations) and introduce concurrent stress tests.
• Adjust hard-delete behavior to also delete quantized vectors when present via a small abstraction over the quant store.
• Reorder dual-store writes in set_element to write full-precision first, then quantized.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
diskann-bftree/src/neighbors.rs	Adds striped locking around neighbor-list read/modify/write and adds concurrent stress tests.
diskann-bftree/src/provider.rs	Adds quant-store cleanup on delete via DeleteQuant and reorders full/quant write sequencing in set_element.
diskann-bftree/src/quant.rs	Adds quant-store delete support used by hard-delete cleanup.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[harsha-simhadri]

Jordan, what is the perf impact of this change? Is ingestion speed the same as before? thanks

[codecov-commenter]

Codecov Report

❌ Patch coverage is 96.91630% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.48%. Comparing base (1bdf1a1) to head (566d5ee).
⚠️ Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
diskann-bftree/src/neighbors.rs	97.72%	4 Missing ⚠️
diskann-bftree/src/provider.rs	88.46%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1158      +/-   ##
==========================================
+ Coverage   89.46%   89.48%   +0.01%     
==========================================
  Files         487      488       +1     
  Lines       92102    92378     +276     
==========================================
+ Hits        82401    82664     +263     
- Misses       9701     9714      +13     

Flag	Coverage Δ
miri	89.48% <96.91%> (+0.01%)	⬆️
unittests	89.13% <96.91%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
diskann-bftree/src/lib.rs	47.36% <ø> (ø)
diskann-bftree/src/locks.rs	100.00% <100.00%> (ø)
diskann-bftree/src/quant.rs	89.69% <100.00%> (+0.17%)	⬆️
diskann-bftree/src/provider.rs	90.85% <88.46%> (-0.12%)	⬇️
diskann-bftree/src/neighbors.rs	95.33% <97.72%> (+1.38%)	⬆️

... and 74 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[JordanMaples]

@copilot give it another review.

[Copilot]

@copilot give it another review.

I re-reviewed and found one remaining concurrency issue: beam expansion paths were reading neighbors without taking the striped read lock. I fixed both call sites in commit 562c341 by acquiring self.provider.locks.read(n.into_usize()) before get_neighbors.

Validation run:

• cargo test -p diskann-bftree --profile ci ✅
• cargo clippy -p diskann-bftree --all-targets --config 'build.rustflags=["-Dwarnings"]' -- -D warnings ✅
• Copilot code review ✅ (no additional comments)
• CodeQL scan timed out in the validation tool (no rerun per tool guidance).

[JordanMaples]

@harsha-simhadri after chatting a bit with Mark and and changing the approach to locking in the provider. I ran a few more wiki100k streaming benchmarks on my devbox instance it looks like the perf overhead is negligible.