From 0374363a1e7ff08462fa34d2b44608ebcdfd3ea6 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Sun, 26 Oct 2025 16:51:38 +0000 Subject: [PATCH 1/2] reading indexUpdateScheduled under the lock --- src/VecSim/algorithms/svs/svs_tiered.h | 15 +++-- tests/unit/test_svs_tiered.cpp | 79 +++++++++++--------------- 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index a8fb8594b..5aa4fc963 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -874,11 +874,16 @@ class TieredSVSIndex : public VecSimTieredIndex { VecSimIndexDebugInfo debugInfo() const override { auto info = Base::debugInfo(); - SvsTieredInfo svsTieredInfo = {.trainingTriggerThreshold = this->trainingTriggerThreshold, - .updateTriggerThreshold = this->updateTriggerThreshold, - .updateJobWaitTime = this->updateJobWaitTime, - .indexUpdateScheduled = - static_cast(this->indexUpdateScheduled.test())}; + SvsTieredInfo svsTieredInfo = { + .trainingTriggerThreshold = this->trainingTriggerThreshold, + .updateTriggerThreshold = this->updateTriggerThreshold, + .updateJobWaitTime = this->updateJobWaitTime, + }; + { + std::lock_guard lock(this->updateJobMutex); + svsTieredInfo.indexUpdateScheduled = + this->indexUpdateScheduled.test() == VecSimBool_TRUE; + } info.tieredInfo.specificTieredBackendInfo.svsTieredInfo = svsTieredInfo; // prevent parallel updates std::lock_guard lock(this->updateJobMutex); diff --git a/tests/unit/test_svs_tiered.cpp b/tests/unit/test_svs_tiered.cpp index d11e3b021..512ba06cd 100644 --- a/tests/unit/test_svs_tiered.cpp +++ b/tests/unit/test_svs_tiered.cpp @@ -500,9 +500,11 @@ TYPED_TEST(SVSTieredIndexTest, CreateIndexInstance) { ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 1); } -TYPED_TEST(SVSTieredIndexTest, addVector) { +TYPED_TEST(SVSTieredIndexTest, background_indexing_check) { // Create TieredSVS index instance with a mock queue. - size_t dim = 4; + size_t dim = 2; + constexpr size_t training_th = DEFAULT_BLOCK_SIZE; + constexpr size_t update_th = DEFAULT_BLOCK_SIZE; SVSParams params = {.type = TypeParam::get_index_type(), .dim = dim, .metric = VecSimMetric_L2, @@ -511,59 +513,42 @@ TYPED_TEST(SVSTieredIndexTest, addVector) { auto mock_thread_pool = tieredIndexMock(); - auto tiered_params = this->CreateTieredSVSParams(svs_params, mock_thread_pool, 1, 1); + auto tiered_params = + this->CreateTieredSVSParams(svs_params, mock_thread_pool, training_th, update_th); auto *tiered_index = this->CreateTieredSVSIndex(tiered_params, mock_thread_pool); ASSERT_INDEX(tiered_index); - // Get the allocator from the tiered index. - auto allocator = tiered_index->getAllocator(); + mock_thread_pool.init_threads(); - BFParams bf_params = {.type = TypeParam::get_index_type(), - .dim = dim, - .metric = VecSimMetric_L2, - .multi = TypeParam::isMulti()}; + for (size_t i = 0; i < training_th; i++) { + TEST_DATA_T vector[dim]; + GenerateVector(vector, dim, i); + VecSimIndex_AddVector(tiered_index, vector, i); + } - size_t expected_mem = TieredFactory::EstimateInitialSize(&tiered_params); - ASSERT_LE(expected_mem, tiered_index->getAllocationSize()); - ASSERT_GE(expected_mem * 1.02, tiered_index->getAllocationSize()); - ASSERT_EQ(mock_thread_pool.jobQ.size(), 0); + while (tiered_index->debugInfo().tieredInfo.backgroundIndexing != VecSimBool_FALSE) { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } - // Create a vector and add it to the tiered index. - labelType vec_label = 1; - TEST_DATA_T vector[dim]; - GenerateVector(vector, dim, vec_label); - VecSimIndex_AddVector(tiered_index, vector, vec_label); - // Validate that the vector was inserted to the flat buffer properly. - ASSERT_EQ(tiered_index->indexSize(), 1); - ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 1); - ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 0); - ASSERT_EQ(tiered_index->GetFlatIndex()->indexCapacity(), DEFAULT_BLOCK_SIZE); - ASSERT_EQ(tiered_index->indexCapacity(), DEFAULT_BLOCK_SIZE); - ASSERT_EQ(tiered_index->GetFlatIndex()->getDistanceFrom_Unsafe(vec_label, vector), 0); - ASSERT_EQ(mock_thread_pool.jobQ.size(), mock_thread_pool.thread_pool_size); + ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), training_th); + ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 0); + ASSERT_EQ(tiered_index->indexSize(), training_th); - // Account for the allocation of a new block due to the vector insertion. - expected_mem += (BruteForceFactory::EstimateElementSize(&bf_params)) * DEFAULT_BLOCK_SIZE; - // Account for the memory that was allocated in the labelToId map (approx.) - expected_mem += sizeof(vecsim_stl::unordered_map::value_type) + - sizeof(void *) + sizeof(size_t); - // Account for the insert job that was created. - expected_mem += - SVSMultiThreadJob::estimateSize(mock_thread_pool.thread_pool_size) + sizeof(size_t); - auto actual_mem = tiered_index->getAllocationSize(); - ASSERT_GE(expected_mem * 1.02, tiered_index->getAllocationSize()); - ASSERT_LE(expected_mem, tiered_index->getAllocationSize()); - - if constexpr (TypeParam::isMulti()) { - // Add another vector under the same label - VecSimIndex_AddVector(tiered_index, vector, vec_label); - ASSERT_EQ(tiered_index->indexSize(), 2); - ASSERT_EQ(tiered_index->indexLabelCount(), 1); - ASSERT_EQ(tiered_index->GetBackendIndex()->indexSize(), 0); - ASSERT_EQ(tiered_index->GetFlatIndex()->indexSize(), 2); - // Validate that there still 1 update jobs set - ASSERT_EQ(mock_thread_pool.jobQ.size(), mock_thread_pool.thread_pool_size); + constexpr size_t second_batch = 2500; + + for (size_t i = 0; i < second_batch; i++) { + TEST_DATA_T vector[dim]; + GenerateVector(vector, dim, i); + VecSimIndex_AddVector(tiered_index, vector, training_th + i); } + + while (tiered_index->debugInfo().tieredInfo.backgroundIndexing != VecSimBool_FALSE) { + std::this_thread::sleep_for(std::chrono::milliseconds(100)); + } + + ASSERT_GT(tiered_index->GetBackendIndex()->indexSize(), training_th + second_batch / update_th); + ASSERT_LT(tiered_index->GetFlatIndex()->indexSize(), update_th); + ASSERT_EQ(tiered_index->indexSize(), second_batch + training_th); } TYPED_TEST(SVSTieredIndexTest, insertJob) { From 6d446ec52ff14135be67be30f72cfe3ad0395225 Mon Sep 17 00:00:00 2001 From: meiravgri Date: Mon, 27 Oct 2025 08:25:10 +0000 Subject: [PATCH 2/2] dont lock twice --- src/VecSim/algorithms/svs/svs_tiered.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/VecSim/algorithms/svs/svs_tiered.h b/src/VecSim/algorithms/svs/svs_tiered.h index 5aa4fc963..c31c3d757 100644 --- a/src/VecSim/algorithms/svs/svs_tiered.h +++ b/src/VecSim/algorithms/svs/svs_tiered.h @@ -885,8 +885,6 @@ class TieredSVSIndex : public VecSimTieredIndex { this->indexUpdateScheduled.test() == VecSimBool_TRUE; } info.tieredInfo.specificTieredBackendInfo.svsTieredInfo = svsTieredInfo; - // prevent parallel updates - std::lock_guard lock(this->updateJobMutex); info.tieredInfo.backgroundIndexing = svsTieredInfo.indexUpdateScheduled && info.tieredInfo.frontendCommonInfo.indexSize > 0 ? VecSimBool_TRUE