Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Consolidate MetaStorageError variants #16760

Merged
merged 1 commit into from
Nov 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion src/meta/embedded/src/meta_embedded.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl MetaEmbedded {
/// Creates a kvapi::KVApi impl with a random and unique name.
pub async fn new_temp() -> Result<MetaEmbedded, MetaStorageError> {
let temp_dir =
tempfile::tempdir().map_err(|e| MetaStorageError::SledError(AnyError::new(&e)))?;
tempfile::tempdir().map_err(|e| MetaStorageError::Damaged(AnyError::new(&e)))?;

init_temp_sled_db(temp_dir);

Expand Down
9 changes: 4 additions & 5 deletions src/meta/raft-store/src/ondisk/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use std::fmt::Debug;
pub use data_version::DataVersion;
use databend_common_meta_sled_store::sled;
use databend_common_meta_sled_store::SledTree;
use databend_common_meta_stoerr::MetaBytesError;
use databend_common_meta_stoerr::MetaStorageError;
pub use header::Header;
use log::info;
Expand Down Expand Up @@ -84,16 +83,16 @@ impl OnDisk {
let ks = tree.key_space::<DataHeader>();

let header = ks.get(&Self::KEY_HEADER.to_string()).map_err(|e| {
MetaStorageError::BytesError(MetaBytesError {
source: AnyError::error(format!(
MetaStorageError::Damaged(
AnyError::error(format!(
"Unable to read meta-service data version from disk; \
Possible reasons: opening future version meta-service with old version binary, \
or the on-disk data is damaged. \
error: {}",
e
))
.add_context(|| "open on-disk data"),
})
)
})?;
info!("Loaded header: {:?}", header);

Expand Down Expand Up @@ -177,7 +176,7 @@ impl OnDisk {

let last_snapshot = snapshot_store.load_last_snapshot().await.map_err(|e| {
let ae = AnyError::new(&e).add_context(|| "load last snapshot");
MetaStorageError::SnapshotError(ae)
MetaStorageError::Damaged(ae)
})?;

if last_snapshot.is_some() {
Expand Down
6 changes: 3 additions & 3 deletions src/meta/raft-store/src/ondisk/upgrade_to_v003.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl OnDisk {
self.convert_snapshot_v002_to_v003(snapshot_id.clone(), snapshot_data)
.await
.map_err(|e| {
MetaStorageError::snapshot_error(&e, || {
MetaStorageError::damaged(&e, || {
format!("convert v002 snapshot to v003 {}", snapshot_id)
})
})?;
Expand All @@ -76,7 +76,7 @@ impl OnDisk {

let last_snapshot = loader.load_last_snapshot().await.map_err(|e| {
let ae = AnyError::new(&e).add_context(|| "load last snapshot");
MetaStorageError::SnapshotError(ae)
MetaStorageError::Damaged(ae)
})?;

if last_snapshot.is_some() {
Expand Down Expand Up @@ -115,7 +115,7 @@ impl OnDisk {
let dir = snapshot_config.snapshot_dir();

fs::remove_dir_all(&dir).map_err(|e| {
MetaStorageError::snapshot_error(&e, || format!("removing v002 snapshot dir: {}", dir))
MetaStorageError::damaged(&e, || format!("removing v002 snapshot dir: {}", dir))
})?;
Ok(())
}
Expand Down
2 changes: 1 addition & 1 deletion src/meta/raft-store/src/sm_v003/snapshot_store_v002.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ impl From<SnapshotStoreError> for StorageError {

impl From<SnapshotStoreError> for MetaStorageError {
fn from(value: SnapshotStoreError) -> Self {
MetaStorageError::snapshot_error(&value.source, || {
MetaStorageError::damaged(&value.source, || {
format!("when {}: {}", value.verb, value.context)
})
}
Expand Down
2 changes: 1 addition & 1 deletion src/meta/service/src/meta_service/meta_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ impl MetaNode {

fn get_db_size(&self) -> Result<u64, MetaError> {
self.sto.db.size_on_disk().map_err(|e| {
let se = MetaStorageError::SledError(AnyError::new(&e).add_context(|| "get db_size"));
let se = MetaStorageError::Damaged(AnyError::new(&e).add_context(|| "get db_size"));
MetaError::StorageError(se)
})
}
Expand Down
4 changes: 2 additions & 2 deletions src/meta/service/src/store/store_inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl StoreInner {

fn to_startup_err(e: impl std::error::Error + 'static) -> MetaStartupError {
let ae = AnyError::new(&e);
let store_err = MetaStorageError::SnapshotError(ae);
let store_err = MetaStorageError::Damaged(ae);
MetaStartupError::StoreOpenError(store_err)
}

Expand Down Expand Up @@ -296,7 +296,7 @@ impl StoreInner {
pub async fn do_install_snapshot(&self, db: DB) -> Result<(), MetaStorageError> {
let mut sm = self.state_machine.write().await;
sm.install_snapshot_v003(db).await.map_err(|e| {
MetaStorageError::SnapshotError(
MetaStorageError::Damaged(
AnyError::new(&e).add_context(|| "replacing state-machine with snapshot"),
)
})?;
Expand Down
3 changes: 1 addition & 2 deletions src/meta/sled-store/src/bytes_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

use anyerror::AnyError;
use databend_common_meta_stoerr::MetaBytesError;
use databend_common_meta_stoerr::MetaStorageError;

/// Errors that occur when encode/decode
Expand Down Expand Up @@ -47,6 +46,6 @@ impl From<std::string::FromUtf8Error> for SledBytesError {
// TODO: remove this: after refactoring, sled should not use MetaStorageError directly.
impl From<SledBytesError> for MetaStorageError {
fn from(e: SledBytesError) -> Self {
MetaStorageError::BytesError(MetaBytesError::new(&e))
MetaStorageError::Damaged(AnyError::new(&e))
}
}
10 changes: 2 additions & 8 deletions src/meta/sled-store/src/sled_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,16 +188,10 @@ impl SledTree {
warn!("txn error: {:?}", meta_sto_err);

match &meta_sto_err {
MetaStorageError::BytesError(_e) => {
Err(ConflictableTransactionError::Abort(meta_sto_err))
}
MetaStorageError::SledError(_e) => {
Err(ConflictableTransactionError::Abort(meta_sto_err))
}
MetaStorageError::TransactionConflict => {
Err(ConflictableTransactionError::Conflict)
}
MetaStorageError::SnapshotError(_e) => {
MetaStorageError::Damaged(_e) => {
Err(ConflictableTransactionError::Abort(meta_sto_err))
}
}
Expand All @@ -210,7 +204,7 @@ impl SledTree {
Err(txn_err) => match txn_err {
TransactionError::Abort(meta_sto_err) => Err(meta_sto_err),
TransactionError::Storage(sto_err) => {
Err(MetaStorageError::SledError(AnyError::new(&sto_err)))
Err(MetaStorageError::Damaged(AnyError::new(&sto_err)))
}
},
}
Expand Down
1 change: 0 additions & 1 deletion src/meta/stoerr/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ test = true
[dependencies]
anyerror = { workspace = true }
databend-common-exception = { workspace = true }
prost = { workspace = true }
serde_json = { workspace = true }
sled = { workspace = true }
thiserror = { workspace = true }
Expand Down
2 changes: 0 additions & 2 deletions src/meta/stoerr/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
// See the License for the specific language governing permissions and
// limitations under the License.

pub mod meta_bytes_error;
mod meta_storage_errors;

pub use meta_bytes_error::MetaBytesError;
pub use meta_storage_errors::MetaStorageError;
55 changes: 0 additions & 55 deletions src/meta/stoerr/src/meta_bytes_error.rs

This file was deleted.

41 changes: 11 additions & 30 deletions src/meta/stoerr/src/meta_storage_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,22 +19,11 @@ use anyerror::AnyError;
use databend_common_exception::ErrorCode;
use sled::transaction::UnabortableTransactionError;

use crate::MetaBytesError;

/// Storage level error that is raised by meta service.
#[derive(Debug, Clone, PartialEq, Eq, thiserror::Error)]
pub enum MetaStorageError {
/// An error raised when encode/decode data to/from underlying storage.
#[error(transparent)]
BytesError(MetaBytesError),

/// An AnyError built from sled::Error.
#[error(transparent)]
SledError(AnyError),

/// Error that is related to snapshot
#[error(transparent)]
SnapshotError(AnyError),
#[error("Data damaged: {0}")]
Damaged(AnyError),

// TODO(1): remove this error
/// An internal error that inform txn to retry.
Expand All @@ -43,52 +32,44 @@ pub enum MetaStorageError {
}

impl MetaStorageError {
pub fn snapshot_error<D: fmt::Display, F: FnOnce() -> D>(
pub fn damaged<D: fmt::Display, F: FnOnce() -> D>(
error: &(impl std::error::Error + 'static),
context: F,
) -> Self {
MetaStorageError::SnapshotError(AnyError::new(error).add_context(context))
MetaStorageError::Damaged(AnyError::new(error).add_context(context))
}

pub fn name(&self) -> &'static str {
match self {
MetaStorageError::BytesError(_) => "BytesError",
MetaStorageError::SledError(_) => "SledError",
MetaStorageError::SnapshotError(_) => "SnapshotError",
MetaStorageError::Damaged(_) => "Damaged",
MetaStorageError::TransactionConflict => "TransactionConflict",
}
}
}

impl From<std::string::FromUtf8Error> for MetaStorageError {
fn from(error: std::string::FromUtf8Error) -> Self {
MetaStorageError::BytesError(MetaBytesError::new(&error))
MetaStorageError::Damaged(AnyError::new(&error))
}
}

impl From<serde_json::Error> for MetaStorageError {
fn from(error: serde_json::Error) -> MetaStorageError {
MetaStorageError::BytesError(MetaBytesError::new(&error))
}
}

impl From<MetaBytesError> for MetaStorageError {
fn from(error: MetaBytesError) -> Self {
MetaStorageError::BytesError(error)
MetaStorageError::Damaged(AnyError::new(&error))
}
}

impl From<sled::Error> for MetaStorageError {
fn from(e: sled::Error) -> MetaStorageError {
MetaStorageError::SledError(AnyError::new(&e))
fn from(error: sled::Error) -> MetaStorageError {
MetaStorageError::Damaged(AnyError::new(&error))
}
}

impl From<UnabortableTransactionError> for MetaStorageError {
fn from(error: UnabortableTransactionError) -> Self {
match error {
UnabortableTransactionError::Storage(e) => {
MetaStorageError::SledError(AnyError::new(&e))
UnabortableTransactionError::Storage(error) => {
MetaStorageError::Damaged(AnyError::new(&error))
}
UnabortableTransactionError::Conflict => MetaStorageError::TransactionConflict,
}
Expand Down
Loading