Support fine-grained SVS index by razdoburdin · Pull Request #1058 · RedisAI/VectorSimilarity

razdoburdin · 2026-09-23T08:47:13Z

Reopening of #1027
Adds support for using newly-added separate fine-grained SVS index with minor corresponding test revisions

Includes SVS 0.5.0 rc1 binaries, which will eventually be swapped out for the official release binaries after validation.

Redisearch CI failures at test_vecsim_svs.py:711 and test_vecsim_svs.py:729 come from changes in internal logic not being reflected in tests yet.

Each deletion now submit a single label consolidation job, that can change size of the reverse edges lists. This breaks the test_vecsim_svs.py:711.
Some of the submitted consolidation jobs can be unfinished before GC launch. This breaks the test_vecsim_svs.py:729.

Note

High Risk
Large changes to concurrent mutation, locking, and async job lifecycles in tiered SVS/HNSW and the SVS backend; incorrect ordering could cause duplicate/missing vectors or query skew.

Overview
Upgrades SVS integration to 0.5.0rc10 prebuilts and moves the stack onto Intel’s svs::concurrent APIs: mutable Vamana indices, SegmentedBlocked storage/graph allocators, updated CMake download URLs, a higher minimum GLIBC (2.28), and feature detection via svs/concurrent/dynamic_index.h.

Standalone SVSIndex now treats the backend as lazily created (atomic impl_, CAS on first build), uses concurrent add/delete/consolidate/compact semantics, filters no_external_id from search results, and adjusts relabel/GC/load-save paths for the new library.

Tiered SVS is reworked from periodic batch flushes to per-vector async insert jobs plus per-label consolidate jobs after deletes; backend initialization snapshots ids_to_init_ training rows, publishes the built index, then drains deferred jobs. Relabel coordinates delayed/parked insert jobs and consolidate takeover. Tiered HNSW shares the same tiered plumbing (TieredInsertJob, TieredIndex_BatchIterator, shared insert-job invalidation/removal) and adds ScopedLocks / tier-specific lock hooks for size and queries.

Smaller follow-ons: element-size estimate includes reverse-edge overhead, Python runGC() calls the index directly, and tests/flow exercise GC after tiered builds.

^{Reviewed by Cursor Bugbot for commit 720d8df. Bugbot is set up for automated code reviews on this repo. Configure here.}

* MOD-14916 Devirtualize distance + getElement on HNSW search hot path MOD-14916 / LTK perf investigation. Two virtual dispatches per HNSW candidate added between v2.10.21 and v8.2.6 account for a measurable share of the KNN regression observed in the LTK benchmarks (-38% throughput on Intel). Both are removed here with the minimum possible change. V1 - distance computation: Every calcDistance() call goes through IndexCalculatorInterface's vtable to reach DistanceCalculatorCommon, which then calls the underlying SIMD function pointer. The intermediate vtable hop is pure indirection; the concrete calculator class is fixed for the life of an index. Expose the underlying dist_func via a new pure-virtual getDistFunc() on IndexCalculatorInterface, implemented by DistanceCalculatorCommon. Cache the returned function pointer in VecSimIndexAbstract at construction time and call it directly in calcDistance(), bypassing the virtual dispatch. V2 - vector fetch: HNSWIndex::getDataByInternalId and BruteForceIndex::getDataByInternalId call this->vectors->getElement(id), which is virtual through the RawDataContainer base. DataBlocksContainer is the only concrete implementation, and this->vectors is always a DataBlocksContainer (created and owned by VecSimIndexAbstract's constructor). Use a static_cast to DataBlocksContainer* plus a qualified call to DataBlocksContainer::getElement to skip the vtable lookup. No behavior change; per-candidate distance and neighbor-fetch calls on HNSW / brute force search paths become direct function-pointer / direct-member calls. Headers in index_factories, hnsw_serializer, and brute_force_factory compile cleanly. * MOD-14916 Inline DataBlocksContainer::getElement on HNSW search hot path Follow-up to the previous V1/V2 devirt commit. The static_cast+qualified call in getDataByInternalId removed the vtable lookup but left the larger cost on the table: DataBlocksContainer::getElement was still defined in data_blocks_container.cpp, so every per-candidate neighbor fetch still paid a real out-of-line function call and a bounds-checked blocks.at() lookup. Without LTO the compiler could neither inline the body nor hoist the div/mod in the HNSW hot loop. Move the definition into the header as inline and drop the .at() bounds check to match the v2.10.21 baseline, which used unchecked operator[] and was fully inlined into processCandidate. Also add a getDistFunc() override to DistanceCalculatorDummy in test_components.cpp so BUILD_TESTS still compiles after the pure virtual added in the previous commit. (cherry picked from commit 4ca500a)

