[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 tiered index locking and backend initialization/deletion flows, so concurrency/consistency bugs could surface under racey update/delete workloads despite added test coverage.

Overview
Enables two-stage initialization of the backend SVS index to reduce time spent holding the tiered index’s write lock during the initial build.

SVSIndexBase now exposes createImpl()/setImpl() (plus isLabelExists()), and SVSIndex refactors initial build into an initImpl() that returns the implementation so the tiered update job can build the SVS impl under a shared lock and install it under an exclusive lock. Tiered SVS also starts journaling labels deleted/overwritten during an update and applies those deletions to the backend after the update, and adjusts delete paths to check label existence under shared locks before taking exclusive locks. A new unit test covers the two-stage init and re-init behavior.

^{Written by Cursor Bugbot for commit e270c00. 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 (e270c00).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #903      +/-   ##
==========================================
+ Coverage   97.09%   97.11%   +0.01%     
==========================================
  Files         129      129              
  Lines        7500     7549      +49     
==========================================
+ Hits         7282     7331      +49     
  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

cursor · 2026-02-18T11:46:30Z

src/VecSim/algorithms/svs/svs.h

+
+        this->markIndexUpdate(deleted_num);
+        return deleted_num;
+    }


Missing single-threading enforcement for single-element deletions

Medium Severity

The new deleteVectorImpl doesn't enforce single-threading before calling impl_->delete_entries with a single element. The removed code from deleteVectorsImpl explicitly set numThreads to 1 when n == 1, with a comment stating "we should ensure single-threading." The addVector path has an analogous assertion (getNumThreads() == 1). The tiered callers (addVector async mode and deleteVector) also don't call setNumThreads(1) before invoking backendIndex->deleteVector, unlike the in-place addVector path which explicitly does setNumThreads(1).

Additional Locations (2)

src/VecSim/algorithms/svs/svs_tiered.h#L841-L844

src/VecSim/algorithms/svs/svs_tiered.h#L937-L940

SVS deleteVectors() operation does not utilize the threadpool - this why any TP manipulations are redundant here.

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.

cursor

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

cursor · 2026-02-18T13:24:03Z

src/VecSim/algorithms/svs/svs_tiered.h

+                auto svs_index = GetSVSIndex();
+                svs_index->deleteVectors(deleted_labels_journal.data(),
+                                         deleted_labels_journal.size());
+            }


Lock ordering inversion causes potential deadlock

High Severity

The new cleanup code in updateSVSIndex acquires mainIndexGuard exclusive (line 714) while already holding flatIndexGuard exclusive (line 706), establishing a flat → main lock order. The pre-existing in-place addVector acquires flatIndexGuard exclusive (line 796) while holding mainIndexGuard shared (line 788), establishing a main → flat lock order. This inverted ordering can deadlock when both paths run concurrently — for example, when the update job is in its cleanup phase and a concurrent in-place addVector sees an empty backend.

Additional Locations (1)

src/VecSim/algorithms/svs/svs_tiered.h#L787-L796

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 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

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

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!

alonre24 Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

rfsaliev Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

alonre24 Feb 9, 2026

Choose a reason for hiding this comment

Uh oh!

rfsaliev Feb 16, 2026

Choose a reason for hiding this comment

Uh oh!

alonre24 Feb 9, 2026

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!

meiravgri Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

rfsaliev Feb 16, 2026

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!

cursor bot Feb 18, 2026

Choose a reason for hiding this comment

Missing single-threading enforcement for single-element deletions

Uh oh!

rfsaliev Feb 18, 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!

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

Lock ordering inversion causes potential deadlock

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

rfsaliev Feb 18, 2026 •

edited

Loading