[C API] Add SVS_SYNC_KIND_FINE_GRAIN support using ConcurrentDynamicVamana by rfsaliev · Pull Request #418 · intel/ScalableVectorSearch

rfsaliev · 2026-10-09T13:23:04Z

This pull request adds support for a concurrent dynamic index ("fine-grained" synchronization) in the C API, updating both the implementation and documentation to reflect the new concurrency model. It introduces a generalized way to select storage layouts based on the concurrency mode, and updates various data builder utilities to use this new approach. The documentation and C API headers are also updated to clarify the behavior and memory usage of the new concurrent index.

Concurrent index support and synchronization model:

Added new source files and headers for concurrent and dynamic Vamana index implementations to the C API build configuration (dispatcher_concurrent_vamana.hpp/cpp, dispatcher_dynamic_vamana_copy.hpp). [1] [2]
Updated documentation and C API headers to describe the new SVS_SYNC_KIND_FINE_GRAIN mode, which uses a concurrent index allowing parallel updates and searches, and clarified the locking behavior and memory reporting for this mode. [1] [2] [3] [4]

Generalization of storage layout selection:

Introduced a new BlockKind enum and a generalized MaybeBlockedAlloc template to select between contiguous, blocked, or segmented (concurrent) storage layouts based on the concurrency mode.
Updated all data builder specialization utilities (for_simple_specializations, for_sq_specializations, for_leanvec_specializations, for_lvq_specializations) to use the new BlockKind-based selection instead of a simple boolean for blocked/unblocked storage. [1] [2] [3] [4] [5] [6]

Concurrent extension support for data builders:

Included new concurrent extension headers in data builder implementations to enable support for concurrent index layouts in LeanVec, LVQ, and scalar quantization data builders. [1] [2] [3]

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open findings

This dispatcher now registers both BlockKind::None and BlockKind::Blocked specializations under… · New
This dispatcher now registers both BlockKind::None and BlockKind::Blocked specializations under… · New
The reverse-edge memory estimate is very approximate: sizeof(std::vector<T>) doesn’t include heap… · New
Initializing min_id to 0 can permanently expand the sampled ID range when the first inserted… · New
This early return leaks storage when storage_usable(storage) is false. Please free storage… · New

What changed in this PR

Adds C API support for SVS_SYNC_KIND_FINE_GRAIN by wiring in the concurrent dynamic Vamana implementation and updating storage-layout selection, memory reporting, and docs to reflect the fine-grained concurrency model.

Changes:

Introduces concurrent dynamic Vamana dispatch/build/load/copy paths and a ConcurrentIndexVamana wrapper for C API synchronization semantics.
Generalizes dataset/graph storage layout selection via BlockKind / MaybeBlockedAlloc across all data builders and dispatchers.
Updates C API docs/headers and tests to cover fine-grained sync behavior and memory accounting (including reverse edges).

File	Description
include/svs/orchestrators/concurrent_dynamic_vamana.h	Adds logger plumbing and stream-load graph type tweaks for concurrent dynamic Vamana.
bindings/c/src/index_vamana.hpp / .cpp	Adds `ConcurrentIndexVamana` and atomic ID-range tracking for concurrent updates.
bindings/c/src/index_builder.hpp / .cpp	Selects concurrent vs regular dynamic index implementation; adds sync-kind-aware memory estimation.
bindings/c/src/dispatcher_concurrent_vamana.hpp / .cpp	New dispatcher for build/load/copy and memory estimate for concurrent dynamic Vamana.
bindings/c/src/dispatcher_dynamic_vamana_copy.hpp	New shared dynamic-index conversion layer (flavors) used by regular/concurrent paths.
bindings/c/src/dispatcher_dynamic_vamana.cpp	Refactors copy path to use shared copy dispatcher and `BlockKind`-based specialization enumeration.
bindings/c/src/dispatcher_vamana.cpp	Switches static Vamana dispatcher specialization enumeration to `BlockKind`.
bindings/c/src/allocator.hpp	Introduces `BlockKind` and `MaybeBlockedAlloc` to choose None/Blocked/Segmented layouts.
bindings/c/src/data_builder*.hpp / data_builder.hpp / data_builder.cpp	Updates specialization enumeration to `BlockKind` and threads concurrent extension headers.
bindings/c/src/svs_c.cpp	Updates memory breakdown reporting to include concurrent reverse edges as graph memory.
bindings/c/include/svs/c/svs_c.h	Documents fine-grained sync behavior and memory estimation semantics.
bindings/c/docs/C_API_Design.md	Updates public design docs for fine-grain sync, conversion, and reverse-edge memory reporting.
bindings/c/tests/c_api_dynamic_index_sync.cpp	Adds tests for sync-kind conversion, fine-grain storage compatibility, and memory reporting.
bindings/c/CMakeLists.txt	Adds new dispatcher/copy sources to the C API target.

