Add volatile multi dense vector storage#6564
Conversation
fd1d80c to
57f02bc
Compare
57f02bc to
7bc5ead
Compare
7bc5ead to
3897979
Compare
📝 WalkthroughWalkthroughThis change introduces a new in-memory, multi-dense vector storage backend called Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
lib/segment/src/vector_storage/multi_dense/volatile_multi_dense_vector_storage.rs (1)
219-227: Size estimation can be off by orders of magnitude
size_of_available_vectors_in_bytes()approximates storage size by multiplying the
total number of inner vectors withvector_dimand then scaling by the ratio of
available/total outer vectors.
Because the average number of inner vectors per outer vector may vary greatly,
this heuristic can seriously under- or over-estimate memory use.Consider computing the exact size instead (cheap for in-mem storage):
self.vectors.len() * self.vector_dim() * std::mem::size_of::<T>()or store/maintain a running
bytes_usedcounter during insertions/deletions.lib/segment/src/vector_storage/tests/test_appendable_multi_dense_vector_storage.rs (1)
290-297: Include volatile storage in the parametrised rstests for broader coverageCurrently the new volatile backend is exercised only by dedicated unit tests,
while the generic#[rstest]suites still iterate overRocksDbFloatand
AppendableMmapFloatonly. Adding the volatile variant here would let the same
edge-case matrix (delete/update/large-vector) run against all back-ends and
prevent silent divergence.- MultiDenseStorageType::RocksDbFloat, - MultiDenseStorageType::AppendableMmapFloat + MultiDenseStorageType::RocksDbFloat, + MultiDenseStorageType::AppendableMmapFloat, + // Volatile variant (treated as in-mem, use a dummy path) + // MultiDenseStorageType::VolatileFloat, // requires enum & factory wiring(You may need to promote the volatile option into
MultiDenseStorageTypeand
route it throughcreate_vector_storage.)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
lib/segment/src/index/hnsw_index/gpu/gpu_vector_storage/mod.rs(1 hunks)lib/segment/src/vector_storage/multi_dense/mod.rs(1 hunks)lib/segment/src/vector_storage/multi_dense/volatile_multi_dense_vector_storage.rs(1 hunks)lib/segment/src/vector_storage/quantized/quantized_vectors.rs(1 hunks)lib/segment/src/vector_storage/raw_scorer.rs(1 hunks)lib/segment/src/vector_storage/tests/test_appendable_multi_dense_vector_storage.rs(8 hunks)lib/segment/src/vector_storage/vector_storage_base.rs(22 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
lib/segment/src/vector_storage/multi_dense/volatile_multi_dense_vector_storage.rs (3)
lib/segment/src/vector_storage/vector_storage_base.rs (30)
distance(42-42)distance(585-623)datatype(44-44)datatype(625-663)is_on_disk(46-46)is_on_disk(667-705)total_vector_count(51-51)total_vector_count(707-745)get_vector(66-66)get_vector(747-785)get_vector_sequential(69-69)get_vector_sequential(787-825)get_vector_opt(72-72)get_vector_opt(827-865)insert_vector(74-79)insert_vector(867-934)update_from(87-91)update_from(936-994)flusher(93-93)flusher(996-1034)files(95-95)files(1036-1074)delete_vector(104-104)delete_vector(1076-1114)is_deleted_vector(107-107)is_deleted_vector(1116-1154)deleted_vector_count(123-123)deleted_vector_count(1156-1194)deleted_vector_bitslice(129-129)deleted_vector_bitslice(1196-1234)lib/segment/src/data_types/query_context.rs (1)
hardware_counter(187-189)lib/segment/src/common/operation_error.rs (1)
check_process_stopped(242-247)
⏰ Context from checks skipped due to timeout of 90000ms (13)
- GitHub Check: test-consistency
- GitHub Check: test-snapshot-operations-s3-minio
- GitHub Check: test-shard-snapshot-api-s3-minio
- GitHub Check: test-low-resources
- GitHub Check: Basic TLS/HTTPS tests
- GitHub Check: test-consensus-compose
- GitHub Check: rust-tests (ubuntu-latest)
- GitHub Check: rust-tests (windows-latest)
- GitHub Check: integration-tests
- GitHub Check: rust-tests (macos-latest)
- GitHub Check: integration-tests-consensus
- GitHub Check: lint
- GitHub Check: storage-compat-test
🔇 Additional comments (5)
lib/segment/src/vector_storage/multi_dense/mod.rs (1)
3-4: Good addition of test-only volatile storage implementation.The module declaration for
volatile_multi_dense_vector_storageis properly flagged with#[cfg(test)]to ensure it's only included in test builds. This aligns with the PR's objective to add volatile multi-dense vector storage.lib/segment/src/vector_storage/raw_scorer.rs (1)
108-113: Consistent implementation of raw scorer support for volatile multi-dense vectors.The added match arms for the new test-only variants correctly delegate to the appropriate multi-scorer implementation methods, maintaining consistency with the existing pattern for other multi-dense vector types. All variants are properly conditioned with
#[cfg(test)]to ensure they only appear in test builds.lib/segment/src/vector_storage/quantized/quantized_vectors.rs (1)
301-312: Properly integrated volatile multi-dense storage with quantization.The added match arms for the test-only variants correctly delegate to the
create_multi_implmethod, maintaining consistency with other multi-dense vector implementations. All variants are appropriately guarded with#[cfg(test)]for test-only compilation.lib/segment/src/index/hnsw_index/gpu/gpu_vector_storage/mod.rs (1)
411-425: Correct GPU vector storage integration for volatile multi-dense vectors.The implementation properly maps each new test-only variant to the corresponding constructor method:
MultiDenseVolatileusesnew_multi_f32with force_half_precision flagMultiDenseVolatileByteusesnew_multiMultiDenseVolatileHalfusesnew_multi_f16This follows the established pattern for other storage types and ensures GPU compatibility with the new volatile storage implementations. All variants are properly guarded with
#[cfg(test)].lib/segment/src/vector_storage/vector_storage_base.rs (1)
20-22: Visibility reminder: test-only variants silently disappear in release builds
VolatileMultiDenseVectorStorageand its enum variants are gated behind
#[cfg(test)]. Any code that accidentally slips outside a#[cfg(test)]block
(e.g. in benchmarks or integration tools) will fail to compile.
Please double-check all call-sites (particularly helpers in other modules) and add
#[cfg(test)]guards where necessary.
lib/segment/src/vector_storage/multi_dense/volatile_multi_dense_vector_storage.rs
Show resolved
Hide resolved
lib/segment/src/vector_storage/multi_dense/volatile_multi_dense_vector_storage.rs
Show resolved
Hide resolved
* Add volatile multi dense vector storage * Add and use volatile multi dense vector storage test
* Add volatile multi dense vector storage * Add and use volatile multi dense vector storage test
Depends on #6561
Same as #6561, but for multi dense vectors.
Tasks
devAll Submissions:
devbranch. Did you create your branch fromdev?New Feature Submissions:
cargo +nightly fmt --allcommand prior to submission?cargo clippy --all --all-featurescommand?