refactor: replace StructPayloadIndex::open bool flags with StorageType and IndexLoadMode (#9754)

StructPayloadIndex::open took two adjacent bools (is_appendable, create)
and call sites passed every literal combination: (true, true),
(true, false) and (false, true) all exist. A transposed pair compiles
and silently yields e.g. non-appendable + create instead of
appendable + load-only.

The target enum already existed: open immediately converted the bool
into the private StorageType { Appendable, NonAppendable }, so the bool
survived only at the API boundary, exactly where the swap hazard lives.
Make StorageType public, take it directly, and introduce
IndexLoadMode { CreateIfMissing, LoadExisting } for the create flag.

create_segment had the same trailing create: bool with bare literals at
both callers, so its parameter is lifted to IndexLoadMode as well:
load_segment passes LoadExisting, build_segment passes CreateIfMissing.
No behavior change.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Arnaud Gourlay
2026-07-14 10:47:43 +02:00
committed by generall
parent 18d1c445b5
commit 3d2e31341a
12 changed files with 75 additions and 59 deletions

View File

@@ -11,7 +11,7 @@ use segment::fixtures::payload_context_fixture::{
create_struct_payload_index,
};
use segment::fixtures::payload_fixtures::BOOL_KEY;
use segment::index::struct_payload_index::StructPayloadIndex;
use segment::index::struct_payload_index::{IndexLoadMode, StorageType, StructPayloadIndex};
use segment::index::{PayloadIndex, PayloadIndexRead};
use segment::types::{Condition, FieldCondition, Filter, Match, PayloadSchemaType, ValueVariants};
use tempfile::Builder;
@@ -108,8 +108,8 @@ pub fn keyword_index_boolean_query_points(c: &mut Criterion) {
id_tracker,
std::collections::HashMap::new(),
dir.path(),
true,
true,
StorageType::Appendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();

View File

@@ -13,7 +13,7 @@ use rand::prelude::StdRng;
use rand::{Rng, RngExt, SeedableRng};
use segment::fixtures::payload_context_fixture::create_id_tracker_fixture;
use segment::fixtures::payload_fixtures::{FLT_KEY, INT_KEY};
use segment::index::struct_payload_index::StructPayloadIndex;
use segment::index::struct_payload_index::{IndexLoadMode, StorageType, StructPayloadIndex};
use segment::index::{PayloadIndex, PayloadIndexRead};
use segment::payload_json;
use segment::payload_storage::PayloadStorage;
@@ -70,8 +70,8 @@ fn range_filtering(c: &mut Criterion) {
id_tracker.clone(),
std::collections::HashMap::new(),
dir.path(),
true,
true,
StorageType::Appendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();
@@ -148,8 +148,8 @@ fn range_filtering(c: &mut Criterion) {
id_tracker,
std::collections::HashMap::new(),
dir.path(),
false,
true,
StorageType::NonAppendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();

View File

@@ -20,7 +20,7 @@ use segment::index::sparse_index::sparse_index_config::{SparseIndexConfig, Spars
use segment::index::sparse_index::sparse_vector_index::{
SparseVectorIndex, SparseVectorIndexOpenArgs,
};
use segment::index::struct_payload_index::StructPayloadIndex;
use segment::index::struct_payload_index::{IndexLoadMode, StorageType, StructPayloadIndex};
use segment::payload_storage::in_memory_payload_storage::InMemoryPayloadStorage;
use segment::types::VectorStorageDatatype;
use segment::vector_storage::sparse::simple_sparse_vector_storage::open_simple_sparse_vector_storage;
@@ -53,8 +53,8 @@ fn sparse_vector_index_build_benchmark(c: &mut Criterion) {
id_tracker.clone(),
std::collections::HashMap::new(),
payload_dir.path(),
true,
true,
StorageType::Appendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();
let wrapped_payload_index = Arc::new(AtomicRefCell::new(payload_index));

View File

@@ -16,7 +16,7 @@ use crate::id_tracker::in_memory_id_tracker::InMemoryIdTracker;
use crate::id_tracker::{IdTracker, IdTrackerEnum};
use crate::index::PayloadIndex;
use crate::index::plain_payload_index::PlainPayloadIndex;
use crate::index::struct_payload_index::StructPayloadIndex;
use crate::index::struct_payload_index::{IndexLoadMode, StorageType, StructPayloadIndex};
use crate::payload_storage::PayloadStorage;
use crate::payload_storage::in_memory_payload_storage::InMemoryPayloadStorage;
use crate::payload_storage::query_checker::SimpleConditionChecker;
@@ -111,8 +111,8 @@ pub fn create_struct_payload_index(
id_tracker,
std::collections::HashMap::new(),
path,
true,
true,
StorageType::Appendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();

View File

@@ -19,7 +19,7 @@ use crate::index::sparse_index::sparse_index_config::{SparseIndexConfig, SparseI
use crate::index::sparse_index::sparse_vector_index::{
SparseVectorIndex, SparseVectorIndexOpenArgs,
};
use crate::index::struct_payload_index::StructPayloadIndex;
use crate::index::struct_payload_index::{IndexLoadMode, StorageType, StructPayloadIndex};
use crate::payload_storage::in_memory_payload_storage::InMemoryPayloadStorage;
use crate::vector_storage::sparse::mmap_sparse_vector_storage::MmapSparseVectorStorage;
use crate::vector_storage::{VectorStorage, VectorStorageEnum, VectorStorageRead};
@@ -47,8 +47,8 @@ pub fn fixture_sparse_index_from_iter<I: InvertedIndexReadWrite<MmapFile>>(
id_tracker.clone(),
std::collections::HashMap::new(),
payload_dir,
true,
true,
StorageType::Appendable,
IndexLoadMode::CreateIfMissing,
)?;
let wrapped_payload_index = Arc::new(AtomicRefCell::new(payload_index));

View File

@@ -30,12 +30,33 @@ use crate::payload_storage::payload_storage_enum::PayloadStorageEnum;
use crate::types::{Memory, PayloadFieldSchema, PayloadKeyType, VectorNameBuf};
use crate::vector_storage::VectorStorageEnum;
#[derive(Debug)]
enum StorageType {
/// Desired storage type for payload indices of a segment.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum StorageType {
Appendable,
NonAppendable,
}
impl StorageType {
/// Storage type for payload indices of a segment, given whether the segment is appendable.
pub fn from_appendable(appendable: bool) -> Self {
if appendable {
StorageType::Appendable
} else {
StorageType::NonAppendable
}
}
}
/// Whether opening a payload index may create missing index data or must only load existing data.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
pub enum IndexLoadMode {
/// Create missing index data while loading.
CreateIfMissing,
/// Only load existing index data.
LoadExisting,
}
/// `PayloadIndex` implementation, which actually uses index structures for providing faster search
#[derive(Debug)]
pub struct StructPayloadIndex {
@@ -203,8 +224,8 @@ impl StructPayloadIndex {
id_tracker: Arc<AtomicRefCell<IdTrackerEnum>>,
vector_storages: HashMap<VectorNameBuf, Arc<AtomicRefCell<VectorStorageEnum>>>,
path: &Path,
is_appendable: bool,
create: bool,
storage_type: StorageType,
load_mode: IndexLoadMode,
) -> OperationResult<Self> {
fs::create_dir_all(path)?;
let config_path = PayloadConfig::get_config_path(path);
@@ -214,12 +235,6 @@ impl StructPayloadIndex {
PayloadConfig::default()
};
let storage_type = if is_appendable {
StorageType::Appendable
} else {
StorageType::NonAppendable
};
let mut index = StructPayloadIndex {
payload,
id_tracker,
@@ -236,7 +251,7 @@ impl StructPayloadIndex {
index.save_config()?;
}
index.load_all_fields(create)?;
index.load_all_fields(load_mode == IndexLoadMode::CreateIfMissing)?;
Ok(index)
}

View File

@@ -250,7 +250,7 @@ fn drop_index_if_incompatible_keeps_non_appendable_index_on_on_disk_only_change(
use atomic_refcell::AtomicRefCell;
use super::StructPayloadIndex;
use super::{IndexLoadMode, StorageType, StructPayloadIndex};
use crate::data_types::index::IntegerIndexParams;
use crate::fixtures::payload_context_fixture::{
create_id_tracker_fixture, create_payload_storage_fixture,
@@ -272,8 +272,8 @@ fn drop_index_if_incompatible_keeps_non_appendable_index_on_on_disk_only_change(
id_tracker,
HashMap::new(),
dir.path(),
false,
true,
StorageType::NonAppendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();
@@ -349,7 +349,7 @@ fn build_index_reloads_in_new_mode_on_on_disk_change() {
use atomic_refcell::AtomicRefCell;
use super::StructPayloadIndex;
use super::{IndexLoadMode, StorageType, StructPayloadIndex};
use crate::data_types::index::IntegerIndexParams;
use crate::fixtures::payload_context_fixture::{
create_id_tracker_fixture, create_payload_storage_fixture,
@@ -371,8 +371,8 @@ fn build_index_reloads_in_new_mode_on_on_disk_change() {
id_tracker,
HashMap::new(),
dir.path(),
false,
true,
StorageType::NonAppendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();

View File

@@ -39,7 +39,7 @@ use crate::id_tracker::in_memory_id_tracker::InMemoryIdTracker;
use crate::id_tracker::{IdTracker, IdTrackerEnum, IdTrackerRead, for_each_unique_point};
use crate::index::field_index::FieldIndex;
use crate::index::sparse_index::sparse_vector_index::SparseVectorIndexOpenArgs;
use crate::index::struct_payload_index::StructPayloadIndex;
use crate::index::struct_payload_index::{IndexLoadMode, StorageType, StructPayloadIndex};
use crate::index::{PayloadIndex, PayloadIndexRead, VectorIndexEnum};
use crate::payload_storage::PayloadStorage;
use crate::payload_storage::payload_storage_enum::PayloadStorageEnum;
@@ -673,8 +673,8 @@ impl SegmentBuilder {
id_tracker_arc.clone(),
vector_storages_arc.clone(),
&payload_index_path,
appendable_flag,
true,
StorageType::from_appendable(appendable_flag),
IndexLoadMode::CreateIfMissing,
)?;
for (field, payload_schema, progress) in indexed_fields {
progress.start();

View File

@@ -23,7 +23,7 @@ use crate::common::operation_error::{OperationResult, check_process_stopped};
use crate::id_tracker::{IdTrackerEnum, IdTrackerFormat, IdTrackerRead};
use crate::index::VectorIndexEnum;
use crate::index::sparse_index::sparse_vector_index::SparseVectorIndexOpenArgs;
use crate::index::struct_payload_index::StructPayloadIndex;
use crate::index::struct_payload_index::{IndexLoadMode, StorageType, StructPayloadIndex};
use crate::segment::{Segment, VectorData};
use crate::types::{
SegmentConfig, SegmentType, SeqNumberType, SparseVectorDataConfig, VectorDataConfig, VectorName,
@@ -35,8 +35,8 @@ use crate::vector_storage::{VectorStorageEnum, VectorStorageRead};
///
/// Opens the payload storage, id tracker, every (dense and sparse) vector
/// storage, the payload index, and finally each vector's index, wiring them all
/// into a [`Segment`]. Used by both [`super::load_segment`] (`create = false`)
/// and [`super::build_segment`] (`create = true`).
/// into a [`Segment`]. Used by both [`super::load_segment`] (`LoadExisting`)
/// and [`super::build_segment`] (`CreateIfMissing`).
#[allow(clippy::too_many_arguments)]
pub(super) fn create_segment(
initial_version: Option<SeqNumberType>,
@@ -46,7 +46,7 @@ pub(super) fn create_segment(
deferred_internal_id: Option<PointOffsetType>,
config: &SegmentConfig,
stopped: &AtomicBool,
create: bool,
load_mode: IndexLoadMode,
) -> OperationResult<Segment> {
let started = Instant::now();
let payload_storage = sp(create_payload_storage(segment_path, config)?);
@@ -103,8 +103,8 @@ pub(super) fn create_segment(
id_tracker.clone(),
vector_storages.clone(),
&payload_index_path,
appendable_flag,
create,
StorageType::from_appendable(appendable_flag),
load_mode,
)?);
log_load_timing(segment_path, "payload_index", started);

View File

@@ -14,6 +14,7 @@ use uuid::Uuid;
use super::create_segment::create_segment;
use super::legacy_state::{load_segment_state_v3, load_segment_state_v5};
use crate::common::operation_error::{OperationError, OperationResult};
use crate::index::struct_payload_index::IndexLoadMode;
use crate::segment::{Segment, SegmentVersion};
use crate::types::SegmentConfig;
@@ -161,7 +162,7 @@ pub fn load_segment(
deferred_internal_id,
&segment_state.config,
stopped,
false,
IndexLoadMode::LoadExisting,
)?;
log_load_timing(path, "total", total_started);
@@ -202,7 +203,7 @@ pub fn build_segment(
deferred_internal_id,
config,
&stopped,
true,
IndexLoadMode::CreateIfMissing,
)?;
segment.save_current_state()?;

View File

@@ -7,7 +7,7 @@ use common::condition_checker::ConditionChecker;
use common::counter::hardware_counter::HardwareCounterCell;
use common::types::PointOffsetType;
use segment::fixtures::payload_context_fixture::create_id_tracker_fixture;
use segment::index::struct_payload_index::StructPayloadIndex;
use segment::index::struct_payload_index::{IndexLoadMode, StorageType, StructPayloadIndex};
use segment::index::{PayloadIndex, PayloadIndexRead};
use segment::json_path::JsonPath;
use segment::payload_json;
@@ -79,8 +79,8 @@ fn test_filtering_context_consistency() {
id_tracker,
HashMap::new(),
dir.path(),
true,
true,
StorageType::Appendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();

View File

@@ -36,7 +36,7 @@ use segment::fixtures::payload_fixtures::{
};
use segment::id_tracker::IdTrackerRead;
use segment::index::field_index::{FieldIndex, PayloadFieldIndexRead, PrimaryCondition};
use segment::index::struct_payload_index::StructPayloadIndex;
use segment::index::struct_payload_index::{IndexLoadMode, StorageType, StructPayloadIndex};
use segment::index::{PayloadIndex, PayloadIndexRead};
use segment::json_path::JsonPath;
use segment::payload_json;
@@ -1313,8 +1313,8 @@ fn test_update_payload_index_type() {
id_tracker,
HashMap::new(),
dir.path(),
true,
true,
StorageType::Appendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();
@@ -1372,8 +1372,8 @@ fn test_bool_index_appendable_reopen_accepts_updates() {
id_tracker,
HashMap::new(),
dir.path(),
true,
true,
StorageType::Appendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();
@@ -1393,8 +1393,8 @@ fn test_bool_index_appendable_reopen_accepts_updates() {
id_tracker,
HashMap::new(),
dir.path(),
true,
false,
StorageType::Appendable,
IndexLoadMode::LoadExisting,
)
.unwrap();
@@ -1434,8 +1434,8 @@ fn test_null_index_appendable_reopen_loads_and_accepts_updates() {
id_tracker,
HashMap::new(),
dir.path(),
true,
true,
StorageType::Appendable,
IndexLoadMode::CreateIfMissing,
)
.unwrap();
@@ -1457,8 +1457,8 @@ fn test_null_index_appendable_reopen_loads_and_accepts_updates() {
id_tracker,
HashMap::new(),
dir.path(),
true,
false,
StorageType::Appendable,
IndexLoadMode::LoadExisting,
)
.unwrap();