Skip to content

Commit ce88fa8

Browse files
committed
refactor(meta): remove MetaError::Fatal.
Crate meta/types: remove error `MetaError::Fatal`. Usages in test are replaced with panic. Usages in meta-service startup is replaced with `AnyError`, since no one really cares about the type of this error. Crate meta/api: - Refactor: meta/api: rename mod `kv_api_utils` to `util`. - Refactor: move `get_kv_data()` to mod `testing`, it is only used for tests, and will panic if the expected key does not exist.
1 parent bd95e0e commit ce88fa8

File tree

9 files changed

+85
-74
lines changed

9 files changed

+85
-74
lines changed

src/meta/api/src/lib.rs

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ mod id_generator;
2020
mod kv_api;
2121
mod kv_api_key;
2222
mod kv_api_test_suite;
23-
mod kv_api_utils;
2423
mod schema_api;
2524
mod schema_api_impl;
2625
mod schema_api_keys;
@@ -29,6 +28,8 @@ mod share_api;
2928
mod share_api_impl;
3029
mod share_api_keys;
3130
mod share_api_test_suite;
31+
pub(crate) mod testing;
32+
pub(crate) mod util;
3233

3334
pub use id::Id;
3435
pub(crate) use id_generator::IdGenerator;
@@ -40,34 +41,32 @@ pub use kv_api::KVApi;
4041
pub use kv_api_key::KVApiKey;
4142
pub use kv_api_key::KVApiKeyError;
4243
pub use kv_api_test_suite::KVApiTestSuite;
43-
pub use kv_api_utils::db_has_to_exist;
44-
pub use kv_api_utils::deserialize_struct;
45-
pub use kv_api_utils::deserialize_u64;
46-
pub use kv_api_utils::fetch_id;
47-
pub use kv_api_utils::get_kv_data;
48-
pub use kv_api_utils::get_object_shared_by_share_ids;
49-
pub use kv_api_utils::get_share_account_meta_or_err;
50-
pub use kv_api_utils::get_share_database_id_and_privilege;
51-
pub use kv_api_utils::get_share_id_to_name_or_err;
52-
pub use kv_api_utils::get_share_meta_by_id_or_err;
53-
pub use kv_api_utils::get_share_or_err;
54-
pub use kv_api_utils::get_struct_value;
55-
pub use kv_api_utils::get_u64_value;
56-
pub use kv_api_utils::is_all_db_data_removed;
57-
pub use kv_api_utils::is_db_need_to_be_remove;
58-
pub use kv_api_utils::list_keys;
59-
pub use kv_api_utils::list_u64_value;
60-
pub use kv_api_utils::send_txn;
61-
pub use kv_api_utils::serialize_struct;
62-
pub use kv_api_utils::serialize_u64;
63-
pub use kv_api_utils::table_has_to_exist;
64-
pub use kv_api_utils::txn_cond_seq;
65-
pub use kv_api_utils::txn_op_del;
66-
pub use kv_api_utils::txn_op_put;
67-
pub use kv_api_utils::txn_op_put_with_expire;
68-
pub use kv_api_utils::TXN_MAX_RETRY_TIMES;
6944
pub use schema_api::SchemaApi;
7045
pub(crate) use schema_api_impl::get_db_or_err;
7146
pub use schema_api_test_suite::SchemaApiTestSuite;
7247
pub use share_api::ShareApi;
7348
pub use share_api_test_suite::ShareApiTestSuite;
49+
pub use util::db_has_to_exist;
50+
pub use util::deserialize_struct;
51+
pub use util::fetch_id;
52+
pub use util::get_object_shared_by_share_ids;
53+
pub use util::get_share_account_meta_or_err;
54+
pub use util::get_share_database_id_and_privilege;
55+
pub use util::get_share_id_to_name_or_err;
56+
pub use util::get_share_meta_by_id_or_err;
57+
pub use util::get_share_or_err;
58+
pub use util::get_struct_value;
59+
pub use util::get_u64_value;
60+
pub use util::is_all_db_data_removed;
61+
pub use util::is_db_need_to_be_remove;
62+
pub use util::list_keys;
63+
pub use util::list_u64_value;
64+
pub use util::send_txn;
65+
pub use util::serialize_struct;
66+
pub use util::serialize_u64;
67+
pub use util::table_has_to_exist;
68+
pub use util::txn_cond_seq;
69+
pub use util::txn_op_del;
70+
pub use util::txn_op_put;
71+
pub use util::txn_op_put_with_expire;
72+
pub use util::TXN_MAX_RETRY_TIMES;

