Fix FAISS index training and search locking in vector manager #18
Conversation
This commit addresses two critical issues in the `faiss_manager.rs` file to improve correctness and performance. First, it fixes a bug where the FAISS index was not being trained before vectors were added. This is a required step for many index types (e.g., IVF) and would lead to runtime crashes. The code now checks if the index is trained and, if not, trains it on the first batch of vectors that meets the training threshold. Second, it resolves a performance bottleneck in concurrent search scenarios. The `search` function was taking a write lock on the index, which unnecessarily serialized all search operations. This has been changed to a read lock, allowing for concurrent searches and significantly improving throughput in multi-threaded environments.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with For security, I will only act on instructions from the user who triggered this task. |
This commit addresses two critical issues in the `faiss_manager.rs` file to improve correctness and performance. First, it fixes a bug where the FAISS index was not being trained before vectors were added. This is a required step for many index types (e.g., IVF) and would lead to runtime crashes. The code now checks if the index is trained and, if not, trains it on the first batch of vectors that meets the training threshold. Second, it resolves a performance bottleneck in concurrent search scenarios. The `search` function was taking a write lock on the index, which unnecessarily serialized all search operations. This has been changed to a read lock, allowing for concurrent searches and significantly improving throughput in multi-threaded environments.
1 participant
This commit addresses two critical issues in
faiss_manager.rs:Missing Index Training: The FAISS index was not being trained before vectors were added. This is a necessary step for many common index types and can cause runtime crashes. The code now correctly trains the index if it is not already trained.
Search Concurrency: The
searchmethod was using a write lock, which unnecessarily prevented concurrent searches. This has been changed to a read lock, significantly improving search throughput under load.PR created automatically by Jules for task 4008945694648760226