Skip to content

Commit 2986fed

Browse files
[0.8] Fix delete in-place multi swap (#539)
Fix delete in-place multi swap (#538) fix bug in multi (cherry picked from commit a4015a5) Co-authored-by: alonre24 <alon.reshef@redis.com>
1 parent 921e66c commit 2986fed

File tree

3 files changed

+29
-3
lines changed

3 files changed

+29
-3
lines changed

src/VecSim/algorithms/hnsw/hnsw_tiered.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -495,15 +495,17 @@ int TieredHNSWIndex<DataType, DistType>::deleteLabelFromHNSWInplace(labelType la
495495
auto *hnsw_index = this->getHNSWIndex();
496496

497497
auto ids = hnsw_index->getElementIds(label);
498-
hnsw_index->removeLabel(label);
499-
// dispose pending repair and swap jobs for the removed ids.
498+
// Dispose pending repair and swap jobs for the removed ids.
500499
vecsim_stl::vector<idType> idsToRemove(this->allocator);
501500
idsToRemove.reserve(ids.size());
502501
readySwapJobs += ids.size(); // account for the current ids that are going to be removed.
503-
for (auto id : ids) {
502+
for (size_t id_ind = 0; id_ind < ids.size(); id_ind++) {
503+
// Get the id in every iteration, since the ids can be swapped in every iteration.
504+
idType id = hnsw_index->getElementIds(label).at(id_ind);
504505
hnsw_index->removeVectorInPlace(id);
505506
this->executeSwapJob(id, idsToRemove);
506507
}
508+
hnsw_index->removeLabel(label);
507509
for (idType id : idsToRemove) {
508510
idToSwapJob.erase(id);
509511
}

src/VecSim/algorithms/hnsw/hnsw_tiered_tests_friends.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_runGCAPI_Test)
5151
INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_FitMemoryTest_Test)
5252
INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_deleteBothAsyncAndInplace_Test)
5353
INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_deleteBothAsyncAndInplaceMulti_Test)
54+
INDEX_TEST_FRIEND_CLASS(HNSWTieredIndexTestBasic_deleteInplaceMultiSwapId_Test)
5455

5556
friend class BF16TieredTest;
5657
friend class FP16TieredTest;

tests/unit/test_hnsw_tiered.cpp

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3655,3 +3655,26 @@ TYPED_TEST(HNSWTieredIndexTestBasic, deleteBothAsyncAndInplaceMulti) {
36553655
// Id 1 is now ready due to the deletion of 0 and its associated jobs.
36563656
ASSERT_EQ(tiered_index->readySwapJobs, 1);
36573657
}
3658+
3659+
TYPED_TEST(HNSWTieredIndexTestBasic, deleteInplaceMultiSwapId) {
3660+
// Create TieredHNSW index instance with a mock queue.
3661+
size_t dim = 4;
3662+
HNSWParams params = {
3663+
.type = TypeParam::get_index_type(), .dim = dim, .metric = VecSimMetric_L2, .multi = true};
3664+
VecSimParams hnsw_params = CreateParams(params);
3665+
3666+
auto mock_thread_pool = tieredIndexMock();
3667+
3668+
auto *tiered_index = this->CreateTieredHNSWIndex(hnsw_params, mock_thread_pool);
3669+
auto allocator = tiered_index->getAllocator();
3670+
3671+
// Insert three vector to HNSW - first and last under the same label.
3672+
GenerateAndAddVector<TEST_DATA_T>(tiered_index->backendIndex, dim, 0);
3673+
GenerateAndAddVector<TEST_DATA_T>(tiered_index->backendIndex, dim, 1);
3674+
GenerateAndAddVector<TEST_DATA_T>(tiered_index->backendIndex, dim, 0);
3675+
tiered_index->setWriteMode(VecSim_WriteInPlace);
3676+
// Delete in-place and validate that the second id of label 0 swapped properly with the last id
3677+
// before the deletion, and that eventually entry point was set correctly,
3678+
ASSERT_EQ(tiered_index->deleteVector(0), 2);
3679+
ASSERT_EQ(tiered_index->getHNSWIndex()->safeGetEntryPointState().first, 0);
3680+
}

0 commit comments

Comments
 (0)