perf(core): replace Mutex with RwLock in HnswBackend for highly concurrent reads#12
perf(core): replace Mutex with RwLock in HnswBackend for highly concurrent reads#12
Conversation
… concurrent reads
There was a problem hiding this comment.
Pull request overview
This PR improves read-side concurrency in the HNSW vector index backend by switching the shared usearch::Index guard from a Mutex to an RwLock, allowing multiple concurrent searches while still requiring exclusive access for mutations.
Changes:
- Replace
Arc<Mutex<usearch::Index>>withArc<RwLock<usearch::Index>>inHnswBackend. - Update lock acquisition to
read()for search/len/save andwrite()for add/remove.
Comments suppressed due to low confidence (2)
crates/cortexadb-core/src/index/hnsw.rs:170
save_to_file()now takes a sharedread()lock. Unlike the previousMutex, this allows multiple concurrentsave_to_file()calls to proceed in parallel and potentially write to the same path at the same time, risking file corruption/partial writes. Consider taking awrite()lock here (or adding a separate persistence lock) to preserve exclusive access during serialization to disk.
pub fn save_to_file(&self, path: &Path) -> Result<()> {
let index = self.index.read().map_err(|_| HnswError::LockError)?;
if let Some(parent) = path.parent() {
std::fs::create_dir_all(parent)?;
}
let path_str = path.to_string_lossy().to_string();
index.save(&path_str).map_err(|e| HnswError::UsearchError(e.to_string()))?;
Ok(())
crates/cortexadb-core/src/index/hnsw.rs:129
search()checksindex.capacity() == 0to detect an empty index, butcapacity()reflects reserved/allocated capacity (andnew()callsreserve(10000)), so this will be non-zero even when there are 0 vectors. That meansNoVectorsmay never be returned and callers (e.g.,VectorIndex::search_scoped) can incorrectly treat an empty HNSW index as a successful search and skip the exact-search fallback. Useindex.size() == 0(or equivalent) for the emptiness check, or explicitly treat an empty result set asNoVectorswhensize() == 0.
let index = self.index.read().map_err(|_| HnswError::LockError)?;
if index.capacity() == 0 {
return Err(HnswError::NoVectors);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| pub fn len(&self) -> usize { | ||
| self.index.lock().map(|i| i.size()).unwrap_or(0) | ||
| self.index.read().map(|i| i.size()).unwrap_or(0) |
There was a problem hiding this comment.
len() silently returns 0 if the RwLock is poisoned. This hides a serious synchronization failure and can make the system behave as if the index is empty. Consider returning a Result<usize> (propagating HnswError::LockError) or at least logging the poison error rather than defaulting to 0.
| self.index.read().map(|i| i.size()).unwrap_or(0) | |
| match self.index.read() { | |
| Ok(index) => index.size(), | |
| Err(e) => { | |
| eprintln!("HnswIndex RwLock poisoned in len(): {}", e); | |
| 0 | |
| } | |
| } |
Description
This PR addresses a critical read concurrency bottleneck in the vector search engine.
Previously,
usearch::Indexwas wrapped in aMutex, forcing all concurrent search queries (db.ask()) to execute sequentially. By replacing it with anRwLock,CortexaDBcan now handle massively parallel semantic searches simultaneously, only acquiring exclusive locks during embedding insertions/deletions.Changes
crates/cortexadb-core/src/index/hnsw.rs: ReplacedMutexwithRwLock.lock()calls toread()(for searches and len) orwrite()(for insertions and removals).