Use CollectionResult where possible (#6463)

This commit is contained in:
Arnaud Gourlay
2025-04-30 09:49:04 +02:00
committed by generall
parent d968726c72
commit b00f68535e
13 changed files with 56 additions and 62 deletions

View File

@@ -256,7 +256,7 @@ async fn clean_task(
shard_holder: Weak<LockedShardHolder>,
shard_id: ShardId,
sender: Sender<ShardCleanStatus>,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
// Do not measure the hardware usage of these deletes as clean the shard is always considered an internal operation
// users should not be billed for.

View File

@@ -116,7 +116,7 @@ impl Collection {
update_runtime: Option<Handle>,
optimizer_resource_budget: ResourceBudget,
optimizers_overwrite: Option<OptimizersConfigDiff>,
) -> Result<Self, CollectionError> {
) -> CollectionResult<Self> {
let start_time = std::time::Instant::now();
let mut shard_holder = ShardHolder::new(path)?;

View File

@@ -5,7 +5,7 @@ use segment::types::ShardKey;
use crate::collection::Collection;
use crate::config::ShardingMethod;
use crate::operations::types::CollectionError;
use crate::operations::types::{CollectionError, CollectionResult};
use crate::operations::{
CollectionUpdateOperations, CreateIndex, FieldIndexOperations, OperationWithClockTag,
};
@@ -19,7 +19,7 @@ impl Collection {
shard_key: Option<ShardKey>,
replicas: &[PeerId],
init_state: Option<ReplicaState>,
) -> Result<ShardReplicaSet, CollectionError> {
) -> CollectionResult<ShardReplicaSet> {
let is_local = replicas.contains(&self.this_peer_id);
let peers = replicas
@@ -60,7 +60,7 @@ impl Collection {
&self,
shard_key: ShardKey,
placement: ShardsPlacement,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let hw_counter = HwMeasurementAcc::disposable(); // Internal operation. No measurement needed.
let state = self.state().await;
@@ -142,7 +142,7 @@ impl Collection {
Ok(())
}
pub async fn drop_shard_key(&self, shard_key: ShardKey) -> Result<(), CollectionError> {
pub async fn drop_shard_key(&self, shard_key: ShardKey) -> CollectionResult<()> {
let state = self.state().await;
match state.config.params.sharding_method.unwrap_or_default() {

View File

@@ -2,7 +2,7 @@ use segment::types::{Filter, SearchParams, StrictModeConfig};
use super::StrictModeVerification;
use crate::collection::Collection;
use crate::operations::types::{CollectionError, DiscoverRequestBatch, DiscoverRequestInternal};
use crate::operations::types::{CollectionResult, DiscoverRequestBatch, DiscoverRequestInternal};
impl StrictModeVerification for DiscoverRequestInternal {
fn query_limit(&self) -> Option<usize> {
@@ -31,7 +31,7 @@ impl StrictModeVerification for DiscoverRequestBatch {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
for i in self.searches.iter() {
i.discover_request
.check_strict_mode(collection, strict_mode_config)

View File

@@ -13,7 +13,7 @@ use std::fmt::Display;
use segment::types::{Filter, SearchParams, StrictModeConfig};
use super::types::CollectionError;
use super::types::{CollectionError, CollectionResult};
use crate::collection::Collection;
// Creates a new `VerificationPass` without actually verifying anything.
@@ -40,7 +40,7 @@ pub trait StrictModeVerification {
&self,
_collection: &Collection,
_strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
Ok(())
}
@@ -62,10 +62,7 @@ pub trait StrictModeVerification {
fn request_search_params(&self) -> Option<&SearchParams>;
/// Checks the 'exact' parameter.
fn check_request_exact(
&self,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
fn check_request_exact(&self, strict_mode_config: &StrictModeConfig) -> CollectionResult<()> {
check_bool_opt(
self.request_exact(),
strict_mode_config.search_allow_exact,
@@ -78,7 +75,7 @@ pub trait StrictModeVerification {
fn check_request_query_limit(
&self,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
check_limit_opt(
self.query_limit(),
strict_mode_config.max_query_limit,
@@ -92,7 +89,7 @@ pub trait StrictModeVerification {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
if let Some(search_params) = self.request_search_params() {
Box::pin(search_params.check_strict_mode(collection, strict_mode_config)).await?;
}
@@ -104,10 +101,10 @@ pub trait StrictModeVerification {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let check_filter = |filter: Option<&Filter>,
allow_unindexed_filter: Option<bool>|
-> Result<(), CollectionError> {
-> CollectionResult<()> {
let Some(filter) = filter else {
return Ok(());
};
@@ -154,7 +151,7 @@ pub trait StrictModeVerification {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
self.check_custom(collection, strict_mode_config).await?;
self.check_request_query_limit(strict_mode_config)?;
self.check_request_filter(collection, strict_mode_config)?;
@@ -168,7 +165,7 @@ pub trait StrictModeVerification {
fn check_filter_limits(
filter: &Filter,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
// Filter condition count limit
if let Some(filter_condition_limit) = strict_mode_config.filter_max_conditions {
let filter_conditions = filter.total_conditions_count();
@@ -203,7 +200,7 @@ fn check_filter_limits(
pub fn check_timeout(
timeout: usize,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
check_limit_opt(Some(timeout), strict_mode_config.max_timeout, "timeout")
}
@@ -212,7 +209,7 @@ pub(crate) fn check_bool_opt(
allowed: Option<bool>,
name: &str,
parameter: &str,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
if allowed != Some(false) || !value.unwrap_or_default() {
return Ok(());
}
@@ -227,7 +224,7 @@ pub(crate) fn check_limit_opt<T: PartialOrd + Display>(
value: Option<T>,
limit: Option<T>,
name: &str,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let (Some(limit), Some(value)) = (limit, value) else {
return Ok(());
};
@@ -262,7 +259,7 @@ impl StrictModeVerification for SearchParams {
&self,
_collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
check_limit_opt(
self.quantization.and_then(|i| i.oversampling),
strict_mode_config.search_max_oversampling,

View File

@@ -82,7 +82,7 @@ impl StrictModeVerification for CollectionPrefetch {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), crate::operations::types::CollectionError> {
) -> CollectionResult<()> {
// CollectionPrefetch.prefetch is of type CollectionPrefetch (recursive type)
for prefetch in &self.prefetch {
Box::pin(prefetch.check_strict_mode(collection, strict_mode_config)).await?;

View File

@@ -3,7 +3,7 @@ use segment::types::{Filter, SearchParams, StrictModeConfig};
use super::StrictModeVerification;
use crate::collection::Collection;
use crate::operations::types::{CollectionError, CoreSearchRequest, SearchRequestBatch};
use crate::operations::types::{CollectionResult, CoreSearchRequest, SearchRequestBatch};
impl StrictModeVerification for SearchRequestInternal {
fn indexed_filter_read(&self) -> Option<&Filter> {
@@ -54,7 +54,7 @@ impl StrictModeVerification for SearchRequestBatch {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
for search_request in &self.searches {
search_request
.search_request

View File

@@ -13,7 +13,7 @@ use crate::collection::Collection;
use crate::common::collection_size_stats::CollectionSizeAtomicStats;
use crate::operations::payload_ops::{DeletePayload, SetPayload};
use crate::operations::point_ops::PointsSelector;
use crate::operations::types::CollectionError;
use crate::operations::types::{CollectionError, CollectionResult};
use crate::operations::vector_ops::DeleteVectors;
impl StrictModeVerification for PointsSelector {
@@ -68,7 +68,7 @@ impl StrictModeVerification for SetPayload {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
if let Some(payload_size_limit_bytes) = strict_mode_config.max_collection_payload_size_bytes
{
if let Some(local_stats) = collection.estimated_collection_stats().await {
@@ -127,7 +127,7 @@ impl StrictModeVerification for PointInsertOperations {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
check_limit_opt(
Some(self.len()),
strict_mode_config.upsert_max_batchsize,
@@ -173,7 +173,7 @@ impl StrictModeVerification for UpdateVectors {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
check_limit_opt(
Some(self.points.len()),
strict_mode_config.upsert_max_batchsize,
@@ -218,7 +218,7 @@ impl StrictModeVerification for UpdateVectors {
async fn check_collection_size_limit(
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let vector_limit = strict_mode_config.max_collection_vector_size_bytes;
let payload_limit = strict_mode_config.max_collection_payload_size_bytes;
let point_limit = strict_mode_config.max_points_count;
@@ -250,7 +250,7 @@ async fn check_collection_size_limit(
fn check_collection_points_count_limit(
points_count_limit: usize,
stats: &CollectionSizeAtomicStats,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let points_count = stats.get_points_count();
if points_count >= points_count_limit {
return Err(CollectionError::bad_request(format!(
@@ -265,7 +265,7 @@ fn check_collection_points_count_limit(
fn check_collection_vector_size_limit(
max_vec_storage_size_bytes: usize,
stats: &CollectionSizeAtomicStats,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let vec_storage_size_bytes = stats.get_vector_storage_size();
if vec_storage_size_bytes >= max_vec_storage_size_bytes {
@@ -282,7 +282,7 @@ fn check_collection_vector_size_limit(
fn check_collection_payload_size_limit(
max_payload_storage_size_bytes: usize,
stats: &CollectionSizeAtomicStats,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let payload_storage_size_bytes = stats.get_payload_storage_size();
if payload_storage_size_bytes >= max_payload_storage_size_bytes {
@@ -329,7 +329,7 @@ async fn multivector_limits_by_name(
async fn check_multivectors_limits_update(
point_insert: &UpdateVectors,
multivector_strict_config: &StrictModeMultivectorConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let Some(multivector_max_size_by_name) =
multivector_limits_by_name(multivector_strict_config).await
else {
@@ -370,7 +370,7 @@ async fn sparse_limits(
async fn check_sparse_vector_limits_update(
point_insert: &UpdateVectors,
sparse_config: &StrictModeSparseConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let Some(sparse_max_size_by_name) = sparse_limits(sparse_config).await else {
return Ok(());
};
@@ -385,7 +385,7 @@ async fn check_sparse_vector_limits_update(
async fn check_sparse_vector_limits_insert(
point_insert: &PointInsertOperations,
sparse_config: &StrictModeSparseConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let Some(sparse_max_size_by_name) = sparse_limits(sparse_config).await else {
return Ok(());
};
@@ -430,7 +430,7 @@ async fn check_sparse_vector_limits_insert(
fn check_sparse_vecstruct_limit(
vector: &VectorStruct,
sparse_max_size_by_name: &TinyMap<&VectorName, usize>,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
match vector {
VectorStruct::Named(named) => {
for (name, vec) in named {
@@ -450,7 +450,7 @@ fn check_named_sparse_vec_limit(
name: &VectorName,
vector: &Vector,
sparse_max_size_by_name: &TinyMap<&VectorName, usize>,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
if let Vector::Sparse(sparse) = vector {
if let Some(strict_sparse_limit) = sparse_max_size_by_name.get(name) {
check_sparse_vector_limit(name, sparse, *strict_sparse_limit)?;
@@ -463,7 +463,7 @@ fn check_sparse_vector_limit(
name: &VectorName,
sparse: &sparse::common::sparse_vector::SparseVector,
max_size: usize,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let vector_len = sparse.indices.len();
if vector_len > max_size || sparse.values.len() > max_size {
@@ -477,7 +477,7 @@ fn check_sparse_vector_limit(
async fn check_multivectors_limits_insert(
point_insert: &PointInsertOperations,
multivector_strict_config: &StrictModeMultivectorConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let Some(multivector_max_size_by_name) =
multivector_limits_by_name(multivector_strict_config).await
else {
@@ -546,7 +546,7 @@ fn check_named_multivectors_vecstruct_limit(
name: &VectorName,
vector: &VectorStruct,
multivector_max_size_by_name: &TinyMap<VectorNameBuf, usize>,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
match vector {
VectorStruct::MultiDense(multi) => {
check_named_multivector_limit(name, multi, multivector_max_size_by_name)
@@ -568,7 +568,7 @@ fn check_named_multivectors_vec_limit(
name: &VectorName,
vector: &Vector,
multivector_max_size_by_name: &TinyMap<VectorNameBuf, usize>,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
match vector {
Vector::MultiDense(multi) => {
check_named_multivector_limit(name, multi, multivector_max_size_by_name)
@@ -585,7 +585,7 @@ fn check_named_multivector_limit(
name: &VectorName,
multi: &MultiDenseVector,
multivector_max_size_by_name: &TinyMap<VectorNameBuf, usize>,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
if let Some(strict_multi_limit) = multivector_max_size_by_name.get(name) {
check_multivector_limit(name, multi, *strict_multi_limit)?
}
@@ -596,7 +596,7 @@ fn check_multivector_limit(
name: &VectorName,
multi: &MultiDenseVector,
max_size: usize,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let multi_len = multi.len();
if multi_len > max_size {
return Err(CollectionError::bad_request(format!(

View File

@@ -66,7 +66,7 @@ impl ChannelService {
commit: u64,
term: u64,
timeout: Duration,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let requests = self
.id_to_address
.read()
@@ -110,7 +110,7 @@ impl ChannelService {
commit: u64,
term: u64,
timeout: Duration,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
let response = self
.with_qdrant_client(peer_id, |mut client| async move {
let request = WaitOnConsensusCommitRequest {
@@ -141,7 +141,7 @@ impl ChannelService {
&self,
peer_id: PeerId,
f: impl Fn(QdrantInternalClient<InterceptedService<Channel, AddTimeout>>) -> O,
) -> Result<T, CollectionError> {
) -> CollectionResult<T> {
let address = self
.id_to_address
.read()

View File

@@ -392,7 +392,7 @@ impl LocalShard {
fusion: FusionInternal,
score_threshold: Option<f32>,
limit: usize,
) -> Result<Vec<ScoredPoint>, CollectionError> {
) -> CollectionResult<Vec<ScoredPoint>> {
let fused = match fusion {
FusionInternal::Rrf => rrf_scoring(sources),
FusionInternal::Dbsf => score_fusion(sources, ScoreFusion::dbsf()),

View File

@@ -142,10 +142,7 @@ impl ShardHolder {
Ok(())
}
pub async fn drop_and_remove_shard(
&mut self,
shard_id: ShardId,
) -> Result<(), CollectionError> {
pub async fn drop_and_remove_shard(&mut self, shard_id: ShardId) -> CollectionResult<()> {
if let Some(replica_set) = self.shards.remove(&shard_id) {
let shard_path = replica_set.shard_path.clone();
drop(replica_set);
@@ -170,7 +167,7 @@ impl ShardHolder {
&mut self,
shard_id: ShardId,
shard_key: &ShardKey,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
self.key_mapping.write_optional(|key_mapping| {
if !key_mapping.contains_key(shard_key) {
return None;
@@ -190,7 +187,7 @@ impl ShardHolder {
shard_id: ShardId,
shard: ShardReplicaSet,
shard_key: Option<ShardKey>,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
self.shards.insert(shard_id, shard);
self.rings
.entry(shard_key.clone())
@@ -217,7 +214,7 @@ impl ShardHolder {
Ok(())
}
pub async fn remove_shard_key(&mut self, shard_key: &ShardKey) -> Result<(), CollectionError> {
pub async fn remove_shard_key(&mut self, shard_key: &ShardKey) -> CollectionResult<()> {
let mut remove_shard_ids = Vec::new();
self.key_mapping.write_optional(|key_mapping| {
@@ -275,7 +272,7 @@ impl ShardHolder {
shard_ids: HashSet<ShardId>,
shard_key_mapping: ShardKeyMapping,
extra_shards: HashMap<ShardId, ShardReplicaSet>,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
self.shards.extend(extra_shards.into_iter());
let all_shard_ids = self.shards.keys().cloned().collect::<HashSet<_>>();

View File

@@ -4,7 +4,7 @@ use std::sync::Arc;
use collection::collection::{Collection, RequestShardTransfer};
use collection::config::{CollectionConfigInternal, CollectionParams, WalConfig};
use collection::operations::types::CollectionError;
use collection::operations::types::CollectionResult;
use collection::operations::vector_params_builder::VectorParamsBuilder;
use collection::optimizers_builder::OptimizersConfig;
use collection::shards::CollectionId;
@@ -87,7 +87,7 @@ pub async fn new_local_collection(
path: &Path,
snapshots_path: &Path,
config: &CollectionConfigInternal,
) -> Result<Collection, CollectionError> {
) -> CollectionResult<Collection> {
let collection = Collection::new(
id,
0,

View File

@@ -6,7 +6,7 @@ use collection::operations::conversions::write_ordering_from_proto;
use collection::operations::payload_ops::*;
use collection::operations::point_ops::*;
use collection::operations::shard_selector_internal::ShardSelectorInternal;
use collection::operations::types::{CollectionError, UpdateResult};
use collection::operations::types::{CollectionResult, UpdateResult};
use collection::operations::vector_ops::*;
use collection::operations::verification::*;
use collection::operations::*;
@@ -126,7 +126,7 @@ impl StrictModeVerification for UpdateOperation {
&self,
collection: &Collection,
strict_mode_config: &StrictModeConfig,
) -> Result<(), CollectionError> {
) -> CollectionResult<()> {
match self {
UpdateOperation::Upsert(op) => {
op.upsert