src/meta/api/src/schema_api_test_suite.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,9 @@ use common_meta_types::UpsertKVReq;
7474
use tracing::debug;
7575
use tracing::info;
7676

77-
use crate::get_kv_data;
7877
use crate::is_all_db_data_removed;
7978
use crate::serialize_struct;
79+
use crate::testing::get_kv_data;
8080
use crate::ApiBuilder;
8181
use crate::AsKVApi;
8282
use crate::KVApi;

src/meta/api/src/share_api_test_suite.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ use common_meta_types::MetaError;
2525
use enumflags2::BitFlags;
2626
use tracing::info;
2727

28-
use crate::get_kv_data;
2928
use crate::get_object_shared_by_share_ids;
3029
use crate::get_share_account_meta_or_err;
3130
use crate::get_share_id_to_name_or_err;
3231
use crate::get_share_meta_by_id_or_err;
3332
use crate::get_share_or_err;
3433
use crate::is_all_db_data_removed;
34+
use crate::testing::get_kv_data;
3535
use crate::ApiBuilder;
3636
use crate::AsKVApi;
3737
use crate::KVApi;

src/meta/api/src/testing.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// Copyright 2022 Datafuse Labs.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
//! Supporting utilities for tests.
16+
17+
use common_meta_types::MetaError;
18+
use common_proto_conv::FromToProto;
19+
20+
use crate::KVApi;
21+
use crate::KVApiKey;
22+
23+
/// Get existing value by key. Panic if key is absent.
24+
pub(crate) async fn get_kv_data<T>(
25+
kv_api: &(impl KVApi + ?Sized),
26+
key: &impl KVApiKey,
27+
) -> Result<T, MetaError>
28+
where
29+
T: FromToProto,
30+
T::PB: common_protos::prost::Message + Default,
31+
{
32+
let res = kv_api.get_kv(&key.to_key()).await?;
33+
if let Some(res) = res {
34+
let s = crate::deserialize_struct(&res.data)?;
35+
return Ok(s);
36+
};
37+
38+
unreachable!("get_kv expects non-None for key: {}", key.to_key())
39+
}

src/meta/api/src/kv_api_utils.rs renamed to src/meta/api/src/util.rs

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ use common_meta_app::schema::DatabaseMeta;
2020
use common_meta_app::schema::DatabaseNameIdent;
2121
use common_meta_app::schema::TableNameIdent;
2222
use common_meta_app::share::*;
23-
use common_meta_types::anyerror::AnyError;
2423
use common_meta_types::app_error::AppError;
2524
use common_meta_types::app_error::ShareHasNoGrantedDatabase;
2625
use common_meta_types::app_error::UnknownDatabase;
@@ -525,28 +524,6 @@ where
525524
Ok((false, None))
526525
}
527526

