diff --git a/docs/redoc/master/openapi.json b/docs/redoc/master/openapi.json index 2183676f55..fb49379420 100644 --- a/docs/redoc/master/openapi.json +++ b/docs/redoc/master/openapi.json @@ -7232,7 +7232,7 @@ "type": "object", "properties": { "memory": { - "description": "Memory placement of the point id mapping in indexed segments: `cold` keeps it on disk and reads it on demand, `pinned` keeps it in RAM. `cached` is not supported. Default: `pinned`.", + "description": "Memory placement of the point id mapping in indexed segments: `cold` keeps it on disk and reads it on demand, `cached` keeps it on disk but primes the page cache with it on load, `pinned` keeps it in RAM. Default: `pinned`.", "anyOf": [ { "$ref": "#/components/schemas/Memory" diff --git a/lib/api/src/grpc/proto/collections.proto b/lib/api/src/grpc/proto/collections.proto index d0a192019e..13aaae59f8 100644 --- a/lib/api/src/grpc/proto/collections.proto +++ b/lib/api/src/grpc/proto/collections.proto @@ -530,8 +530,8 @@ message PayloadStorageParams { message IdTrackerParams { // Memory placement of the point id mapping in indexed segments: - // `Cold` keeps it on disk and reads it on demand, `Pinned` keeps it in RAM. - // `Cached` is not supported. + // `Cold` keeps it on disk and reads it on demand, `Cached` keeps it on disk + // but primes the page cache with it on load, `Pinned` keeps it in RAM. optional Memory memory = 1; } diff --git a/lib/api/src/grpc/qdrant.rs b/lib/api/src/grpc/qdrant.rs index 4be46c8010..b4af2a7527 100644 --- a/lib/api/src/grpc/qdrant.rs +++ b/lib/api/src/grpc/qdrant.rs @@ -1183,8 +1183,8 @@ pub struct PayloadStorageParams { #[derive(Clone, Copy, PartialEq, Eq, Hash, ::prost::Message)] pub struct IdTrackerParams { /// Memory placement of the point id mapping in indexed segments: - /// `Cold` keeps it on disk and reads it on demand, `Pinned` keeps it in RAM. - /// `Cached` is not supported. + /// `Cold` keeps it on disk and reads it on demand, `Cached` keeps it on disk + /// but primes the page cache with it on load, `Pinned` keeps it in RAM. #[prost(enumeration = "Memory", optional, tag = "1")] pub memory: ::core::option::Option, } diff --git a/lib/collection/src/config.rs b/lib/collection/src/config.rs index be1510fd3e..c84b4b208f 100644 --- a/lib/collection/src/config.rs +++ b/lib/collection/src/config.rs @@ -208,10 +208,10 @@ impl PayloadStorageParams { #[anonymize(false)] pub struct IdTrackerParams { /// Memory placement of the point id mapping in indexed segments: `cold` keeps it on disk and - /// reads it on demand, `pinned` keeps it in RAM. `cached` is not supported. + /// reads it on demand, `cached` keeps it on disk but primes the page cache with it on load, + /// `pinned` keeps it in RAM. /// Default: `pinned`. #[serde(default, skip_serializing_if = "Option::is_none")] - #[validate(custom(function = "validate_id_tracker_memory"))] pub memory: Option, } @@ -225,21 +225,6 @@ impl IdTrackerParams { } } -/// Reject memory placements not supported by the id tracker. -/// `validator` unwraps `Option` before calling, so we receive `&Memory`. -fn validate_id_tracker_memory(memory: &Memory) -> Result<(), ValidationError> { - match memory { - Memory::Cold | Memory::Pinned => Ok(()), - Memory::Cached => { - let mut error = ValidationError::new("unsupported_memory_placement"); - error.message = Some(std::borrow::Cow::from( - "`cached` memory placement is not supported for id tracker", - )); - Err(error) - } - } -} - /// Reject memory placements not supported by payload storage. /// `validator` unwraps `Option` before calling, so we receive `&Memory`. fn validate_payload_storage_memory(memory: &Memory) -> Result<(), ValidationError> { diff --git a/lib/segment/src/id_tracker/disk_id_tracker/lifecycle.rs b/lib/segment/src/id_tracker/disk_id_tracker/lifecycle.rs index fa9bfe8cb5..3846514907 100644 --- a/lib/segment/src/id_tracker/disk_id_tracker/lifecycle.rs +++ b/lib/segment/src/id_tracker/disk_id_tracker/lifecycle.rs @@ -38,8 +38,9 @@ where } /// Open an existing disk-resident id tracker: `deleted` and `versions` are - /// read into RAM (small, mutated in place), the mapping stays on disk. - pub fn open(fs: &S::Fs, segment_path: &Path) -> OperationResult { + /// read into RAM (small, mutated in place), the mapping stays on disk; + /// `populate` says whether to prime the page cache with it. + pub fn open(fs: &S::Fs, segment_path: &Path, populate: Populate) -> OperationResult { let deleted_storage = StoredBitSlice::open( fs, deleted_path(segment_path), @@ -71,7 +72,7 @@ where let internal_to_version_wrapper = SliceBufferedUpdateWrapper::new(internal_to_version_file.inner)?; - let reader = DiskMappingReader::open(fs, segment_path)?; + let reader = DiskMappingReader::open(fs, segment_path, populate)?; Ok(Self { path: segment_path.to_path_buf(), @@ -158,7 +159,9 @@ where deleted_wrapper.flusher()()?; internal_to_version_wrapper.flusher()()?; - let reader = DiskMappingReader::open(fs, path)?; + // Just written, so cached already; the configured placement applies + // when the built segment is loaded. + let reader = DiskMappingReader::open(fs, path, Populate::No)?; Ok(Self { path: path.to_path_buf(), diff --git a/lib/segment/src/id_tracker/disk_id_tracker/read_only/lifecycle.rs b/lib/segment/src/id_tracker/disk_id_tracker/read_only/lifecycle.rs index 38691d18aa..79e79b355c 100644 --- a/lib/segment/src/id_tracker/disk_id_tracker/read_only/lifecycle.rs +++ b/lib/segment/src/id_tracker/disk_id_tracker/read_only/lifecycle.rs @@ -16,11 +16,11 @@ use crate::id_tracker::immutable_id_tracker::{deleted_path, version_mapping_path use crate::types::SeqNumberType; impl ReadOnlyDiskIdTracker { - pub(super) fn open_options() -> OpenOptions { + pub(super) fn open_options(populate: Populate) -> OpenOptions { OpenOptions { writeable: false, need_sequential: false, - populate: Populate::No, + populate, advice: AdviceSetting::Global, } } @@ -37,16 +37,18 @@ impl ReadOnlyDiskIdTracker { /// Schedule background prefetch of every file [`try_open`](Self::try_open) /// will read. Returns `false` (nothing scheduled) when the tracker is not - /// in the on-disk format. + /// in the on-disk format. `populate` is the placement of the per-point + /// data, see [`open`](Self::open). pub fn try_preopen( fs: &impl CachedReadFs, segment_path: &Path, + populate: Populate, ) -> OperationResult { - if !DiskMappingReader::try_preopen(fs, segment_path)? { + if !DiskMappingReader::try_preopen(fs, segment_path, populate)? { return Ok(false); } - let options = Self::open_options(); + let options = Self::open_options(populate); fs.schedule_open(&version_mapping_path(segment_path), Some(options), None); fs.schedule_open( &deleted_path(segment_path), @@ -58,12 +60,17 @@ impl ReadOnlyDiskIdTracker { } /// Open a read-only disk id tracker at `segment_path`; all per-point data - /// except the `is_uuid` bitmap stays on the backing store. + /// except the `is_uuid` bitmap stays on the backing store. A populating + /// `populate` primes the page cache with the mapping and versions. /// /// Errors if the segment is not in the on-disk format; use /// [`try_open`](Self::try_open) to probe without erroring. - pub fn open(fs: &impl UniversalReadFs, segment_path: &Path) -> OperationResult { - Self::try_open(fs, segment_path)?.ok_or_else(|| { + pub fn open( + fs: &impl UniversalReadFs, + segment_path: &Path, + populate: Populate, + ) -> OperationResult { + Self::try_open(fs, segment_path, populate)?.ok_or_else(|| { OperationError::service_error(format!( "on-disk id tracker not found in segment {}", segment_path.display(), @@ -76,12 +83,13 @@ impl ReadOnlyDiskIdTracker { pub fn try_open( fs: &impl UniversalReadFs, segment_path: &Path, + populate: Populate, ) -> OperationResult> { - let Some(reader) = DiskMappingReader::try_open(fs, segment_path)? else { + let Some(reader) = DiskMappingReader::try_open(fs, segment_path, populate)? else { return Ok(None); }; - let options = Self::open_options(); + let options = Self::open_options(populate); let versions = TypedStorage::::new(fs.open( version_mapping_path(segment_path), diff --git a/lib/segment/src/id_tracker/disk_id_tracker/read_only/live_reload.rs b/lib/segment/src/id_tracker/disk_id_tracker/read_only/live_reload.rs index 87357824d4..7d6e666773 100644 --- a/lib/segment/src/id_tracker/disk_id_tracker/read_only/live_reload.rs +++ b/lib/segment/src/id_tracker/disk_id_tracker/read_only/live_reload.rs @@ -3,7 +3,7 @@ use common::bitvec::BitVec; use common::stored_bitslice::StoredBitSlice; use common::types::PointOffsetType; -use common::universal_io::{CachedReadFs, OkUnchanged, UniversalRead, UniversalReadFs}; +use common::universal_io::{CachedReadFs, OkUnchanged, Populate, UniversalRead, UniversalReadFs}; use futures::future::BoxFuture; use super::ReadOnlyDiskIdTracker; @@ -45,7 +45,7 @@ impl ReadOnlyDiskIdTracker { let Some(fresh) = StoredBitSlice::::open( fs, deleted_path(&self.path), - Self::open_options(), + Self::open_options(Populate::No), Default::default(), ) .ok_unchanged()? diff --git a/lib/segment/src/id_tracker/disk_id_tracker/reader/lifecycle.rs b/lib/segment/src/id_tracker/disk_id_tracker/reader/lifecycle.rs index 14ef1d85dc..1c161e239b 100644 --- a/lib/segment/src/id_tracker/disk_id_tracker/reader/lifecycle.rs +++ b/lib/segment/src/id_tracker/disk_id_tracker/reader/lifecycle.rs @@ -21,14 +21,19 @@ use crate::id_tracker::disk_id_tracker::on_disk_format::{ type Endian = LittleEndian; impl DiskMappingReader { - fn open_options() -> OpenOptions { + /// Options for the `i2e`/`e2i` handles. A non-populating open still + /// prefetches the headers, which are read at open regardless. + fn open_options(populate: Populate) -> OpenOptions { + let populate = match populate { + Populate::Blocking | Populate::PreferBackground => populate, + Populate::Auto | Populate::No | Populate::Partial(_) => Populate::Partial( + ReadRange::new(0, size_of::().max(size_of::()) as u64), + ), + }; OpenOptions { writeable: false, need_sequential: false, - populate: Populate::Partial(ReadRange::new( - 0, - size_of::().max(size_of::()) as u64, - )), + populate, advice: AdviceSetting::Global, } } @@ -46,17 +51,19 @@ impl DiskMappingReader { /// Schedule background prefetch of every file [`try_open`](Self::try_open) /// will open. Returns `false` (nothing scheduled) when the mapping is not - /// in the on-disk format. + /// in the on-disk format. `populate` is the mapping placement, see + /// [`open`](Self::open). pub fn try_preopen( fs: &impl CachedReadFs, segment_path: &Path, + populate: Populate, ) -> OperationResult { let i2e_path = i2e_path(segment_path); if !UniversalReadFileOps::exists(fs, &i2e_path)? { return Ok(false); } - let options = Self::open_options(); + let options = Self::open_options(populate); fs.schedule_open(&i2e_path, Some(options), None); fs.schedule_open(&e2i_path(segment_path), Some(options), None); @@ -75,12 +82,18 @@ impl DiskMappingReader { } /// Open the reader, loading only headers, the sparse index, and the - /// `is_uuid` bitmap into RAM; no per-point mapping data is read. + /// `is_uuid` bitmap into RAM; no per-point mapping data is read. A + /// populating `populate` additionally primes the page cache with the + /// mapping files, which otherwise are paged in on demand. /// /// Errors if the segment is not in the on-disk format (`i2e` absent). Use /// [`try_open`](Self::try_open) to probe without erroring. - pub fn open(fs: &impl UniversalReadFs, segment_path: &Path) -> OperationResult { - Self::try_open(fs, segment_path)?.ok_or_else(|| { + pub fn open( + fs: &impl UniversalReadFs, + segment_path: &Path, + populate: Populate, + ) -> OperationResult { + Self::try_open(fs, segment_path, populate)?.ok_or_else(|| { OperationError::service_error(format!( "on-disk id tracker mapping ({}) not found", i2e_path(segment_path).display(), @@ -95,8 +108,9 @@ impl DiskMappingReader { pub fn try_open( fs: &impl UniversalReadFs, segment_path: &Path, + populate: Populate, ) -> OperationResult> { - let options = Self::open_options(); + let options = Self::open_options(populate); let Some(i2e) = fs .open(i2e_path(segment_path), options, Default::default()) diff --git a/lib/segment/src/id_tracker/disk_id_tracker/tests.rs b/lib/segment/src/id_tracker/disk_id_tracker/tests.rs index 5695265c5f..08b3236c3a 100644 --- a/lib/segment/src/id_tracker/disk_id_tracker/tests.rs +++ b/lib/segment/src/id_tracker/disk_id_tracker/tests.rs @@ -1,6 +1,6 @@ use ahash::AHashMap; use common::types::DeferredBehavior; -use common::universal_io::{MmapFile, MmapFs}; +use common::universal_io::{MmapFile, MmapFs, Populate}; use rand::SeedableRng as _; use rand::rngs::StdRng; use tempfile::Builder; @@ -169,7 +169,8 @@ fn batch_lookups_match_single() { let disk = DiskIdTracker::::new(&MmapFs, dir.path(), &versions, mappings).unwrap(); assert_batch_parity(&disk); - let read_only = ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path()).unwrap(); + let read_only = + ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path(), Populate::No).unwrap(); assert_batch_parity(&read_only); } @@ -193,7 +194,8 @@ fn read_only_matches_immutable() { // Writing the files also validates the on-disk format round-trips. let _disk = DiskIdTracker::::new(&MmapFs, dir.path(), &versions, mappings).unwrap(); - let read_only = ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path()).unwrap(); + let read_only = + ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path(), Populate::No).unwrap(); assert_read_parity(&immutable, &read_only); } @@ -228,8 +230,13 @@ fn detect_and_load_selects_disk_format() { let disk_dir = Builder::new().prefix("disk").tempdir().unwrap(); let _disk = DiskIdTracker::::new(&MmapFs, disk_dir.path(), &versions, mappings).unwrap(); - let loaded = - ReadOnlyIdTrackerEnum::::detect_and_load(&MmapFs, disk_dir.path(), None).unwrap(); + let loaded = ReadOnlyIdTrackerEnum::::detect_and_load( + &MmapFs, + disk_dir.path(), + None, + Populate::No, + ) + .unwrap(); assert_eq!(loaded.name(), "read-only disk id tracker"); assert_read_parity(&immutable, &loaded); @@ -238,15 +245,24 @@ fn detect_and_load_selects_disk_format() { let imm_dir = Builder::new().prefix("imm").tempdir().unwrap(); let _imm = ImmutableIdTracker::::new(&MmapFs, imm_dir.path(), &versions2, mappings2) .unwrap(); - let loaded = - ReadOnlyIdTrackerEnum::::detect_and_load(&MmapFs, imm_dir.path(), None).unwrap(); + let loaded = ReadOnlyIdTrackerEnum::::detect_and_load( + &MmapFs, + imm_dir.path(), + None, + Populate::No, + ) + .unwrap(); assert_eq!(loaded.name(), "read-only immutable id tracker"); // An empty segment (no mapping files) falls back to the appendable reader. let empty_dir = Builder::new().prefix("empty").tempdir().unwrap(); - let loaded = - ReadOnlyIdTrackerEnum::::detect_and_load(&MmapFs, empty_dir.path(), None) - .unwrap(); + let loaded = ReadOnlyIdTrackerEnum::::detect_and_load( + &MmapFs, + empty_dir.path(), + None, + Populate::No, + ) + .unwrap(); assert_eq!(loaded.name(), "read-only appendable id tracker"); } @@ -261,7 +277,8 @@ fn is_uuid_sidecar_written_and_listed() { assert!(disk.files().contains(&is_uuid_file)); assert!(disk.immutable_files().contains(&is_uuid_file)); - let read_only = ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path()).unwrap(); + let read_only = + ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path(), Populate::No).unwrap(); assert!(read_only.files().contains(&is_uuid_file)); } @@ -301,7 +318,8 @@ fn read_by_id_does_not_materialize_deleted_set() { disk.point_mappings().iter_from(None).collect() }; - let read_only = ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path()).unwrap(); + let read_only = + ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path(), Populate::No).unwrap(); // Point lookups must not trigger the full deleted-set materialization. for (external_id, offset) in live.iter().take(200) { @@ -349,7 +367,8 @@ fn deletion_and_live_reload() { DiskIdTracker::::new(&MmapFs, dir.path(), &versions, mappings).unwrap(); // A reader opened before the deletions; it will pick them up via live_reload. - let mut read_only = ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path()).unwrap(); + let mut read_only = + ReadOnlyDiskIdTracker::::open(&MmapFs, dir.path(), Populate::No).unwrap(); // Establish the diff baseline (a search-style access) so the next reload // reports only the incremental deletions, not every build-time deletion. let _ = read_only.deleted_point_bitslice(); @@ -441,7 +460,8 @@ fn deletion_and_live_reload_disk_cache() { ReadOnlyImmutableIdTracker::>::open(&cache_fs, &immutable_path) .unwrap(); let mut read_only_disk = - ReadOnlyDiskIdTracker::>::open(&cache_fs, &disk_path).unwrap(); + ReadOnlyDiskIdTracker::>::open(&cache_fs, &disk_path, Populate::No) + .unwrap(); // Establish the diff baseline (a search-style access) so the reload // reports only the incremental deletions, not every build-time deletion. let _ = read_only_disk.deleted_point_bitslice(); diff --git a/lib/segment/src/id_tracker/id_tracker_base/read_only_tracker_enum.rs b/lib/segment/src/id_tracker/id_tracker_base/read_only_tracker_enum.rs index 76a78271c0..66af41755c 100644 --- a/lib/segment/src/id_tracker/id_tracker_base/read_only_tracker_enum.rs +++ b/lib/segment/src/id_tracker/id_tracker_base/read_only_tracker_enum.rs @@ -2,7 +2,7 @@ use std::path::Path; use common::bitvec::BitSlice; use common::types::PointOffsetType; -use common::universal_io::{CachedReadFs, UniversalRead, UniversalReadFs}; +use common::universal_io::{CachedReadFs, Populate, UniversalRead, UniversalReadFs}; use futures::future::BoxFuture; use crate::common::operation_error::OperationResult; @@ -23,8 +23,14 @@ pub enum ReadOnlyIdTrackerEnum { impl ReadOnlyIdTrackerEnum { /// Schedule background prefetch for whichever id-tracker format is /// present, probing in the same order as [`Self::detect_and_load`]. - pub fn preopen(fs: &impl CachedReadFs, segment_path: &Path) -> OperationResult<()> { - if ReadOnlyDiskIdTracker::try_preopen(fs, segment_path)? { + /// `populate` applies to the disk-resident format only: the other formats + /// hold their per-point data in RAM regardless. + pub fn preopen( + fs: &impl CachedReadFs, + segment_path: &Path, + populate: Populate, + ) -> OperationResult<()> { + if ReadOnlyDiskIdTracker::try_preopen(fs, segment_path, populate)? { return Ok(()); } if ReadOnlyImmutableIdTracker::try_preopen(fs, segment_path)? { @@ -40,12 +46,14 @@ impl ReadOnlyIdTrackerEnum { /// Order: disk-resident (the serverless/object-storage format) first, then /// the in-RAM immutable format, then the appendable/mutable format (whose /// open tolerates absent files, i.e. a fresh or empty segment). + /// `populate` applies to the disk-resident format only, see [`Self::preopen`]. pub fn detect_and_load( fs: &impl UniversalReadFs, segment_path: &Path, deferred_internal_id: Option, + populate: Populate, ) -> OperationResult { - if let Some(tracker) = ReadOnlyDiskIdTracker::try_open(fs, segment_path)? { + if let Some(tracker) = ReadOnlyDiskIdTracker::try_open(fs, segment_path, populate)? { return Ok(Self::DiskResident(tracker)); } if let Some(tracker) = ReadOnlyImmutableIdTracker::try_open(fs, segment_path)? { diff --git a/lib/segment/src/segment/read_only/lifecycle.rs b/lib/segment/src/segment/read_only/lifecycle.rs index d59073a694..413f57571c 100644 --- a/lib/segment/src/segment/read_only/lifecycle.rs +++ b/lib/segment/src/segment/read_only/lifecycle.rs @@ -63,6 +63,17 @@ fn payload_populate(config: &SegmentConfig) -> Populate { } } +/// How the disk-resident id tracker's per-point data is brought into memory; +/// the other tracker formats hold it in RAM regardless. +fn id_tracker_populate(config: &SegmentConfig) -> Populate { + let memory = config.id_tracker_memory_placement().clamp_to_low_memory(); + if memory.populate_on_open() { + Populate::PreferBackground + } else { + Populate::No + } +} + /// How one sparse vector's storage is brought into memory. Search never reads /// it, so it normally stays cold — except when the (non-persisted) mutable-RAM /// sparse index is configured: that index is rebuilt from the storage at open, @@ -141,7 +152,7 @@ impl + 'static> ReadOnlySegment ReadOnlyPayloadStorage::preopen(fs, segment_path.to_path_buf(), payload_storage_populate)?; // Id tracker; always loaded — every request resolves ids through it. - ReadOnlyIdTrackerEnum::preopen(fs, segment_path)?; + ReadOnlyIdTrackerEnum::preopen(fs, segment_path, id_tracker_populate(&config))?; // Vector storages for (vector_name, vector_config) in &config.vector_data { @@ -255,6 +266,7 @@ impl + 'static> ReadOnlySegment &fs, segment_path, deferred_internal_id, + id_tracker_populate(&config), )?)); // Open all vector storages up front: the payload index needs them. diff --git a/lib/segment/src/segment/update_only/lookup/lifecycle.rs b/lib/segment/src/segment/update_only/lookup/lifecycle.rs index e7cdb0cf20..7fd443839a 100644 --- a/lib/segment/src/segment/update_only/lookup/lifecycle.rs +++ b/lib/segment/src/segment/update_only/lookup/lifecycle.rs @@ -121,6 +121,7 @@ impl LookupSegment { &fs, segment_path, deferred_internal_id.filter(|_| appendable), + WRITER_POPULATE, )?)); let mut vector_data = HashMap::new(); @@ -174,7 +175,7 @@ impl LookupSegment { } = read_json_via(fs, segment_path.join(SEGMENT_STATE_FILE))?; ReadOnlyPayloadStorage::preopen(fs, segment_path.to_path_buf(), WRITER_POPULATE)?; - ReadOnlyIdTrackerEnum::preopen(fs, segment_path)?; + ReadOnlyIdTrackerEnum::preopen(fs, segment_path, WRITER_POPULATE)?; for (vector_name, vector_config) in &config.vector_data { let path = get_vector_storage_path(segment_path, vector_name); diff --git a/lib/segment/src/segment_constructor/segment_builder.rs b/lib/segment/src/segment_constructor/segment_builder.rs index e83688bd17..342d3927be 100644 --- a/lib/segment/src/segment_constructor/segment_builder.rs +++ b/lib/segment/src/segment_constructor/segment_builder.rs @@ -592,8 +592,8 @@ impl SegmentBuilder { let id_tracker = match id_tracker { IdTrackerEnum::InMemoryIdTracker(in_memory_id_tracker) => { match segment_config.id_tracker_memory_placement() { - // Mapping stays on disk. `Cached` is rejected by API validation; - // it defensively maps to the closest supported placement. + // Mapping stays on disk; a cached placement primes the page cache + // with it when the built segment is loaded. Memory::Cold | Memory::Cached => { let disk_id_tracker = DiskIdTracker::from_in_memory_tracker( &MmapFs, diff --git a/lib/segment/src/segment_constructor/segment_constructor_base/create_segment.rs b/lib/segment/src/segment_constructor/segment_constructor_base/create_segment.rs index 3ea06fef5c..0f93ad61c2 100644 --- a/lib/segment/src/segment_constructor/segment_constructor_base/create_segment.rs +++ b/lib/segment/src/segment_constructor/segment_constructor_base/create_segment.rs @@ -8,7 +8,7 @@ use atomic_refcell::AtomicRefCell; use common::defaults::log_load_timing; use common::is_alive_lock::IsAliveLock; use common::types::PointOffsetType; -use common::universal_io::MmapFs; +use common::universal_io::{MmapFs, Populate}; use parking_lot::Mutex; use uuid::Uuid; @@ -58,9 +58,19 @@ pub(super) fn create_segment( let deferred_internal_id = deferred_internal_id.filter(|_| appendable_flag); let id_tracker_format = IdTrackerFormat::detect_local(segment_path, appendable_flag); + let id_tracker_populate = Populate::from( + config + .id_tracker_memory_placement() + .clamp_to_low_memory() + .populate_on_open(), + ); let started = Instant::now(); - let id_tracker = - create_segment_id_tracker(id_tracker_format, segment_path, deferred_internal_id)?; + let id_tracker = create_segment_id_tracker( + id_tracker_format, + segment_path, + deferred_internal_id, + id_tracker_populate, + )?; log_load_timing(segment_path, "id_tracker", started); let mut vector_storages = HashMap::new(); diff --git a/lib/segment/src/segment_constructor/segment_constructor_base/id_tracker.rs b/lib/segment/src/segment_constructor/segment_constructor_base/id_tracker.rs index 14f8219dc8..3200c7ef06 100644 --- a/lib/segment/src/segment_constructor/segment_constructor_base/id_tracker.rs +++ b/lib/segment/src/segment_constructor/segment_constructor_base/id_tracker.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use atomic_refcell::AtomicRefCell; use common::types::PointOffsetType; -use common::universal_io::MmapFs; +use common::universal_io::{MmapFs, Populate}; use super::sp; use crate::common::operation_error::OperationResult; @@ -19,10 +19,13 @@ pub(crate) fn create_mutable_id_tracker( MutableIdTracker::open(segment_path, deferred_internal_id) } +/// `populate` is the placement of the disk-resident format's mapping; the other +/// formats hold it in RAM regardless. pub(crate) fn create_segment_id_tracker( format: IdTrackerFormat, segment_path: &Path, deferred_internal_id: Option, + populate: Populate, ) -> OperationResult>> { let id_tracker = match format { IdTrackerFormat::Mutable => IdTrackerEnum::MutableIdTracker(create_mutable_id_tracker( @@ -33,7 +36,7 @@ pub(crate) fn create_segment_id_tracker( IdTrackerEnum::ImmutableIdTracker(ImmutableIdTracker::open(&MmapFs, segment_path)?) } IdTrackerFormat::Disk => { - IdTrackerEnum::DiskIdTracker(DiskIdTracker::open(&MmapFs, segment_path)?) + IdTrackerEnum::DiskIdTracker(DiskIdTracker::open(&MmapFs, segment_path, populate)?) } }; Ok(sp(id_tracker)) diff --git a/tests/openapi/test_memory_placement.py b/tests/openapi/test_memory_placement.py index 06ad4b6c09..bfdc9a5058 100644 --- a/tests/openapi/test_memory_placement.py +++ b/tests/openapi/test_memory_placement.py @@ -150,15 +150,21 @@ def test_id_tracker_memory_placement(): assert response.ok, response.text assert response.json()["result"]["config"]["params"]["id_tracker"]["memory"] == "pinned" - # The id tracker has no populate-on-open variant: `cached` is rejected response = request_with_validation( api="/collections/{collection_name}", method="PATCH", path_params={"collection_name": collection_name}, body={"params": {"id_tracker": {"memory": "cached"}}}, ) - assert response.status_code == 422, response.text - assert "cached" in response.json()["status"]["error"] + assert response.ok, response.text + + response = request_with_validation( + api="/collections/{collection_name}", + method="GET", + path_params={"collection_name": collection_name}, + ) + assert response.ok, response.text + assert response.json()["result"]["config"]["params"]["id_tracker"]["memory"] == "cached" def test_pinned_payload_storage_is_rejected():