reading indexUpdateScheduled under the lock guard (#813)

* reading indexUpdateScheduled under the lock

* dont lock twice

Author: meiravgri

src/VecSim/algorithms/svs/svs_tiered.h

@@ -874,14 +874,17 @@ class TieredSVSIndex : public VecSimTieredIndex<DataType, float> {
     VecSimIndexDebugInfo debugInfo() const override {
         auto info = Base::debugInfo();
 
-        SvsTieredInfo svsTieredInfo = {.trainingTriggerThreshold = this->trainingTriggerThreshold,
-                                       .updateTriggerThreshold = this->updateTriggerThreshold,
-                                       .updateJobWaitTime = this->updateJobWaitTime,
-                                       .indexUpdateScheduled =
-                                           static_cast<bool>(this->indexUpdateScheduled.test())};
+        SvsTieredInfo svsTieredInfo = {
+            .trainingTriggerThreshold = this->trainingTriggerThreshold,
+            .updateTriggerThreshold = this->updateTriggerThreshold,
+            .updateJobWaitTime = this->updateJobWaitTime,
+        };
+        {
+            std::lock_guard<std::mutex> lock(this->updateJobMutex);
+            svsTieredInfo.indexUpdateScheduled =
+                this->indexUpdateScheduled.test() == VecSimBool_TRUE;
+        }
         info.tieredInfo.specificTieredBackendInfo.svsTieredInfo = svsTieredInfo;
-        // prevent parallel updates
-        std::lock_guard<std::mutex> lock(this->updateJobMutex);
         info.tieredInfo.backgroundIndexing =
             svsTieredInfo.indexUpdateScheduled && info.tieredInfo.frontendCommonInfo.indexSize > 0
                 ? VecSimBool_TRUE

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<TEST_DATA_T>(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<TEST_DATA_T>(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<labelType, idType>::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<TEST_DATA_T>(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) {