🧠 Review effort: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

razdoburdin

I have few findings. The most critical one is a serialization of operations by NativeThreadPool being used by default.

razdoburdin · 2026-10-14T10:46:47Z

+
+    void compact(size_t batchsize) override {
+        auto lock = read_lock();
+        compact_unlocked(batchsize);


There is a deadlock vs filtered_topk_search if NativeThreadPool is used.

The compact() takes compact_mutex then threadpool takes it's use_mutex_.
The filtered_topk_searchtakes use_mutex_ than compact_mutex (iterator.next() calls lock_for_search())

Can you please provide a code suggestion to prevent internal deadlocks?
Thank you.

I think we should first decide if we still use NativeThreadPool. If a threadpool doesn't take an internal mutex, this deadlock will be fixed automatically.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

9 open findings

This dispatcher now registers both BlockKind::None and BlockKind::Blocked specializations under…
This dispatcher now registers both BlockKind::None and BlockKind::Blocked specializations under…
Initializing min_id/max_id to extreme values is unsafe if the index is ever empty or… · New
Initializing min_id to 0 can permanently expand the sampled ID range when the first inserted…
The reverse-edge memory estimate is very approximate: sizeof(std::vector<T>) doesn’t include heap…
allocator.hpp is already included near the top of this header; re-including it in the `#else //… · New
This header includes allocator.hpp multiple times (including in the disabled-feature stub… · New
This header includes allocator.hpp multiple times (including in the disabled-feature stub… · New
This header includes allocator.hpp multiple times (including in the disabled-feature stub… · New

1 resolved since last review

This early return leaks storage when storage_usable(storage) is false. Please free storage…

🧠 Review effort: Lite

…amana - Added atomic tracking for minimum and maximum IDs in DynamicIndexVamana to support concurrent operations. - Introduced ConcurrentIndexVamana to manage concurrent access and operations on the dynamic index. - Updated memory estimation functions to account for new synchronization parameters. - Implemented comprehensive tests for synchronization kind conversion and memory usage. - Created dispatcher for concurrent Vamana index build and load operations. - Added support for copying between concurrent and regular dynamic indices.

razdoburdin · 2026-10-16T10:59:09Z

+    }
+
+    void compact(size_t batchsize) override {
+        auto lock = read_lock();


Suggested change

auto lock = read_lock();

// exclusive lock prevents potential deadlock in case of custom threadpool takes internal mutex in parallel_for method

auto lock = write_lock();

razdoburdin · 2026-10-16T11:07:34Z

+/// With SVS_SYNC_KIND_GLOBAL, mutating operations (add_points, delete_points,
+/// consolidate, compact, save, set_num_threads) take an exclusive lock.
+///
+/// With SVS_SYNC_KIND_FINE_GRAIN, the handle is backed by a concurrent index that


Concurrent execution of add_points(), delete_entries() and search() are properly teste for a single query operations only. These methods are called by svs_index_dynamic_add_points, svs_index_dynamic_delete_points, svs_index_search_topk.

yuejiaointel · 2026-10-17T06:14:40Z

    SVS_SYNC_KIND_GLOBAL = 1,    ///< Index-wide reader/writer lock
-    SVS_SYNC_KIND_FINE_GRAIN = 2 ///< Currently the same as GLOBAL; reserved for
-                                 ///< finer-grained locking
+    SVS_SYNC_KIND_FINE_GRAIN = 2 ///< Concurrent index; updates run in parallel


An index built with FINE_GRAIN and loaded without sync_kind silently becomes a normal index with no locking. Could we save the sync kind and warn when the load uses a different one? A test for this case would help too.

yuejiaointel · 2026-10-17T06:26:24Z

    );
    auto vamana_algorithm = static_cast<AlgorithmVamana*>(algorithm.get());
-    return dispatch_dynamic_vamana_memory_estimate(
+    auto estimate = uses_concurrent_index(sync_kind)


Could we apply the same patter for build and load too?

yuejiaointel · 2026-10-17T21:11:36Z

@@ -237,4 +249,27 @@ void DynamicIndexVamana::set_num_threads(size_t num_threads) {
    index.set_threadpool(this->builder->pool_builder.build());


Suggested change

index.set_threadpool(this->builder->pool_builder.build());

index.set_threadpool(this->builder->pool_builder.build(sync_kind));

Should also pass sync_kind here, otherwise calling this function on a FINE_GRAIN index goes back to NativeThreadPool

yuejiaointel · 2026-10-17T21:27:51Z

    );

+    // The concurrent index's reverse edges are reported as part of the graph.
+    const size_t graph_bytes = breakdown.graph_bytes + breakdown.reverse_edges_bytes;


#369 added reverse_edges_bytes to the C++ MemoryBreakdown as its own field, could we add a reverse_edges_bytes field too/move the reverse edge bytes to graph_bytes on C++, to keep a unified design?

rfsaliev force-pushed the rfsaliev/c-api-fine-grain-sync branch from 6d21b35 to a31079e Compare October 13, 2026 13:32

rfsaliev requested a balanced review from Copilot October 13, 2026 13:34

Copilot AI reviewed Oct 13, 2026

View reviewed changes

Comment thread bindings/c/src/data_builder.cpp Outdated

Comment thread bindings/c/src/data_builder.cpp Outdated

Comment thread bindings/c/src/dispatcher_concurrent_vamana.cpp

Comment thread bindings/c/src/index_vamana.hpp Outdated

Comment thread bindings/c/tests/c_api_dynamic_index_sync.cpp

Copilot stopped reviewing on behalf of rfsaliev due to an error October 13, 2026 14:17
The job was not acquired by a runner even after multiple attempts.

rfsaliev force-pushed the rfsaliev/c-api-fine-grain-sync branch from a31079e to 5b1bd90 Compare October 13, 2026 14:58

rfsaliev requested a balanced review from Copilot October 14, 2026 12:16

razdoburdin reviewed Oct 14, 2026

View reviewed changes

Copilot started reviewing on behalf of rfsaliev October 14, 2026 12:32

View session

Copilot AI reviewed Oct 14, 2026

View reviewed changes

Comment thread bindings/c/src/index_vamana.hpp

Comment thread bindings/c/src/data_builder/leanvec.hpp Outdated

Comment thread bindings/c/src/data_builder/lvq.hpp

Comment thread bindings/c/src/data_builder/lvq.hpp Outdated

Comment thread bindings/c/src/data_builder/lvq.hpp Outdated

rfsaliev marked this pull request as ready for review October 14, 2026 13:10

rfsaliev requested review from ahuber21 and ethanglaser as code owners October 14, 2026 13:10

eshenayo mentioned this pull request Oct 14, 2026

svs: run writers concurrently on SVS_VAMANA (FINE_GRAIN), build the C API without OpenMP, gate and test the threadpool (issue #8) eshenayo/valkey-search#19

Merged

Refactor ConcurrentDynamicVamana to use logger

cfdbd7e

razdoburdin mentioned this pull request Oct 15, 2026

Fix writer starvation issue for compact mutex #424

Open

rfsaliev added 6 commits October 15, 2026 09:19

Some refactoring

38f9561

Code review fixes

6f9592d

Fix copy/ref data issue in copy_dynamic_vamana_index()

bfb7b7e

Code review s1e2

50b3d61

Force fine-grain index use QueueThreadPool instead of Native.

9bbee5f

rfsaliev force-pushed the rfsaliev/c-api-fine-grain-sync branch from a39da58 to 9bbee5f Compare October 15, 2026 16:35

razdoburdin mentioned this pull request Oct 16, 2026

Filter deleted ids from search results #429

Open

razdoburdin reviewed Oct 16, 2026

View reviewed changes

yuejiaointel reviewed Oct 17, 2026

View reviewed changes

	auto lock = read_lock();
	// exclusive lock prevents potential deadlock in case of custom threadpool takes internal mutex in parallel_for method
	auto lock = write_lock();

		@@ -237,4 +249,27 @@ void DynamicIndexVamana::set_num_threads(size_t num_threads) {
		index.set_threadpool(this->builder->pool_builder.build());

Conversation

rfsaliev commented Oct 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants