[SVS] Implement 2-stage backend SVS index initialization by rfsaliev · Pull Request #903 · RedisAI/VectorSimilarity

rfsaliev · 2026-02-02T14:06:59Z

Purpose: prevent long time SVS Tiered index lock at initialization time.

Logic:

First stage: create svs::...::MutableVamanaIndex instance with R/O shared lock
Second stage: set svs::...::MutableVamanaIndex created before under R/W unique lock

Which issues this PR fixes

#7831

Main objects this PR modified

2 methods added to SVSIndex: createImpl() and setImpl()
SVSTiered update job is modified to call createImpl() under shared lock and setImpl() under unique lock if backend index is empty.

Mark if applicable

This PR introduces API changes
This PR introduces serialization changes

Note

Medium Risk
Touches concurrency and synchronization in tiered indexing and backend initialization; regressions could surface as deadlocks, stale/deleted labels remaining searchable, or incorrect update behavior under contention.

Overview
Implements two-stage initialization for the backend SVS index to reduce long exclusive locks during tiered index updates: the SVS implementation can now be constructed via createImpl() under a shared lock and installed via setImpl() under a unique lock.

Tiered SVS update jobs were adjusted to use this path when the backend is empty, and to track and propagate deletions that happen during an update (via a new deleted-labels journal) so those labels are removed from the backend after the move. Deletion logic was tightened by adding SVSIndexBase::isLabelExists() and routing single-label deletes through a dedicated deleteVectorImpl().

Adds unit coverage for two-stage init and for deletion synchronization during an in-flight tiered update (single- and multi-value).

^{Written by Cursor Bugbot for commit 2a3d44c. This will update automatically on new commits. Configure here.}

jit-ci · 2026-02-02T14:07:08Z

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

