Skip to content

Commit db0aa29

Browse files
authored
[8.2] [SVS] Fix SVSTiered overwriteVectorAsync and KNNSearchCosine tests (#771)
[Backport][SVS] Fix SVSTieredIndexTestBasic.overwriteVectorAsync test (#733) * Fix overwriteVectorAsync Two fixes applied 1. wrong addVector() returning value in case of overwrite 2. Data-racing for SVS index threadpool size * Another fix for addVector() * fixup! Another fix for addVector() * Fix KNNSearchCosine test fails caused by update job delays. Issue: KNNSearchCosine test sporadically failed in case when IndexSize() is called during update job running in between add-to-backend and del-from-frontend steps.
1 parent bf6eb1a commit db0aa29

File tree

2 files changed

+22
-10
lines changed

2 files changed

+22
-10
lines changed

src/VecSim/algorithms/svs/svs_tiered.h

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -507,8 +507,7 @@ class TieredSVSIndex : public VecSimTieredIndex<DataType, float> {
507507
// Release the scheduled flag to allow scheduling again
508508
index->indexUpdateScheduled.clear();
509509
// Update the SVS index
510-
index->GetSVSIndex()->setNumThreads(availableThreads);
511-
index->updateSVSIndex();
510+
index->updateSVSIndex(availableThreads);
512511
}
513512

514513
#ifdef BUILD_TESTS
@@ -528,7 +527,7 @@ class TieredSVSIndex : public VecSimTieredIndex<DataType, float> {
528527
}
529528

530529
private:
531-
void updateSVSIndex() {
530+
void updateSVSIndex(size_t availableThreads) {
532531
std::set<labelType> to_delete;
533532
std::set<labelType> to_add;
534533
// Take a snapshot of the journal to determine which vectors to delete and add.
@@ -567,6 +566,7 @@ class TieredSVSIndex : public VecSimTieredIndex<DataType, float> {
567566
std::scoped_lock lock(this->mainIndexGuard);
568567
auto svs_index = GetSVSIndex();
569568
assert(labels_to_add.size() == vectors_to_add.size() / this->frontendIndex->getDim());
569+
svs_index->setNumThreads(std::min(availableThreads, labels_to_add.size()));
570570
svs_index->addVectors(vectors_to_add.data(), labels_to_add.data(),
571571
labels_to_add.size());
572572
}
@@ -666,13 +666,18 @@ class TieredSVSIndex : public VecSimTieredIndex<DataType, float> {
666666
assert(this->getWriteMode() != VecSim_WriteInPlace && "InPlace mode returns early");
667667

668668
// Async mode - add vector to the frontend index and schedule an update job if needed.
669-
{ // Remove vector from the backend index if it exists.
669+
{
670+
{
671+
std::shared_lock lock(this->flatIndexGuard);
672+
// If the label already exists in the frontend index, we should count it
673+
// to prevent the case when existing vector is moved meanwhile by the update job.
674+
if (this->frontendIndex->isLabelExists(label)) {
675+
ret = -1;
676+
}
677+
}
678+
// Remove vector from the backend index if it exists.
670679
std::scoped_lock lock(this->mainIndexGuard);
671680
ret -= svs_index->deleteVectors(&label, 1);
672-
// If main index is empty then update_threshold is trainingTriggerThreshold,
673-
// overwise it is 1.
674-
update_threshold = this->backendIndex->indexSize() == 0 ? this->trainingTriggerThreshold
675-
: this->updateTriggerThreshold;
676681
}
677682
{ // Add vector to the frontend index and journal.
678683
std::scoped_lock lock(this->flatIndexGuard, this->journal_mutex);
@@ -681,7 +686,13 @@ class TieredSVSIndex : public VecSimTieredIndex<DataType, float> {
681686
// Check frontend index size to determine if an update job schedule is needed.
682687
frontend_index_size = this->frontendIndex->indexSize();
683688
}
684-
689+
{
690+
// If main index is empty then update_threshold is trainingTriggerThreshold,
691+
// overwise it is updateTriggerThreshold.
692+
std::shared_lock lock(this->mainIndexGuard);
693+
update_threshold = this->backendIndex->indexSize() == 0 ? this->trainingTriggerThreshold
694+
: this->updateTriggerThreshold;
695+
}
685696
if (frontend_index_size >= update_threshold) {
686697
scheduleSVSIndexUpdate();
687698
}

tests/unit/test_svs_tiered.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -644,9 +644,10 @@ TYPED_TEST(SVSTieredIndexTest, KNNSearchCosine) {
644644
}
645645
VecSimIndex_AddVector(tiered_index, f, i);
646646
}
647-
ASSERT_EQ(VecSimIndex_IndexSize(tiered_index), n);
648647

649648
mock_thread_pool.thread_pool_join();
649+
650+
ASSERT_EQ(VecSimIndex_IndexSize(tiered_index), n);
650651
// Verify that vectors were moved to SVS as expected
651652
auto sz_f = tiered_index->GetFlatIndex()->indexSize();
652653
auto sz_b = tiered_index->GetBackendIndex()->indexSize();

0 commit comments

Comments
 (0)