528-
/// Get existing value by key. Panic if key is absent.
529-
/// This function is only used for testing.
530-
pub async fn get_kv_data<T>(
531-
kv_api: &(impl KVApi + ?Sized),
532-
key: &impl KVApiKey,
533-
) -> Result<T, MetaError>
534-
where
535-
T: FromToProto,
536-
T::PB: common_protos::prost::Message + Default,
537-
{
538-
let res = kv_api.get_kv(&key.to_key()).await?;
539-
if let Some(res) = res {
540-
let s = deserialize_struct(&res.data)?;
541-
return Ok(s);
542-
};
543-
544-
Err(MetaError::Fatal(AnyError::error(format!(
545-
"failed to get {}",
546-
key.to_key()
547-
))))
548-
}
549-
550527
pub async fn get_object_shared_by_share_ids(
551528
kv_api: &(impl KVApi + ?Sized),
552529
object: &ShareGrantObject,

src/meta/client/src/grpc_client.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,10 @@ impl ClientHandle {
171171
label_increment_gauge_with_val_and_labels(META_GRPC_CLIENT_REQUEST_INFLIGHT, vec![], 1.0);
172172

173173
let res = self.req_tx.send(req).await.map_err(|e| {
174-
MetaError::Fatal(
174+
let cli_err = MetaClientError::ClientRuntimeError(
175175
AnyError::new(&e).add_context(|| "when sending req to MetaGrpcClient worker"),
176-
)
176+
);
177+
MetaError::ClientError(cli_err)
177178
});
178179

179180
if let Err(err) = res {
@@ -193,9 +194,10 @@ impl ClientHandle {
193194
1.0,
194195
);
195196

196-
MetaError::Fatal(
197+
let cli_err = MetaClientError::ClientRuntimeError(
197198
AnyError::new(&e).add_context(|| "when recv resp from MetaGrpcClient worker"),
198-
)
199+
);
200+
MetaError::ClientError(cli_err)
199201
})?;
200202

201203
label_decrement_gauge_with_val_and_labels(META_GRPC_CLIENT_REQUEST_INFLIGHT, vec![], 1.0);

src/meta/service/src/meta_service/raftmeta.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ use common_meta_types::MetaStartupError;
5858
use common_meta_types::MetaStorageError;
5959
use common_meta_types::Node;
6060
use common_meta_types::NodeId;
61-
use common_meta_types::ToMetaError;
6261
use openraft::Config;
6362
use openraft::LogId;
6463
use openraft::Raft;
@@ -132,7 +131,7 @@ pub struct MetaNode {
132131
pub raft: MetaRaft,
133132
pub running_tx: watch::Sender<()>,
134133
pub running_rx: watch::Receiver<()>,
135-
pub join_handles: Mutex<Vec<JoinHandle<MetaResult<()>>>>,
134+
pub join_handles: Mutex<Vec<JoinHandle<Result<(), AnyError>>>>,
136135
pub joined_tasks: AtomicI32,
137136
}
138137

@@ -322,12 +321,11 @@ impl MetaNode {
322321
);
323322
})
324323
.await
325-
.map_error_to_meta_error(
326-
|s| MetaError::Fatal(AnyError::error(s)),
327-
|| "fail to serve",
328-
)?;
324+
.map_err(|e| {
325+
AnyError::new(&e).add_context(|| "when serving meta-service grpc service")
326+
})?;
329327

330-
Ok::<(), MetaError>(())
328+
Ok::<(), AnyError>(())
331329
});
332330

333331
let mut jh = mn.join_handles.lock().await;
@@ -484,12 +482,17 @@ impl MetaNode {
484482
set_meta_metrics_current_term(mm.current_term);
485483
set_meta_metrics_last_log_index(mm.last_log_index.unwrap_or_default());
486484
set_meta_metrics_proposals_applied(mm.last_applied.unwrap_or_default().index);
487-
set_meta_metrics_last_seq(meta_node.get_last_seq().await?);
485+
set_meta_metrics_last_seq(
486+
meta_node
487+
.get_last_seq()
488+
.await
489+
.map_err(|e| AnyError::new(&e))?,
490+
);
488491

489492
last_leader = mm.current_leader;
490493
}
491494

492-
Ok::<(), MetaError>(())
495+
Ok::<(), AnyError>(())
493496
};
494497

495498
let span = tracing::span!(tracing::Level::INFO, "watch-metrics");

src/meta/types/src/meta_errors.rs

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
// See the License for the specific language governing permissions and
1313
// limitations under the License.
1414

15-
use anyerror::AnyError;
1615
use serde::Deserialize;
1716
use serde::Serialize;
1817
use thiserror::Error;
@@ -40,11 +39,6 @@ pub enum MetaError {
4039

4140
#[error(transparent)]
4241
AppError(#[from] AppError),
43-
44-
/// Any other unclassified error.
45-
/// Other crate may return general error such as ErrorCode or anyhow::Error, which can not be classified by type.
46-
#[error(transparent)]
47-
Fatal(AnyError),
4842
}
4943

5044
pub type MetaResult<T> = Result<T, MetaError>;

src/meta/types/src/meta_errors_into.rs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,6 @@ impl From<MetaError> for ErrorCode {
3232
MetaError::StorageError(sto_err) => {
3333
ErrorCode::MetaServiceError(sto_err.to_string()).set_backtrace(sto_err.backtrace())
3434
}
35-
MetaError::Fatal(ae) => {
36-
ErrorCode::MetaServiceError(ae.to_string()).set_backtrace(ae.backtrace())
37-
}
3835
MetaError::ClientError(ce) => ce.into(),
3936
MetaError::APIError(e) => e.into(),
4037
}

0 commit comments

Comments
 (0)