Repository navigation
Conversation
|
Welcome @thlurte! It looks like this is your first PR to zilliztech/knowhere 🎉 |
- Advance next_task_id_ only after successful enqueue under mutex to prevent task sequence gaps - Use RAII AddTaskGuard in SparseInvertedIndexNodeCC::Add to guarantee task release and notification on exception - Pass config by const reference to eliminate unnecessary heap allocation - Capture and rethrow exceptions across OpenMP parallel regions in ProductQuantizer to prevent std::terminate - Add Catch2 regression test forcing allocation failure during enqueue and insert, asserting bounded completion Fixes: zilliztech#1870 Signed-off-by: Ahmed <thlurte@gmail.com>
1f5de6e to
f687fd1
Compare
|
/hold |
|
|
||
| void ProductQuantizer::decode(const uint8_t* code, float* x, size_t n) const { | ||
| int64_t n_signed = n; | ||
| std::exception_ptr ex; |
There was a problem hiding this comment.
There should a change that does not affect the baseline faiss code (thirdparty/faiss), unless it is first pushed into the vanilla faiss (https://github.com/facebookresearch/faiss). So, please either introduce the change in the vanilla faiss by submitting the PR (and I'll ask Faiss team to review it quickly), or please add exception handlers outside of the baseline faiss code.
|
Hi @alexanderguzhva, As suggested, I have submitted the upstream PR directly to Faiss: It wraps the OpenMP parallel loops in Could you please help ask the Faiss team to review facebookresearch/faiss#5771? Thank you! |
…tizer Signed-off-by: Ahmed <thlurte@gmail.com>
34e69d7 to
c703b29
Compare
issue: #1870
Summary
SparseInvertedIndexNodeCC::Add):next_task_id_only afteradd_tasks_.push(task_id)succeeds under the mutex. If the push throws (e.g.std::bad_alloc), no task sequence gap is created that could block subsequent adds.AddTaskGuardimmediately after enqueue so thatadd_tasks_.pop(),current_task_id_++, andcv_.notify_all()are guaranteed to execute during stack unwinding on any exception.configbyconst auto&reference instead of copying to eliminate unnecessary allocations.ProductQuantizer):compute_codesanddecodeusingfaiss::omp_capture_exceptionand safely rethrow them viafaiss::omp_rethrow_if_exceptionafter thread join, preventingstd::terminate.TEST_CASE("Test Sparse Inverted Index CC Exception Safety on Enqueue and Add", "[sparse][cc][exception_safety]")totests/ut/test_sparse.cc.Addand asynchronousSearchcalls complete within a bounded timeout without deadlocking.Test Plan
knowhere_tests '[sparse][cc][exception_safety]': 24 assertions passed across bothSPARSE_INVERTED_INDEX_CCandSPARSE_WAND_CC.knowhere_tests 'Test SINDI Sparse Inverted Index CC*': 21 assertions passed with zero regressions.check-added-large-files,check-merge-conflict,end-of-file-fixer,trailing-whitespace, andclang-format.