diff --git a/meilisearch/src/search_queue.rs b/meilisearch/src/search_queue.rs index 0fe9a5a53..415da0c15 100644 --- a/meilisearch/src/search_queue.rs +++ b/meilisearch/src/search_queue.rs @@ -85,11 +85,13 @@ impl SearchQueue { }, search_request = receive_new_searches.recv() => { - if search_request.is_none() { - continue; - } - // this unwrap is safe because we're sure the `SearchQueue` still lives somewhere in actix-web - let search_request = search_request.unwrap(); + let search_request = match search_request { + Some(search_request) => search_request, + // This should never happen while actix-web is running, but it's not a reason to crash + // and it can generate a lot of noise in the tests. + None => continue, + }; + if searches_running < usize::from(parallelism) && queue.is_empty() { searches_running += 1; // if the search requests die it's not a hard error on our side diff --git a/milli/src/error.rs b/milli/src/error.rs index e4550de1f..009781fcf 100644 --- a/milli/src/error.rs +++ b/milli/src/error.rs @@ -32,6 +32,8 @@ pub enum InternalError { DatabaseClosing, #[error("Missing {} in the {db_name} database.", key.unwrap_or("key"))] DatabaseMissingEntry { db_name: &'static str, key: Option<&'static str> }, + #[error("Missing {key} in the fieldids weights mapping.")] + FieldidsWeightsMapMissingEntry { key: FieldId }, #[error(transparent)] FieldIdMapMissingEntry(#[from] FieldIdMapMissingEntry), #[error("Missing {key} in the field id mapping.")] diff --git a/milli/src/fieldids_weights_map.rs b/milli/src/fieldids_weights_map.rs index fdfe8fba2..72720a02a 100644 --- a/milli/src/fieldids_weights_map.rs +++ b/milli/src/fieldids_weights_map.rs @@ -1,3 +1,5 @@ +//! The fieldids weights map is in charge of storing linking the searchable fields with their weights. + use std::collections::HashMap; use serde::{Deserialize, Serialize}; @@ -10,22 +12,29 @@ pub struct FieldidsWeightsMap { } impl FieldidsWeightsMap { + /// Insert a field id -> weigth into the map. + /// If the map did not have this key present, `None` is returned. + /// If the map did have this key present, the value is updated, and the old value is returned. pub fn insert(&mut self, fid: FieldId, weight: Weight) -> Option { self.map.insert(fid, weight) } + /// Removes a field id from the map, returning the associated weight previously in the map. pub fn remove(&mut self, fid: FieldId) -> Option { self.map.remove(&fid) } + /// Returns weight corresponding to the key. pub fn weight(&self, fid: FieldId) -> Option { self.map.get(&fid).copied() } + /// Returns highest weight contained in the map if any. pub fn max_weight(&self) -> Option { self.map.values().copied().max() } + /// Return an iterator visiting all field ids in arbitrary order. pub fn ids(&self) -> impl Iterator + '_ { self.map.keys().copied() } diff --git a/milli/src/index.rs b/milli/src/index.rs index 7fe9da0ff..c565cdd5b 100644 --- a/milli/src/index.rs +++ b/milli/src/index.rs @@ -26,9 +26,9 @@ use crate::proximity::ProximityPrecision; use crate::vector::EmbeddingConfig; use crate::{ default_criteria, CboRoaringBitmapCodec, Criterion, DocumentId, ExternalDocumentsIds, - FacetDistribution, FieldDistribution, FieldId, FieldIdWordCountCodec, FieldidsWeightsMap, - GeoPoint, ObkvCodec, Result, RoaringBitmapCodec, RoaringBitmapLenCodec, Search, U8StrStrCodec, - Weight, BEU16, BEU32, BEU64, + FacetDistribution, FieldDistribution, FieldId, FieldIdMapMissingEntry, FieldIdWordCountCodec, + FieldidsWeightsMap, GeoPoint, ObkvCodec, Result, RoaringBitmapCodec, RoaringBitmapLenCodec, + Search, U8StrStrCodec, Weight, BEU16, BEU32, BEU64, }; pub const DEFAULT_MIN_WORD_LEN_ONE_TYPO: u8 = 5; @@ -446,22 +446,25 @@ impl Index { pub fn searchable_fields_and_weights<'a>( &self, rtxn: &'a RoTxn, - ) -> heed::Result, FieldId, Weight)>> { + ) -> Result, FieldId, Weight)>> { let fid_map = self.fields_ids_map(rtxn)?; let weight_map = self.fieldids_weights_map(rtxn)?; let searchable = self.searchable_fields(rtxn)?; - Ok(searchable + searchable .into_iter() - .map(|field| { - // the searchable attributes are a subset of the field id map - let fid = fid_map.id(&field).unwrap(); - // all the searchable fields have a weight - let weight = weight_map.weight(fid).unwrap(); + .map(|field| -> Result<_> { + let fid = fid_map.id(&field).ok_or_else(|| FieldIdMapMissingEntry::FieldName { + field_name: field.to_string(), + process: "searchable_fields_and_weights", + })?; + let weight = weight_map + .weight(fid) + .ok_or(InternalError::FieldidsWeightsMapMissingEntry { key: fid })?; - (field, fid, weight) + Ok((field, fid, weight)) }) - .collect()) + .collect() } /* geo rtree */ diff --git a/milli/src/search/new/ranking_rule_graph/fid/mod.rs b/milli/src/search/new/ranking_rule_graph/fid/mod.rs index e10f2fbab..a4a08ea46 100644 --- a/milli/src/search/new/ranking_rule_graph/fid/mod.rs +++ b/milli/src/search/new/ranking_rule_graph/fid/mod.rs @@ -7,7 +7,7 @@ use crate::search::new::interner::{DedupInterner, Interned}; use crate::search::new::query_term::LocatedQueryTermSubset; use crate::search::new::resolve_query_graph::compute_query_term_subset_docids_within_field_id; use crate::search::new::SearchContext; -use crate::{FieldId, Result}; +use crate::{FieldId, InternalError, Result}; #[derive(Clone, PartialEq, Eq, Hash)] pub struct FidCondition { @@ -29,10 +29,9 @@ impl RankingRuleGraphTrait for FidGraph { let docids = if let Some(fid) = condition.fid { // maybe compute_query_term_subset_docids_within_field_id should accept a universe as argument - let mut docids = + let docids = compute_query_term_subset_docids_within_field_id(ctx, &term.term_subset, fid)?; - docids &= universe; - docids + docids & universe } else { RoaringBitmap::new() }; @@ -75,7 +74,9 @@ impl RankingRuleGraphTrait for FidGraph { let mut edges = vec![]; for fid in all_fields.iter().copied() { - let weight = weights_map.weight(fid).unwrap(); + let weight = weights_map + .weight(fid) + .ok_or(InternalError::FieldidsWeightsMapMissingEntry { key: fid })?; edges.push(( weight as u32 * term.term_ids.len() as u32, conditions_interner.insert(FidCondition { term: term.clone(), fid: Some(fid) }), diff --git a/milli/src/update/settings.rs b/milli/src/update/settings.rs index 2e8ac157c..c66148813 100644 --- a/milli/src/update/settings.rs +++ b/milli/src/update/settings.rs @@ -475,33 +475,25 @@ impl<'a, 't, 'i> Settings<'a, 't, 'i> { return Ok(false); } - // every time the searchable attributes are updated, we need to update the - // ids for any settings that uses the facets. (distinct_fields, filterable_fields). - let old_fields_ids_map = self.index.fields_ids_map(self.wtxn)?; - // Since we're updating the settings we can only add new fields at the end of the field id map - let mut new_fields_ids_map = old_fields_ids_map.clone(); - let names = fields - .iter() - // fields are deduplicated, only the first occurrence is taken into account - .unique() - .map(String::as_str) - .collect::>(); + let mut fields_ids_map = self.index.fields_ids_map(self.wtxn)?; + // fields are deduplicated, only the first occurrence is taken into account + let names = fields.iter().unique().map(String::as_str).collect::>(); // Add all the searchable attributes to the field map, and then add the // remaining fields from the old field map to the new one for name in names.iter() { // The fields ids map won't change the field id of already present elements thus only the // new fields will be inserted. - new_fields_ids_map.insert(name).ok_or(UserError::AttributeLimitReached)?; + fields_ids_map.insert(name).ok_or(UserError::AttributeLimitReached)?; } self.index.put_all_searchable_fields_from_fields_ids_map( self.wtxn, Some(&names), - &new_fields_ids_map, + &fields_ids_map, )?; - self.index.put_fields_ids_map(self.wtxn, &new_fields_ids_map)?; + self.index.put_fields_ids_map(self.wtxn, &fields_ids_map)?; Ok(true) } Setting::Reset => Ok(self.index.delete_all_searchable_fields(self.wtxn)?),