codecov · 2026-02-02T14:29:59Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.11%. Comparing base (d5f91a8) to head (2a3d44c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #903      +/-   ##
==========================================
+ Coverage   97.09%   97.11%   +0.01%     
==========================================
  Files         129      129              
  Lines        7500     7551      +51     
==========================================
+ Hits         7282     7333      +51     
  Misses        218      218

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

alonre24

LGTM!
A few small comments.
Also please:

Validate that covering the affected scenarios in unit tests (I believe we are)
Add micro benchmarks that will prove the improvement (add vector/run query while initial index creation is executed in the background)

alonre24 · 2026-02-09T17:43:42Z

src/VecSim/algorithms/svs/svs.h

+    }
+
+    void setImpl(std::unique_ptr<ImplHandler> handler) override {
+        assert(handler && "SVSIndex::setImpl called with null handler");


If this is a debug-only assert, let's add this to the log in a warning level as well

This assert is added just to simplify logic error catching in DEBUG mode - as well as the next-line assert.
In release mode, the logic_error will be thrown later if handler is null.

assert()s here are not really needed - except for debugging purposes.
I can just remove them.

alonre24 · 2026-02-09T18:23:25Z

src/VecSim/algorithms/svs/svs.h

+        std::span<const labelType> ids(labels, n);
+        auto processed_blob = this->preprocessForBatchStorage(vectors_data, n);
+        auto typed_vectors_data = static_cast<DataType *>(processed_blob.get());
+        // Wrap data into SVS SimpleDataView for SVS API
+        auto points = svs::data::SimpleDataView<DataType>{typed_vectors_data, n, this->dim};
+


This logic seems to be a duplication of what we do in addVectorsImpl. Consider unifying these into a single function

The main point here is the processed_blob which lifetime should be managed till end of initImpl() and impl_->add_points() calls.
A single function, which will wrap all this code would look like:

std::tuple<std::span<const labelType>, MemoryUtils::unique_blob, svs::data::SimpleDataView<DataType>> preprocessAndPrepareSVSArgs(...)

alonre24 · 2026-02-09T18:36:03Z

src/VecSim/algorithms/svs/svs.h

+        SVSImplHandler *svs_handler = dynamic_cast<SVSImplHandler *>(handler.get());
+        if (!svs_handler) {
+            throw std::logic_error("Failed to cast to SVSImplHandler");
+        }


What is the motivation to have an abstract ImplHandler rather than have only SVSImplHandler? The dynamic_cast here seems a bit awkward

SVSImplHandler is not just a simple type - it is template class with a number of parameters, and it's full declaration looks like:

template <typename MetricType, typename DataType, bool isMulti, size_t QuantBits, size_t ResidualBits, bool IsLeanVec> struct SVSIndex<MetricType, DataType, isMulti, QuantBits, ResidualBits, IsLeanVec>::SVSImplHandler;

This why the abstract SVSIndexBase::ImplHandler is defined for client code (TieredSVSIndex).

meiravgri · 2026-02-10T14:55:51Z

src/VecSim/algorithms/svs/svs_tiered.h

-            svs_index->setNumThreads(std::min(availableThreads, labels_to_move.size()));
-            svs_index->addVectors(vectors_to_move.data(), labels_to_move.data(),
-                                  labels_to_move.size());
+            if (this->backendIndex->indexSize() == 0) {


Does this also handle re-initialization after the index was emptied? Is that scenario tested?

Yes, as it was before in SVSIndex::AddVectors()

src/VecSim/algorithms/svs/svs_tiered.h

jit-ci · 2026-02-17T13:10:29Z

❌ Jit Scanner failed - Our team is investigating

Jit Scanner failed - Our team has been notified and is working to resolve the issue. Please contact support if you have any questions.

💡 Need to bypass this check? Comment @sera bypass to override.

src/VecSim/algorithms/svs/svs_tiered.h

src/VecSim/algorithms/svs/svs.h

tests/benchmark/bm_vecsim_svs.h

Purpose: prevent long time SVS Tiered index lock at initialization time. Logic: First stage: create `svs::...::MutableVamanaIndex` instance with R/O shared lock Second stage: set `svs::...::MutableVamanaIndex` created before under R/W unique lock

…d index.

src/VecSim/algorithms/svs/svs_tiered.h

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

cursor · 2026-02-18T14:39:31Z

src/VecSim/algorithms/svs/svs.h

+        auto processed_blob = this->preprocessForBatchStorage(vectors_data, n);
+        auto typed_vectors_data = static_cast<DataType *>(processed_blob.get());
+        // Wrap data into SVS SimpleDataView for SVS API
+        auto points = svs::data::SimpleDataView<DataType>{typed_vectors_data, n, this->dim};


Duplicated preprocessing logic in createImpl and addVectorsImpl

Low Severity

The new createImpl method duplicates the same preprocessing sequence found in addVectorsImpl: constructing a std::span from labels, calling preprocessForBatchStorage, casting the blob to DataType *, and constructing a SimpleDataView. If the preprocessing logic changes in one place but not the other, the two code paths will silently diverge, potentially causing subtle bugs.

Additional Locations (1)

src/VecSim/algorithms/svs/svs.h#L271-L276

alonre24 · 2026-02-18T16:44:33Z

src/VecSim/algorithms/svs/svs_tiered.h

+        // delete vectors from backend index that were deleted from the frontend index during
+        // the update process.
+        {
+            std::lock_guard main_lock(this->mainIndexGuard);
+
+            std::sort(deleted_labels_during_update.begin(), deleted_labels_during_update.end());
+            auto it = std::unique(deleted_labels_during_update.begin(),
+                                  deleted_labels_during_update.end());
+            deleted_labels_during_update.erase(it, deleted_labels_during_update.end());
+            auto svs_index = GetSVSIndex();
+            svs_index->deleteVectors(deleted_labels_during_update.data(),
+                                     deleted_labels_during_update.size());
+        }


Jurnal mechianism for removing vectors that were deleted during the training looks good. Can you add a unit test for this edge case? This isn't covered in two_stage_initialization_test from what I understand

Added appropriate test to test_svs_tiered.cpp

alonre24 · 2026-02-22T10:05:32Z

tests/unit/test_svs_tiered.cpp

+        std::unique_lock lock(mtx);
+        EXPECT_TRUE(cv.wait_for(lock, std::chrono::seconds(100), [&] { return adding_to_svs; }));


Is it possible that we get here AFTER the tracing_callback called notify_one? In that case, we would wait for 100 seconds before waking up?

rfsaliev requested review from alonre24 and meiravgri February 2, 2026 14:06

alonre24 reviewed Feb 9, 2026

View reviewed changes

meiravgri reviewed Feb 10, 2026

View reviewed changes

cursor bot reviewed Feb 17, 2026

View reviewed changes

src/VecSim/algorithms/svs/svs_tiered.h Show resolved Hide resolved

rfsaliev force-pushed the rfsaliev/svs-async-create-impl branch from 23d0223 to 2538673 Compare February 17, 2026 11:23

alonre24 added the bm-svs-train-fp32 label Feb 17, 2026

cursor bot reviewed Feb 17, 2026

View reviewed changes

src/VecSim/algorithms/svs/svs_tiered.h Show resolved Hide resolved

alonre24 added the bm-basics-svs-fp32-single label Feb 17, 2026

rfsaliev requested review from alonre24 and meiravgri February 18, 2026 11:01

cursor bot reviewed Feb 18, 2026

View reviewed changes

src/VecSim/algorithms/svs/svs_tiered.h Outdated Show resolved Hide resolved

src/VecSim/algorithms/svs/svs_tiered.h Show resolved Hide resolved

rfsaliev force-pushed the rfsaliev/svs-async-create-impl branch from 33ef4e8 to ff11b80 Compare February 18, 2026 11:23

cursor bot reviewed Feb 18, 2026

View reviewed changes

src/VecSim/algorithms/svs/svs.h Show resolved Hide resolved

tests/benchmark/bm_vecsim_svs.h Outdated Show resolved Hide resolved

rfsaliev added 5 commits February 18, 2026 13:25

Avoid unique locking for deletion of non-existing vectors from backen…

45e6dca

…d index.

Simplify/improve SVSIndex::deleteVector()

adba42a

Add 2-stage initialization test

8f0dd5b

Resolve "undoing the delete" issue

e270c00

rfsaliev force-pushed the rfsaliev/svs-async-create-impl branch from ff11b80 to e270c00 Compare February 18, 2026 13:01

cursor bot reviewed Feb 18, 2026

View reviewed changes

src/VecSim/algorithms/svs/svs_tiered.h Outdated Show resolved Hide resolved

Fix lock ordering inversion issue

9862f95

cursor bot reviewed Feb 18, 2026

View reviewed changes

alonre24 reviewed Feb 18, 2026

View reviewed changes

Add test for deleted_labels_journal

2a3d44c

alonre24 reviewed Feb 22, 2026

View reviewed changes

		std::unique_lock lock(mtx);
		EXPECT_TRUE(cv.wait_for(lock, std::chrono::seconds(100), [&] { return adding_to_svs; }));

Comments

Conversation

rfsaliev commented Feb 2, 2026 • edited by cursor bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jit-ci bot commented Feb 2, 2026

Uh oh!

codecov bot commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

alonre24 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rfsaliev Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

jit-ci bot commented Feb 17, 2026

❌ Jit Scanner failed - Our team is investigating

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cursor bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor bot Feb 18, 2026

Choose a reason for hiding this comment

Duplicated preprocessing logic in createImpl and addVectorsImpl

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rfsaliev commented Feb 2, 2026 •

edited by cursor bot

Loading

codecov bot commented Feb 2, 2026 •

edited

Loading

rfsaliev Feb 16, 2026 •

edited

Loading