Skip to content

vector-store: create an in memory diskann provider#526

Merged
ewienik merged 2 commits into
scylladb:masterfrom
Akvear:diskann-in-mem-provider
Jul 24, 2026
Merged

vector-store: create an in memory diskann provider#526
ewienik merged 2 commits into
scylladb:masterfrom
Akvear:diskann-in-mem-provider

Conversation

@Akvear

@Akvear Akvear commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

In this PR a DiskANNIndex is created in a in-memory DefaultProvider mode using the parameters mapped from VsIndexConfiguration

Fixes: VECTOR-717

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an initial in-memory DiskANN-backed index factory for vector-store, enabling basic (blocking) vector insert/delete operations and plumbing it into configuration and runtime engine selection.

Changes:

  • Add a new DiskANN index implementation (vs_index::diskann) and expose a new_index_factory_diskann constructor.
  • Add DiskannAlpha configuration + env var (VECTOR_STORE_DISKANN_ALPHA) and use it to select DiskANN at runtime.
  • Add a DiskANN integration test for the /info endpoint and wire DiskANN deps into the workspace.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
README.md Documents the new VECTOR_STORE_DISKANN_ALPHA environment variable and cleans up command snippets formatting.
crates/vector-store/tests/integration/info.rs Adds an integration test asserting DiskANN engine version is reported by /info.
crates/vector-store/src/vs_index/mod.rs Exposes the new diskann index module.
crates/vector-store/src/vs_index/diskann.rs Implements an in-memory DiskANN provider + actor handling Add/Remove (search/count currently unimplemented).
crates/vector-store/src/main.rs Selects DiskANN factory when diskann_alpha is set in config.
crates/vector-store/src/lib.rs Adds DiskannAlpha, adds it to Config, and exposes new_index_factory_diskann.
crates/vector-store/src/config_manager.rs Parses VECTOR_STORE_DISKANN_ALPHA into config and adds a config loading test.
crates/vector-store/Cargo.toml Adds DiskANN crates to the vector-store crate deps.
Cargo.toml Adds git-based workspace deps for DiskANN crates.
Cargo.lock Locks DiskANN and its transitive dependencies.

Comment thread crates/vector-store/src/config_manager.rs
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 10 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

crates/vector-store/src/vs_index/diskann.rs:193

  • primary_id_to_u32 performs a lossy u64 -> u32 cast. This can cause ID collisions (and incorrect deletes/search results) whenever PrimaryId exceeds u32::MAX or when epochs differ, leading to silent index corruption.
// TODO: DiskANN's provider hardcodes `ExternalId = u32`, while vector-store keys are
// `u64` [`PrimaryId`]s (a 16-bit epoch packed with a row index). For this PoC we
// simply cast the `PrimaryId` down to `u32`, which silently truncates the upper bits
// (including the epoch) and can make distinct primary ids collide. Replace with a proper
// dense `u32 <-> PrimaryId` mapping before this is used beyond a baseline.
fn primary_id_to_u32(primary_id: PrimaryId) -> u32 {
    u64::from(primary_id) as u32
}

Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs
@Akvear
Akvear force-pushed the diskann-in-mem-provider branch 4 times, most recently from e68d4e9 to 9cc4c17 Compare July 22, 2026 11:46
@Akvear Akvear self-assigned this Jul 22, 2026
@Akvear
Akvear marked this pull request as ready for review July 22, 2026 11:51
@Akvear
Akvear requested review from QuerthDP and knowack1 July 22, 2026 13:16
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
@Akvear
Akvear force-pushed the diskann-in-mem-provider branch 2 times, most recently from 9cc4c17 to 7635092 Compare July 23, 2026 08:48
@Akvear

Akvear commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Changes:

  • Changed the order of the last 2 commits
  • Removed test imports already in production
  • Removed test helper, since it's only used in 1 place
  • Removed unused TryFrom impl

Comment thread crates/vector-store/Cargo.toml
Comment thread crates/vector-store/src/vs_index/diskann.rs
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
Comment thread crates/vector-store/src/vs_index/diskann.rs Outdated
diskann-vector: that's where distance metrics can be imported from
diskann-providers: that's where we import the default provider and it's traits/functions from
@Akvear
Akvear force-pushed the diskann-in-mem-provider branch from baf2316 to a19a80e Compare July 23, 2026 12:30
@Akvear

Akvear commented Jul 23, 2026

Copy link
Copy Markdown
Contributor Author

Changes:

  • joined the 2 troublesome commits
  • added explanation to why crates are neede

@Akvear
Akvear force-pushed the diskann-in-mem-provider branch 2 times, most recently from 52602fc to cc777c3 Compare July 23, 2026 13:12
QuerthDP
QuerthDP previously approved these changes Jul 23, 2026
@Akvear
Akvear requested a review from ewienik July 23, 2026 13:57
ewienik
ewienik previously approved these changes Jul 23, 2026
For the sake of the PoC we construct a specific
full precision provider with:
- a FastMemoryVectorProvider - the default high-performance store for f32 vectors
- NoStore - specifies that we don't store auxilary vectors next to full precision vectors
- TableDeleteProvider - default when deletes are needed

A thread-hint can be passed in DiskANNIndex::new()
Right now None is passed which means that diskann scratch object
will be created on demand, this value is the number of maximum
concurrent operations on a singular index

Mappings are specified in the SEP and go as follows:
- connectivity <-> R (pruned_degree)
- expansion_add <-> l_build
- expansion_search <-> l_value / beam_width (not needed yet)
- space_type <-> distance function (metric)

The space_type <-> metrics mappings are natural

Unit test for mapping were added to verify correctness
@Akvear
Akvear dismissed stale reviews from ewienik and QuerthDP via db91f30 July 24, 2026 08:10
@Akvear
Akvear force-pushed the diskann-in-mem-provider branch from cc777c3 to db91f30 Compare July 24, 2026 08:10
@Akvear

Akvear commented Jul 24, 2026

Copy link
Copy Markdown
Contributor Author

Changes:

  • moved the index into the async block in the new() function

@Akvear
Akvear requested a review from ewienik July 24, 2026 08:13
@ewienik
ewienik added this pull request to the merge queue Jul 24, 2026
Merged via the queue into scylladb:master with commit 18a0756 Jul 24, 2026
42 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants