From 3144cbd27fc5a8d112495635721f4a88a95d8544 Mon Sep 17 00:00:00 2001 From: generall Date: Thu, 23 Jul 2026 16:56:29 +0200 Subject: [PATCH] Support memory placement in service-level storage config defaults Follow-up to #9684: `storage.payload.memory` and `storage.collection.vectors.memory` set service-wide placement defaults for newly created collections, deprecating `storage.on_disk_payload` and `storage.collection.vectors.on_disk`. Defaults resolve as: request `memory` > request legacy flag > service `memory` > service legacy flag; exactly one level is filled to avoid spurious memory-vs-legacy mismatch warnings. Co-Authored-By: Claude Fable 5 --- config/config.yaml | 12 +++++ lib/segment/src/types.rs | 23 +++++++++ .../content_manager/toc/create_collection.rs | 49 ++++++++++++++++--- lib/storage/src/types.rs | 13 ++++- lib/storage/tests/integration/alias_tests.rs | 1 + src/tonic/api/storage_read_api/tests.rs | 5 ++ 6 files changed, 94 insertions(+), 9 deletions(-) diff --git a/config/config.yaml b/config/config.yaml index a610259f4d..62120b7b71 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -34,6 +34,7 @@ storage: # If null, temporary snapshots are stored in: storage/snapshots_temp/ temp_path: null + # Deprecated: use `payload.memory` instead. # If true - point payloads will not be stored in memory. # It will be read from the disk every time it is requested. # This setting saves RAM by (slightly) increasing the response time. @@ -42,6 +43,12 @@ storage: # Default: true on_disk_payload: true + # Default payload storage configuration for newly created collections. + # Overrides the deprecated `on_disk_payload` flag if both are set. + # payload: + # # Memory placement of the payload storage: cold or cached. + # memory: cold + # Load-time memory mode. Only affects how segments are loaded on startup; # does not modify any persisted configuration. Intended as a recovery knob # when a node crash-loops on out-of-memory. @@ -212,9 +219,14 @@ storage: # Default parameters for vectors. vectors: + # Deprecated: use `memory` instead. # Whether vectors should be stored in memory or on disk. on_disk: null + # Memory placement of the vector storage: cold or cached. + # Overrides the deprecated `on_disk` flag if both are set. + # memory: null + # shard_number_per_node: 1 # Default quantization configuration. diff --git a/lib/segment/src/types.rs b/lib/segment/src/types.rs index 219387f7db..fb9e233799 100644 --- a/lib/segment/src/types.rs +++ b/lib/segment/src/types.rs @@ -704,8 +704,31 @@ impl Validate for IdfCorpusParams { /// Configuration for vectors. #[derive(Debug, Deserialize, Validate, Clone, PartialEq, Eq)] pub struct VectorsConfigDefaults { + /// Deprecated: use `memory` instead. #[serde(default)] + #[deprecated(since = "1.19.0", note = "Use `memory` instead")] pub on_disk: Option, + /// Default memory placement of the original vector storage for newly created collections. + /// Overrides the deprecated `on_disk` flag if both are set. `pinned` is not supported for + /// dense vector storage. + #[serde(default)] + #[validate(custom(function = "validate_dense_vector_memory"))] + pub memory: Option, +} + +/// Reject memory placements not supported by dense vector storage. +/// `validator` unwraps `Option` before calling, so we receive `&Memory`. +fn validate_dense_vector_memory(memory: &Memory) -> Result<(), ValidationError> { + match memory { + Memory::Cold | Memory::Cached => Ok(()), + Memory::Pinned => { + let mut error = ValidationError::new("unsupported_memory_placement"); + error.message = Some(Cow::from( + "`pinned` memory placement is not supported for dense vector storage", + )); + Err(error) + } + } } /// Vector index configuration diff --git a/lib/storage/src/content_manager/toc/create_collection.rs b/lib/storage/src/content_manager/toc/create_collection.rs index 8ed8013ad6..926d6b7960 100644 --- a/lib/storage/src/content_manager/toc/create_collection.rs +++ b/lib/storage/src/content_manager/toc/create_collection.rs @@ -6,12 +6,15 @@ use std::num::NonZeroU32; use std::sync::Arc; use collection::collection::Collection; -use collection::config::{self, CollectionConfigInternal, CollectionParams, ShardingMethod}; +use collection::config::{ + self, CollectionConfigInternal, CollectionParams, PayloadStorageParams, ShardingMethod, +}; use collection::operations::config_diff::DiffConfig as _; -use collection::operations::types::{CollectionResult, VectorsConfig}; +use collection::operations::types::{CollectionResult, VectorParams, VectorsConfig}; use collection::shards::collection_shard_distribution::CollectionShardDistribution; use collection::shards::replica_set::replica_set_state::ReplicaState; use collection::shards::shard::{PeerId, ShardId}; +use segment::types::VectorsConfigDefaults; use super::{COLLECTION_DELETE_SPIN_INTERVAL, COLLECTION_DELETE_WAIT_TIMEOUT, TableOfContent}; use crate::common::utils::try_unwrap_with_timeout_async; @@ -20,6 +23,37 @@ use crate::content_manager::collections_ops::Checker as _; use crate::content_manager::consensus_ops::ConsensusOperations; use crate::content_manager::errors::StorageError; +/// Fill exactly one placement level, by precedence: request `memory`, request legacy `on_disk`, +/// default `memory`, default `on_disk`. Filling a lower level alongside a higher one would cause +/// spurious `memory`-vs-legacy mismatch warnings at resolution time. +fn apply_vector_placement_defaults(params: &mut VectorParams, defaults: &VectorsConfigDefaults) { + let VectorsConfigDefaults { on_disk, memory } = defaults; + if params.memory.is_some() || params.on_disk.is_some() { + return; + } + if memory.is_some() { + params.memory = *memory; + } else { + params.on_disk = *on_disk; + } +} + +/// Service-level `payload.memory` default applies unless the request specifies `payload.memory` +/// or the legacy `on_disk_payload` flag. +fn apply_payload_placement_defaults( + payload: Option, + on_disk_payload: Option, + defaults: Option, +) -> Option { + if on_disk_payload.is_some() { + return payload; + } + match (defaults, payload) { + (Some(defaults), Some(payload)) => Some(defaults.update(&payload)), + (defaults, payload) => payload.or(defaults), + } +} + impl TableOfContent { pub(super) async fn create_collection( &self, @@ -120,20 +154,19 @@ impl TableOfContent { if let Some(vectors_defaults) = vectors_defaults { match &mut vectors { VectorsConfig::Single(s) => { - if let Some(on_disk_default) = vectors_defaults.on_disk { - s.on_disk.get_or_insert(on_disk_default); - } + apply_vector_placement_defaults(s, vectors_defaults); } VectorsConfig::Multi(m) => { for vec_params in m.values_mut() { - if let Some(on_disk_default) = vectors_defaults.on_disk { - vec_params.on_disk.get_or_insert(on_disk_default); - } + apply_vector_placement_defaults(vec_params, vectors_defaults); } } }; } + let payload = + apply_payload_placement_defaults(payload, on_disk_payload, self.storage_config.payload); + let collection_params = CollectionParams { vectors, sparse_vectors, diff --git a/lib/storage/src/types.rs b/lib/storage/src/types.rs index 559c009273..b55ec901d1 100644 --- a/lib/storage/src/types.rs +++ b/lib/storage/src/types.rs @@ -1,3 +1,7 @@ +// Deprecated storage placement params (`on_disk`, `always_ram`, `on_disk_payload`) are still +// handled here for backward compatibility with the new `memory` parameter +#![allow(deprecated)] + use std::collections::HashMap; use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; @@ -5,7 +9,7 @@ use std::time::Duration; use chrono::{DateTime, Utc}; use collection::common::snapshots_manager::SnapshotsConfig; -use collection::config::{WalConfig, default_on_disk_payload}; +use collection::config::{PayloadStorageParams, WalConfig, default_on_disk_payload}; use collection::operations::config_diff::OptimizersConfigDiff; use collection::operations::shared_storage_config::{ DEFAULT_IO_SHARD_TRANSFER_LIMIT, DEFAULT_SNAPSHOTS_PATH, SharedStorageConfig, @@ -76,8 +80,15 @@ pub struct StorageConfig { #[validate(custom(function = validate_path))] #[serde(default)] pub temp_path: Option, + /// Deprecated: use `payload.memory` instead. #[serde(default = "default_on_disk_payload")] + #[deprecated(since = "1.19.0", note = "Use `payload.memory` instead")] pub on_disk_payload: bool, + /// Default configuration of the payload storage for newly created collections. + /// Overrides the deprecated `on_disk_payload` flag if both are set. + #[serde(default)] + #[validate(nested)] + pub payload: Option, #[validate(nested)] pub optimizers: OptimizersConfig, #[validate(nested)] diff --git a/lib/storage/tests/integration/alias_tests.rs b/lib/storage/tests/integration/alias_tests.rs index 91265c02e3..e1a7d7a66d 100644 --- a/lib/storage/tests/integration/alias_tests.rs +++ b/lib/storage/tests/integration/alias_tests.rs @@ -36,6 +36,7 @@ fn test_alias_operation() { snapshots_config: Default::default(), temp_path: None, on_disk_payload: false, + payload: None, optimizers: OptimizersConfig { deleted_threshold: 0.5, vacuum_min_vector_number: 100, diff --git a/src/tonic/api/storage_read_api/tests.rs b/src/tonic/api/storage_read_api/tests.rs index 3e754f7db5..67c4057c91 100644 --- a/src/tonic/api/storage_read_api/tests.rs +++ b/src/tonic/api/storage_read_api/tests.rs @@ -1,3 +1,7 @@ +// Deprecated storage placement params (`on_disk`, `always_ram`, `on_disk_payload`) are still +// handled here for backward compatibility with the new `memory` parameter +#![allow(deprecated)] + use std::num::NonZeroUsize; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -35,6 +39,7 @@ fn test_storage_config(storage_path: &Path) -> StorageConfig { snapshots_config: SnapshotsConfig::default(), temp_path: None, on_disk_payload: false, + payload: None, optimizers: OptimizersConfig { deleted_threshold: 0.5, vacuum_min_vector_number: 100,