Remove null characters from end of file

…arity into lockfree

Upstream renamed the three thread-control methods on SVSIndexBase: getNumThreads -> getParallelism setNumThreads -> setParallelism getThreadPoolCapacity -> getPoolSize Adopt those names here ahead of merging upstream/main. This is a pure rename -- 32 lines across 6 files, mechanically verified by reversing the substitution and diffing against the parent commit. Method bodies deliberately keep this branch's threadpool API (size/resize/capacity), since VecSimSVSThreadPool here is still the per-index owned pool. Upstream reworked it into a process-wide singleton with thread renting, sized via VecSim_UpdateThreadPoolSize(); that is genuine divergence to reconcile in the merge, not something a rename should paper over. The point is to remove this conflict class before merging. The names collided on roughly half the affected lines without producing conflict markers, so git resolved some toward upstream and some toward here, yielding a tree that referenced methods it no longer declared. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Resolutions, by class: Thread pool. Upstream replaced the per-index owned VecSimSVSThreadPool with a process-wide singleton that rents threads, sized via VecSim_UpdateThreadPoolSize() and defaulting to parallelism 1. Took upstream throughout: the getParallelism/ setParallelism/getPoolSize bodies in svs.h (this branch's threadpool_.capacity() no longer exists), the constructor, and scheduleSVSIndexInit/GC, which now use createScheduledJobs() so the pool's reserve/release accounting stays balanced. Kept this branch's initSVSIndexWrapper as the callback -- Dmitry renamed updateSVSIndexWrapper, and the name upstream passes is no longer defined. Distance calculators. Upstream's DistanceDispatch supersedes this branch's getDistFunc()/cachedDistFunc (PR #937): same vtable-avoidance goal, but generalized to stateful calculators, which the new SQ8 DistanceCalculatorWithNorm needs, plus asymmetric query distance. Took upstream wholesale for calculator.h, vec_sim_index.h, and test_components.cpp; this branch had touched those files only for the superseded caching, and no references to the old API remain. Concurrent index. Kept this branch throughout, since that is the point of it: SVSIndexBase::addVector (upstream dropped it; svs_tiered.h still needs it), ready(), atomic num_marked_deleted, the SegmentedBlocked/concurrent-namespace retargeting in svs_utils.h and svs_extensions.h, and the write-in-place delete-then-add path. That path keeps only updateJobMutex and not upstream's added mainIndexGuard -- the concurrent backend serializes writes against readers itself. Upstream's executeInsertJob-adjacent conflict was a mis-alignment: the text it offered belongs to the batch-drain function that initSVSIndex() replaces. Carried upstream changes that would otherwise have been lost to that restructuring: the GCJob::before_run_gc tracing hook (fired before taking updateJobMutex, so a test callback cannot deadlock against it) and the empty-batch guard around the backend write in initSVSIndex(), where setParallelism(0) is not a valid request against the shared pool. Tests. Took upstream's expectations: SVSParams.num_threads is now deprecated and ignored with a warning, so deriving expected capacity from it is no longer valid. deps/ScalableVectorSearch. Restored as a proper submodule gitlink at upstream's 7786d43b, discarding commit 8265a7a's symlink into a developer's home directory, which was dangling for everyone else. Not verified by a build: this host has GCC 11.4 and no container runtime, and SVS needs GCC 13+. Audited statically instead -- no conflict markers, no orphaned references to removed APIs, and every SVSIndexBase method used in svs_tiered.h is declared in svs.h. That last check is the one the first attempt at this merge failed: half the thread-API collisions produced no conflict markers, so git resolved some lines toward each side and left the tree calling methods it no longer declared. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…/VectorSimilarity into dev/eglaser-lockfree

cursor

Stale Bugbot comment from a previous run.

cursor

Stale Bugbot comment from a previous run.

cursor

Stale Bugbot comment from a previous run.

alonre24

Thanks for 315a89b. Queries and deletes no longer wait on the initial build, and the bookkeeping for deletes and overwrites during the build looks right to me.

However, the main thread still blocks for the whole build on HSET with RediSearch's default config. Details inline. In short: initSVSIndexWrapper still holds updateJobMutex exclusively for the build, and every addVector during the build takes the direct-to-backend path, which waits on that lock.

The fix looks small: keep buffering while init runs, as main did. On main, async addVector always went to flat and only scheduled an update; flatBufferLimit only capped updateTriggerThreshold. So letting flat grow past the limit during the build is no regression vs. main.

Could you also add the test from my earlier comment (item 4)? From UpdateJob::before_add_to_svs, call addVector with flat at or above flatBufferLimit (the default case) and assert it returns within a timeout. It would have caught this.

alonre24 · 2026-10-14T09:46:33Z

-                    this->swaps_journal.emplace_back(SKIP_LABEL, id, id);
+
+        this->flatIndexGuard.lock_shared();
+        if (this->frontendIndex->indexSize() >= flat_buffer_bound) {


This is where the main thread still blocks during the initial build.

While initInProgress is set, tryBufferForTraining returns nullopt, so we get here with the backend not ready yet. At that point flat holds the whole training batch:

RediSearch passes flatBufferLimit = tieredVecSimIndexBufferLimit, which defaults to DEFAULT_BLOCK_SIZE (1024).

The training threshold defaults to 10240 for compressed indexes. Uncompressed indexes use 1024, and more vectors keep arriving before a worker picks up the init job.

So indexSize() >= flat_buffer_bound is true, and the std::shared_lock(updateJobMutex) below waits for initSVSIndexWrapper to release its exclusive lock, i.e. for the entire build.

Suggestion: restore the ready() check that was dropped here earlier. It's needed again now that tryBufferForTraining can return nullopt before the backend is ready:

if (svs_index->ready() && this->frontendIndex->indexSize() >= flat_buffer_bound) {

Writes during init then go to the buffered insert-job path. Those jobs wait on updateJobMutex on a worker, not on the main thread. Flat grows past the limit only for the duration of the build, the same as on main, where async mode never had a hard cap.

done. I added the related test as well

alonre24 · 2026-10-14T09:46:33Z

-        const bool in_backend = svs_index->isLabelExists(old_label);
-        if (!in_flat && !in_backend) {
-            return VecSimRelabel_OldLabelMissing;
+        std::shared_lock<std::shared_mutex> lock(this->updateJobMutex);


Same issue for relabel: it takes updateJobMutex shared, so a relabelVector on the main thread during the initial build waits for the whole build. Is that acceptable, or should relabel also avoid waiting on init?

I think that is acceptable.

alonre24 · 2026-10-14T09:46:33Z

-                main_shared_lock.unlock();
-                std::lock_guard lock(this->mainIndexGuard);
+                svs_index->setParallelism(1);
                svs_index->setImpl(std::move(impl));


Minor: setImpl now publishes the backend before the final exclusive flat section runs. Until that section drops the copied rows from flat, they are in both tiers:

a query can see the same single-value label from both tiers;

deleteVector on such a label removes it from flat and from the backend, and returns 2 for a single-value index. The final section's deleteVectors then finds nothing.

The window is short, but moving setImpl into the final std::lock_guard flat_lock section closes it at no real cost, since publishing is just a CAS.

good catch.
actually, moving setImpl wasn't enough. In this case there was a short window there deleted label was still visible in the backend.
I added a test for this case as well.

alonre24 · 2026-10-14T09:46:33Z

-            if (this->backendIndex->indexSize() == 0) {
-                // If backend index is empty, we need to initialize it first.
+            // Keep tracking surviving snapshot IDs; new arrivals use insert jobs.
+            initInProgress = true;


Nit: initInProgress (and initDeletedLabels.reserve() above) are written under a shared flatIndexGuard. That's safe today, because every reader of these fields holds the lock exclusively and only one init runs at a time, but it's easy to break later. Please either take the lock exclusively for these writes, make initInProgress atomic, or add a comment documenting the invariant.

A comment is added

cursor

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

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 9ce1c6d. Configure here.}

alonre24

Thanks, the init fix looks good: the main thread no longer waits on the build, setImpl is now published only after the writes made during the build are applied, and the new tests cover both. Two follow-ups inline. The first one is about worker threads getting tied up during the build.

alonre24 · 2026-10-15T15:21:33Z

+            }
+
+            // Insert job to the queue and signal the workers' updater.
+            this->submitSingleJob(new_insert_job);


Insert jobs submitted during the initial build tie up workers.

While initInProgress is set, this submits the insert job right away. executeInsertJobWrapper takes updateJobMutex shared, and initSVSIndexWrapper holds it exclusively for the whole build. So every idle worker that picks up one of these jobs blocks until the build ends.

RediSearch submits index jobs with THPOOL_PRIORITY_LOW, but priority only orders the queue; a worker that has already picked up a job stays stuck. With steady HSETs during a build that can take tens of seconds, every worker the build itself isn't using soon sits blocked on one of these jobs. Queries, which run on the same pool, then wait for the build to end. On main, writes during an update only buffered and created no jobs, so this is a regression there.

Suggestion: while initInProgress is set, still create the job and register it in labelToInsertJobs as now, but don't submit it yet. Add it to a list kept under flatIndexGuard (like initDeletedLabels). Then have initSVSIndexWrapper submit the list after it releases updateJobMutex, the same way it schedules the consolidate jobs for the deleted labels (and the way relabel resubmits parked jobs).

alonre24 · 2026-10-15T15:21:33Z

+                // with the given label.
+                if (initInProgress && !this->labelToInsertJobs.count(label)) {
+                    // A captured row has no insert job; cancel it through the journal.
+                    ret = -removeLabelFromFlat(label);


Could you add a test for this branch? It's for overwriting a label that was copied into the build and has no insert job yet. The new tests cover additions and deletes during the build, but not overwrites. I traced it by hand and it looks correct, but it's the trickiest part of the bookkeeping. Something like: pause at UpdateJob::after_snapshot, overwrite a copied label (single-value), then check that addVector returns 0 and that after init the label has exactly the new vector.

ofiryanai and others added 30 commits April 20, 2026 17:42

initial

4e0412e

optimization

3f987c1

fix threadpool

f2fe02f

minor fix

f806dca

minor cleaning

28503c2

clean up

f2596b3

remove dead code

6dc0722

fixes; tests pass

402691f

refactoring

139161f

fixes for GC

f8efbbe

copy blob before flat unlock

cfca2f5

Remove null characters from end of file

dcbdf54

Merge pull request #1 from mihaic/lockfree-fix-end

fe7dd9c

Remove null characters from end of file

fixes for gc

aeae737

Merge branch 'lockfree' of https://github.com/razdoburdin/VectorSimil…

47508f2

…arity into lockfree

fix writer starvation for compaction

07cd22f

adopt sigle label consolidation

f24187b

vecsim separate fin-grain index enabling

f4a8f52

CI fixes

baff6de

clean-up commented and unused dead code

325f7b5

harmonize test

1051095

Merge branch 'main' into dev/eglaser-lockfree

63758e1

minor revisions for CI

b8b57e3

fix build and tests

b530842

Merge branch 'dev/eglaser-lockfree' of https://github.com/ethanglaser…

7b25c70

…/VectorSimilarity into dev/eglaser-lockfree

fix ub

7e66fea

use SVS rc

7213909

refactor switch-case in relabelVector

f14687a

cursor Bot reviewed Oct 5, 2026

View reviewed changes

Comment thread src/VecSim/algorithms/svs/svs.h

Comment thread src/VecSim/algorithms/svs/svs_batch_iterator.h Outdated

razdoburdin added 2 commits October 5, 2026 05:15

remove unused index_

7578eee

reduce flatIndexGuard scope

a34064d

cursor Bot reviewed Oct 5, 2026

View reviewed changes

Comment thread src/VecSim/algorithms/svs/svs_tiered.h

Comment thread src/VecSim/algorithms/svs/svs_tiered.h

razdoburdin and others added 6 commits October 6, 2026 00:03

adopt set once for svs pimpl

5b9be33

fixes and additional tests

673174b

format

a54595b

merge main

c68edce

GC takes mainIndexGuard to protect baseclass methods

a9616ac

bump to SVS rc7

38c48eb

cursor Bot reviewed Oct 7, 2026

View reviewed changes

Comment thread src/VecSim/algorithms/svs/svs.h

Comment thread src/VecSim/algorithms/svs/svs_tiered.h

alonre24 added bm-svs-train-fp32 bm-basics-svs-fp32-single labels Oct 8, 2026

don't lock mainthread while training

315a89b

alonre24 reviewed Oct 14, 2026

View reviewed changes

razdoburdin added 5 commits October 15, 2026 04:01

buffer added vectors during initSVSIndex

6f76269

close delete-after-impl_publish window

3b2f08e

update initDeletedLabel comment

9521bcf

update relabelVector comment

9092cb6

format

9ce1c6d

cursor Bot reviewed Oct 15, 2026

View reviewed changes

Comment thread src/VecSim/algorithms/svs/svs.h

add test

46dfcd1

alonre24 reviewed Oct 15, 2026

View reviewed changes

razdoburdin and others added 4 commits October 16, 2026 02:04

don't create insert jobs while initilizing svs; add tests

24cb709

format

548930a

SVS rc8

7878b1c

SVS rc9

b455ebc

ethanglaser force-pushed the dev/eglaser-lockfree branch from 720d8df to b455ebc Compare October 16, 2026 22:14

ethanglaser mentioned this pull request Oct 16, 2026

bench fixes and SVS rc10 #1077

Draft

2 tasks

Conversation

razdoburdin commented Sep 23, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot left a comment • 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!

Uh oh!

cursor Bot left a comment • 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!

Uh oh!

cursor Bot left a comment • 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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

razdoburdin Oct 15, 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!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

razdoburdin commented Sep 23, 2026 •

edited by cursor Bot

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

razdoburdin Oct 15, 2026 •

edited

Loading