From ef314b4c7c6a5c0053b99bd2489e5ef576d3cbd1 Mon Sep 17 00:00:00 2001 From: Joan Fontanals Martinez Date: Thu, 19 Mar 2026 17:30:17 +0100 Subject: [PATCH 1/4] proposal to fix flaky test --- src/VecSim/vec_sim_tiered_index.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/VecSim/vec_sim_tiered_index.h b/src/VecSim/vec_sim_tiered_index.h index 61294b90b..43fb4fec8 100644 --- a/src/VecSim/vec_sim_tiered_index.h +++ b/src/VecSim/vec_sim_tiered_index.h @@ -423,6 +423,10 @@ VecSimDebugInfoIterator *VecSimTieredIndex::debugInfoIterato .fieldType = INFOFIELD_UINT64, .fieldValue = {FieldValue{.uintegerValue = info.tieredInfo.bufferLimit}}}); + // Acquire shared locks to safely access the sub-indexes' debug info during background indexing. + std::shared_lock flat_lock(this->flatIndexGuard); + std::shared_lock main_lock(this->mainIndexGuard); + infoIterator->addInfoField(VecSim_InfoField{ .fieldName = VecSimCommonStrings::FRONTEND_INDEX_STRING, .fieldType = INFOFIELD_ITERATOR, From 409ffffee4fee1f5f2659bb9c33fe3ac34911b19 Mon Sep 17 00:00:00 2001 From: Joan Fontanals Martinez Date: Thu, 19 Mar 2026 17:33:25 +0100 Subject: [PATCH 2/4] clarify scope --- src/VecSim/vec_sim_tiered_index.h | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/src/VecSim/vec_sim_tiered_index.h b/src/VecSim/vec_sim_tiered_index.h index 43fb4fec8..d097b1ae4 100644 --- a/src/VecSim/vec_sim_tiered_index.h +++ b/src/VecSim/vec_sim_tiered_index.h @@ -423,18 +423,22 @@ VecSimDebugInfoIterator *VecSimTieredIndex::debugInfoIterato .fieldType = INFOFIELD_UINT64, .fieldValue = {FieldValue{.uintegerValue = info.tieredInfo.bufferLimit}}}); - // Acquire shared locks to safely access the sub-indexes' debug info during background indexing. - std::shared_lock flat_lock(this->flatIndexGuard); - std::shared_lock main_lock(this->mainIndexGuard); - - infoIterator->addInfoField(VecSim_InfoField{ - .fieldName = VecSimCommonStrings::FRONTEND_INDEX_STRING, - .fieldType = INFOFIELD_ITERATOR, - .fieldValue = {FieldValue{.iteratorValue = this->frontendIndex->debugInfoIterator()}}}); + // Acquire shared locks to safely access each sub-index's debug info during background indexing. + // Only hold each lock while accessing its respective index to minimize contention. + { + std::shared_lock flat_lock(this->flatIndexGuard); + infoIterator->addInfoField(VecSim_InfoField{ + .fieldName = VecSimCommonStrings::FRONTEND_INDEX_STRING, + .fieldType = INFOFIELD_ITERATOR, + .fieldValue = {FieldValue{.iteratorValue = this->frontendIndex->debugInfoIterator()}}}); + } - infoIterator->addInfoField(VecSim_InfoField{ - .fieldName = VecSimCommonStrings::BACKEND_INDEX_STRING, - .fieldType = INFOFIELD_ITERATOR, - .fieldValue = {FieldValue{.iteratorValue = this->backendIndex->debugInfoIterator()}}}); + { + std::shared_lock main_lock(this->mainIndexGuard); + infoIterator->addInfoField(VecSim_InfoField{ + .fieldName = VecSimCommonStrings::BACKEND_INDEX_STRING, + .fieldType = INFOFIELD_ITERATOR, + .fieldValue = {FieldValue{.iteratorValue = this->backendIndex->debugInfoIterator()}}}); + } return infoIterator; }; From 1425e0862e60b421623a78770bb3bc198875dc6a Mon Sep 17 00:00:00 2001 From: meiravgri Date: Mon, 23 Mar 2026 10:40:39 +0000 Subject: [PATCH 3/4] fiox preferAdHocSearch and add comment to indexLabelCount() --- src/VecSim/algorithms/svs/svs.h | 2 ++ src/VecSim/vec_sim_tiered_index.h | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 8b03514f0..96a3a6e06 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -376,6 +376,8 @@ class SVSIndex : public VecSimIndexAbstract, fl return impl_ ? storage_traits_t::storage_capacity(impl_->view_data()) : 0; } + // WARNING: Not thread-safe. When used within a tiered index, the caller MUST hold + // mainIndexGuard to avoid crashes caused by concurrent index modifications. size_t indexLabelCount() const override { if constexpr (isMulti) { return impl_ ? impl_->labelcount() : 0; diff --git a/src/VecSim/vec_sim_tiered_index.h b/src/VecSim/vec_sim_tiered_index.h index d097b1ae4..ed31658a8 100644 --- a/src/VecSim/vec_sim_tiered_index.h +++ b/src/VecSim/vec_sim_tiered_index.h @@ -147,6 +147,10 @@ class VecSimTieredIndex : public VecSimIndexInterface { bool preferAdHocSearch(size_t subsetSize, size_t k, bool initial_check) const override { // For now, decide according to the bigger index. + // Hold locks to safely access both indexes - backend's preferAdHocSearch may call + // indexLabelCount() which requires synchronization with concurrent modifications. + std::shared_lock flat_lock(this->flatIndexGuard); + std::shared_lock main_lock(this->mainIndexGuard); return this->backendIndex->indexSize() > this->frontendIndex->indexSize() ? this->backendIndex->preferAdHocSearch(subsetSize, k, initial_check) : this->frontendIndex->preferAdHocSearch(subsetSize, k, initial_check); From 56d0b33cc13cbf1a59205a660532616c65c8fb8b Mon Sep 17 00:00:00 2001 From: meiravgri Date: Wed, 25 Mar 2026 17:11:03 +0200 Subject: [PATCH 4/4] revert preferAdHocSearch --- src/VecSim/algorithms/svs/svs.h | 2 -- src/VecSim/vec_sim_tiered_index.h | 4 ---- 2 files changed, 6 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs.h b/src/VecSim/algorithms/svs/svs.h index 96a3a6e06..8b03514f0 100644 --- a/src/VecSim/algorithms/svs/svs.h +++ b/src/VecSim/algorithms/svs/svs.h @@ -376,8 +376,6 @@ class SVSIndex : public VecSimIndexAbstract, fl return impl_ ? storage_traits_t::storage_capacity(impl_->view_data()) : 0; } - // WARNING: Not thread-safe. When used within a tiered index, the caller MUST hold - // mainIndexGuard to avoid crashes caused by concurrent index modifications. size_t indexLabelCount() const override { if constexpr (isMulti) { return impl_ ? impl_->labelcount() : 0; diff --git a/src/VecSim/vec_sim_tiered_index.h b/src/VecSim/vec_sim_tiered_index.h index ed31658a8..d097b1ae4 100644 --- a/src/VecSim/vec_sim_tiered_index.h +++ b/src/VecSim/vec_sim_tiered_index.h @@ -147,10 +147,6 @@ class VecSimTieredIndex : public VecSimIndexInterface { bool preferAdHocSearch(size_t subsetSize, size_t k, bool initial_check) const override { // For now, decide according to the bigger index. - // Hold locks to safely access both indexes - backend's preferAdHocSearch may call - // indexLabelCount() which requires synchronization with concurrent modifications. - std::shared_lock flat_lock(this->flatIndexGuard); - std::shared_lock main_lock(this->mainIndexGuard); return this->backendIndex->indexSize() > this->frontendIndex->indexSize() ? this->backendIndex->preferAdHocSearch(subsetSize, k, initial_check) : this->frontendIndex->preferAdHocSearch(subsetSize, k, initial_check);