From b2be35c0ff688feb90188c1f85ca456090cbf066 Mon Sep 17 00:00:00 2001 From: Alexey Shekhirin Date: Mon, 14 Aug 2023 14:09:01 +0100 Subject: [PATCH] feat(interfaces): database write error details (#4190) --- crates/interfaces/src/db.rs | 32 ++++++++++--- .../db/src/implementation/mdbx/cursor.rs | 47 ++++++++++++++----- .../storage/db/src/implementation/mdbx/mod.rs | 36 ++++++++++++-- .../storage/db/src/implementation/mdbx/tx.rs | 11 ++++- crates/storage/db/src/lib.rs | 2 +- 5 files changed, 104 insertions(+), 24 deletions(-) diff --git a/crates/interfaces/src/db.rs b/crates/interfaces/src/db.rs index 5249a0ed5cc3..e0f44d7276e9 100644 --- a/crates/interfaces/src/db.rs +++ b/crates/interfaces/src/db.rs @@ -1,4 +1,4 @@ -/// Database error type. They are using u32 to represent error code. +/// Database error type. It uses i32 to represent an error code. #[derive(Debug, thiserror::Error, PartialEq, Eq, Clone)] pub enum DatabaseError { /// Failed to open database. @@ -7,13 +7,22 @@ pub enum DatabaseError { /// Failed to create a table in database. #[error("Table Creating error code: {0:?}")] TableCreation(i32), - /// Failed to insert a value into a table. - #[error("Database write error code: {0:?}")] - Write(i32), - /// Failed to get a value into a table. + /// Failed to write a value into a table. + #[error("Database write operation \"{operation:?}\" for key \"{key:?}\" in table \"{table_name}\" ended with error code: {code:?}")] + Write { + /// Database error code + code: i32, + /// Database write operation type + operation: DatabaseWriteOperation, + /// Table name + table_name: &'static str, + /// Write key + key: Box<[u8]>, + }, + /// Failed to read a value from a table. #[error("Database read error code: {0:?}")] Read(i32), - /// Failed to delete a `(key, value)` pair into a table. + /// Failed to delete a `(key, value)` pair from a table. #[error("Database delete error code: {0:?}")] Delete(i32), /// Failed to commit transaction changes into the database. @@ -36,6 +45,17 @@ pub enum DatabaseError { LogLevelUnavailable(LogLevel), } +/// Database write operation type +#[derive(Debug, PartialEq, Eq, Clone)] +#[allow(missing_docs)] +pub enum DatabaseWriteOperation { + CursorAppend, + CursorUpsert, + CursorInsert, + CursorAppendDup, + Put, +} + #[derive(Debug, PartialEq, Eq, Clone, Copy)] #[cfg_attr(feature = "clap", derive(clap::ValueEnum))] /// Database log level. diff --git a/crates/storage/db/src/implementation/mdbx/cursor.rs b/crates/storage/db/src/implementation/mdbx/cursor.rs index ea7cfe43094b..18d8cef1e95a 100644 --- a/crates/storage/db/src/implementation/mdbx/cursor.rs +++ b/crates/storage/db/src/implementation/mdbx/cursor.rs @@ -1,5 +1,6 @@ //! Cursor wrapper for libmdbx-sys. +use reth_interfaces::db::DatabaseWriteOperation; use std::{borrow::Cow, collections::Bound, marker::PhantomData, ops::RangeBounds}; use crate::{ @@ -230,24 +231,42 @@ impl<'tx, T: Table> DbCursorRW<'tx, T> for Cursor<'tx, RW, T> { /// to properly upsert, you'll need to `seek_exact` & `delete_current` if the key+subkey was /// found, before calling `upsert`. fn upsert(&mut self, key: T::Key, value: T::Value) -> Result<(), DatabaseError> { + let key = key.encode(); // Default `WriteFlags` is UPSERT - self.inner - .put(key.encode().as_ref(), compress_or_ref!(self, value), WriteFlags::UPSERT) - .map_err(|e| DatabaseError::Write(e.into())) + self.inner.put(key.as_ref(), compress_or_ref!(self, value), WriteFlags::UPSERT).map_err( + |e| DatabaseError::Write { + code: e.into(), + operation: DatabaseWriteOperation::CursorUpsert, + table_name: T::NAME, + key: Box::from(key.as_ref()), + }, + ) } fn insert(&mut self, key: T::Key, value: T::Value) -> Result<(), DatabaseError> { + let key = key.encode(); self.inner - .put(key.encode().as_ref(), compress_or_ref!(self, value), WriteFlags::NO_OVERWRITE) - .map_err(|e| DatabaseError::Write(e.into())) + .put(key.as_ref(), compress_or_ref!(self, value), WriteFlags::NO_OVERWRITE) + .map_err(|e| DatabaseError::Write { + code: e.into(), + operation: DatabaseWriteOperation::CursorInsert, + table_name: T::NAME, + key: Box::from(key.as_ref()), + }) } /// Appends the data to the end of the table. Consequently, the append operation /// will fail if the inserted key is less than the last table key fn append(&mut self, key: T::Key, value: T::Value) -> Result<(), DatabaseError> { - self.inner - .put(key.encode().as_ref(), compress_or_ref!(self, value), WriteFlags::APPEND) - .map_err(|e| DatabaseError::Write(e.into())) + let key = key.encode(); + self.inner.put(key.as_ref(), compress_or_ref!(self, value), WriteFlags::APPEND).map_err( + |e| DatabaseError::Write { + code: e.into(), + operation: DatabaseWriteOperation::CursorAppend, + table_name: T::NAME, + key: Box::from(key.as_ref()), + }, + ) } fn delete_current(&mut self) -> Result<(), DatabaseError> { @@ -261,8 +280,14 @@ impl<'tx, T: DupSort> DbDupCursorRW<'tx, T> for Cursor<'tx, RW, T> { } fn append_dup(&mut self, key: T::Key, value: T::Value) -> Result<(), DatabaseError> { - self.inner - .put(key.encode().as_ref(), compress_or_ref!(self, value), WriteFlags::APPEND_DUP) - .map_err(|e| DatabaseError::Write(e.into())) + let key = key.encode(); + self.inner.put(key.as_ref(), compress_or_ref!(self, value), WriteFlags::APPEND_DUP).map_err( + |e| DatabaseError::Write { + code: e.into(), + operation: DatabaseWriteOperation::CursorAppendDup, + table_name: T::NAME, + key: Box::from(key.as_ref()), + }, + ) } } diff --git a/crates/storage/db/src/implementation/mdbx/mod.rs b/crates/storage/db/src/implementation/mdbx/mod.rs index 9bf544279437..b9cce246938c 100644 --- a/crates/storage/db/src/implementation/mdbx/mod.rs +++ b/crates/storage/db/src/implementation/mdbx/mod.rs @@ -158,6 +158,7 @@ impl Deref for Env { mod tests { use super::*; use crate::{ + abstraction::table::{Encode, Table}, cursor::{DbCursorRO, DbCursorRW, DbDupCursorRO, DbDupCursorRW, ReverseWalker, Walker}, database::Database, models::{AccountBeforeTx, ShardedKey}, @@ -166,6 +167,7 @@ mod tests { transaction::{DbTx, DbTxMut}, AccountChangeSet, DatabaseError, }; + use reth_interfaces::db::DatabaseWriteOperation; use reth_libmdbx::{NoWriteMap, WriteMap}; use reth_primitives::{Account, Address, Header, IntegerList, StorageEntry, H160, H256, U256}; use std::{path::Path, str::FromStr, sync::Arc}; @@ -525,7 +527,15 @@ mod tests { assert_eq!(cursor.current(), Ok(Some((key_to_insert, H256::zero())))); // INSERT (failure) - assert_eq!(cursor.insert(key_to_insert, H256::zero()), Err(DatabaseError::Write(-30799))); + assert_eq!( + cursor.insert(key_to_insert, H256::zero()), + Err(DatabaseError::Write { + code: -30799, + operation: DatabaseWriteOperation::CursorInsert, + table_name: CanonicalHeaders::NAME, + key: Box::from(key_to_insert.encode().as_ref()) + }) + ); assert_eq!(cursor.current(), Ok(Some((key_to_insert, H256::zero())))); tx.commit().expect(ERROR_COMMIT); @@ -660,7 +670,15 @@ mod tests { let key_to_append = 2; let tx = db.tx_mut().expect(ERROR_INIT_TX); let mut cursor = tx.cursor_write::().unwrap(); - assert_eq!(cursor.append(key_to_append, H256::zero()), Err(DatabaseError::Write(-30418))); + assert_eq!( + cursor.append(key_to_append, H256::zero()), + Err(DatabaseError::Write { + code: -30418, + operation: DatabaseWriteOperation::CursorAppend, + table_name: CanonicalHeaders::NAME, + key: Box::from(key_to_append.encode().as_ref()) + }) + ); assert_eq!(cursor.current(), Ok(Some((5, H256::zero())))); // the end of table tx.commit().expect(ERROR_COMMIT); @@ -735,14 +753,24 @@ mod tests { transition_id, AccountBeforeTx { address: Address::from_low_u64_be(subkey_to_append), info: None } ), - Err(DatabaseError::Write(-30418)) + Err(DatabaseError::Write { + code: -30418, + operation: DatabaseWriteOperation::CursorAppendDup, + table_name: AccountChangeSet::NAME, + key: Box::from(transition_id.encode().as_ref()) + }) ); assert_eq!( cursor.append( transition_id - 1, AccountBeforeTx { address: Address::from_low_u64_be(subkey_to_append), info: None } ), - Err(DatabaseError::Write(-30418)) + Err(DatabaseError::Write { + code: -30418, + operation: DatabaseWriteOperation::CursorAppend, + table_name: AccountChangeSet::NAME, + key: Box::from((transition_id - 1).encode().as_ref()) + }) ); assert_eq!( cursor.append( diff --git a/crates/storage/db/src/implementation/mdbx/tx.rs b/crates/storage/db/src/implementation/mdbx/tx.rs index 3a49cc04a97a..6e8558726c80 100644 --- a/crates/storage/db/src/implementation/mdbx/tx.rs +++ b/crates/storage/db/src/implementation/mdbx/tx.rs @@ -8,6 +8,7 @@ use crate::{ DatabaseError, }; use parking_lot::RwLock; +use reth_interfaces::db::DatabaseWriteOperation; use reth_libmdbx::{ffi::DBI, EnvironmentKind, Transaction, TransactionKind, WriteFlags, RW}; use reth_metrics::metrics::{self, histogram}; use std::{marker::PhantomData, str::FromStr, sync::Arc, time::Instant}; @@ -124,9 +125,15 @@ impl<'tx, K: TransactionKind, E: EnvironmentKind> DbTx<'tx> for Tx<'tx, K, E> { impl DbTxMut<'_> for Tx<'_, RW, E> { fn put(&self, key: T::Key, value: T::Value) -> Result<(), DatabaseError> { + let key = key.encode(); self.inner - .put(self.get_dbi::()?, &key.encode(), &value.compress(), WriteFlags::UPSERT) - .map_err(|e| DatabaseError::Write(e.into())) + .put(self.get_dbi::()?, key.as_ref(), &value.compress(), WriteFlags::UPSERT) + .map_err(|e| DatabaseError::Write { + code: e.into(), + operation: DatabaseWriteOperation::Put, + table_name: T::NAME, + key: Box::from(key.as_ref()), + }) } fn delete( diff --git a/crates/storage/db/src/lib.rs b/crates/storage/db/src/lib.rs index 76650de0385c..725b2c9dbf6b 100644 --- a/crates/storage/db/src/lib.rs +++ b/crates/storage/db/src/lib.rs @@ -84,7 +84,7 @@ pub mod mdbx { } pub use abstraction::*; -pub use reth_interfaces::db::DatabaseError; +pub use reth_interfaces::db::{DatabaseError, DatabaseWriteOperation}; pub use tables::*; pub use utils::is_database_empty;