Merge #5089

5089: Improve error handling when writing into LMDB r=dureuill a=Kerollmops This PR exposes two new internal error variants: `StoreDelete` and `StorePut`. So that the error messages are better when we fail at writing into LMDB. Related to #5078 Co-authored-by: Clément Renault <[email protected]>
meilisearch · Nov 25, 2024 · 5560452 · 5560452
2 parents d9df7e0 + a3103f3
commit 5560452
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 9 deletions.
diff --git a/crates/milli/src/error.rs b/crates/milli/src/error.rs
@@ -61,6 +61,10 @@ pub enum InternalError {
     Serialization(#[from] SerializationError),
     #[error(transparent)]
     Store(#[from] MdbError),
+    #[error("Cannot delete {key:?} from database {database_name}: {error}")]
+    StoreDeletion { database_name: &'static str, key: Vec<u8>, error: heed::Error },
+    #[error("Cannot insert {key:?} and value with length {value_length} into database {database_name}: {error}")]
+    StorePut { database_name: &'static str, key: Vec<u8>, value_length: usize, error: heed::Error },
     #[error(transparent)]
     Utf8(#[from] str::Utf8Error),
     #[error("An indexation process was explicitly aborted")]

diff --git a/crates/milli/src/update/new/channel.rs b/crates/milli/src/update/new/channel.rs
@@ -11,7 +11,7 @@ use super::extract::FacetKind;
 use super::StdResult;
 use crate::heed_codec::facet::{FieldDocIdFacetF64Codec, FieldDocIdFacetStringCodec};
 use crate::index::main_key::{GEO_FACETED_DOCUMENTS_IDS_KEY, GEO_RTREE_KEY};
-use crate::index::IndexEmbeddingConfig;
+use crate::index::{db_name, IndexEmbeddingConfig};
 use crate::update::new::KvReaderFieldId;
 use crate::vector::Embedding;
 use crate::{DocumentId, Index};
@@ -112,7 +112,7 @@ pub enum Database {
     FacetIdIsNullDocids,
     FacetIdIsEmptyDocids,
     FacetIdExistsDocids,
-    FacetIdF64NumberDocids,
+    FacetIdF64Docids,
     FacetIdStringDocids,
     FieldIdDocidFacetStrings,
     FieldIdDocidFacetF64s,
@@ -133,18 +133,39 @@ impl Database {
             Database::FacetIdIsNullDocids => index.facet_id_is_null_docids.remap_types(),
             Database::FacetIdIsEmptyDocids => index.facet_id_is_empty_docids.remap_types(),
             Database::FacetIdExistsDocids => index.facet_id_exists_docids.remap_types(),
-            Database::FacetIdF64NumberDocids => index.facet_id_f64_docids.remap_types(),
+            Database::FacetIdF64Docids => index.facet_id_f64_docids.remap_types(),
             Database::FacetIdStringDocids => index.facet_id_string_docids.remap_types(),
             Database::FieldIdDocidFacetStrings => index.field_id_docid_facet_strings.remap_types(),
             Database::FieldIdDocidFacetF64s => index.field_id_docid_facet_f64s.remap_types(),
         }
     }
+
+    pub fn database_name(&self) -> &'static str {
+        match self {
+            Database::Main => db_name::MAIN,
+            Database::Documents => db_name::DOCUMENTS,
+            Database::ExternalDocumentsIds => db_name::EXTERNAL_DOCUMENTS_IDS,
+            Database::ExactWordDocids => db_name::EXACT_WORD_DOCIDS,
+            Database::WordDocids => db_name::WORD_DOCIDS,
+            Database::WordFidDocids => db_name::WORD_FIELD_ID_DOCIDS,
+            Database::WordPositionDocids => db_name::WORD_POSITION_DOCIDS,
+            Database::FidWordCountDocids => db_name::FIELD_ID_WORD_COUNT_DOCIDS,
+            Database::WordPairProximityDocids => db_name::WORD_PAIR_PROXIMITY_DOCIDS,
+            Database::FacetIdIsNullDocids => db_name::FACET_ID_IS_NULL_DOCIDS,
+            Database::FacetIdIsEmptyDocids => db_name::FACET_ID_IS_EMPTY_DOCIDS,
+            Database::FacetIdExistsDocids => db_name::FACET_ID_EXISTS_DOCIDS,
+            Database::FacetIdF64Docids => db_name::FACET_ID_F64_DOCIDS,
+            Database::FacetIdStringDocids => db_name::FACET_ID_STRING_DOCIDS,
+            Database::FieldIdDocidFacetStrings => db_name::FIELD_ID_DOCID_FACET_STRINGS,
+            Database::FieldIdDocidFacetF64s => db_name::FIELD_ID_DOCID_FACET_F64S,
+        }
+    }
 }
 
 impl From<FacetKind> for Database {
     fn from(value: FacetKind) -> Self {
         match value {
-            FacetKind::Number => Database::FacetIdF64NumberDocids,
+            FacetKind::Number => Database::FacetIdF64Docids,
             FacetKind::String => Database::FacetIdStringDocids,
             FacetKind::Null => Database::FacetIdIsNullDocids,
             FacetKind::Empty => Database::FacetIdIsEmptyDocids,
@@ -158,6 +179,10 @@ impl DbOperation {
         self.database.database(index)
     }
 
+    pub fn database_name(&self) -> &'static str {
+        self.database.database_name()
+    }
+
     pub fn entry(self) -> EntryOperation {
         self.entry
     }

diff --git a/crates/milli/src/update/new/indexer/mod.rs b/crates/milli/src/update/new/indexer/mod.rs
@@ -41,7 +41,7 @@ use crate::update::settings::InnerIndexSettings;
 use crate::update::{FacetsUpdateBulk, GrenadParameters};
 use crate::vector::{ArroyWrapper, EmbeddingConfigs, Embeddings};
 use crate::{
-    FieldsIdsMap, GlobalFieldsIdsMap, Index, InternalError, Result, ThreadPoolNoAbort,
+    Error, FieldsIdsMap, GlobalFieldsIdsMap, Index, InternalError, Result, ThreadPoolNoAbort,
     ThreadPoolNoAbortBuilder, UserError,
 };
 
@@ -356,13 +356,29 @@ where
             match operation {
                 WriterOperation::DbOperation(db_operation) => {
                     let database = db_operation.database(index);
+                    let database_name = db_operation.database_name();
                     match db_operation.entry() {
-                        EntryOperation::Delete(e) => {
-                            if !database.delete(wtxn, e.entry())? {
-                                unreachable!("We tried to delete an unknown key")
+                        EntryOperation::Delete(e) => match database.delete(wtxn, e.entry()) {
+                            Ok(false) => unreachable!("We tried to delete an unknown key"),
+                            Ok(_) => (),
+                            Err(error) => {
+                                return Err(Error::InternalError(InternalError::StoreDeletion {
+                                    database_name,
+                                    key: e.entry().to_owned(),
+                                    error,
+                                }));
+                            }
+                        },
+                        EntryOperation::Write(e) => {
+                            if let Err(error) = database.put(wtxn, e.key(), e.value()) {
+                                return Err(Error::InternalError(InternalError::StorePut {
+                                    database_name,
+                                    key: e.key().to_owned(),
+                                    value_length: e.value().len(),
+                                    error,
+                                }));
                             }
                         }
-                        EntryOperation::Write(e) => database.put(wtxn, e.key(), e.value())?,
                     }
                 }
                 WriterOperation::ArroyOperation(arroy_operation) => match arroy_operation {