From 638686a9ae3b2dcb71e6220aa7d55e4c9e2d2ea4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=BC=A0=E7=82=8E=E6=B3=BC?= Date: Tue, 2 Jan 2024 16:34:43 +0800 Subject: [PATCH] refactor: Remove `Compatible` error (#14198) `Compatible` error is added in 0.9.41 to accept two types of error returned from meta-service: `KVAppError` and `MetaAPIError`. Such compatible layer can be removed since meta-client's min compatible meta-service version has already increased to a newer version which does not send `KVAppError` any more. --- src/meta/api/src/compat_errors.rs | 98 ------------------------------- src/meta/api/src/lib.rs | 1 - src/meta/api/src/reply.rs | 37 ++---------- src/meta/client/src/lib.rs | 4 +- 4 files changed, 7 insertions(+), 133 deletions(-) delete mode 100644 src/meta/api/src/compat_errors.rs diff --git a/src/meta/api/src/compat_errors.rs b/src/meta/api/src/compat_errors.rs deleted file mode 100644 index b689fc5ae6a7..000000000000 --- a/src/meta/api/src/compat_errors.rs +++ /dev/null @@ -1,98 +0,0 @@ -// Copyright 2021 Datafuse Labs -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -/// Compatible layer to receive different types of errors from meta-service. -/// -/// It allows the server side to switch to return a smaller error type, e.g., from KVAppError to MetaAPIError. -/// -/// Currently: -/// - Meta service kv_api returns KVAppError, while the client only consume the MetaApiError variants -#[derive(thiserror::Error, serde::Serialize, serde::Deserialize, Debug, Clone, PartialEq, Eq)] -#[serde(untagged)] -pub enum Compatible -where - Outer: From, - Outer: TryInto, -{ - Outer(Outer), - Inner(Inner), -} - -impl Compatible -where - Outer: From, - Outer: TryInto, -{ - pub fn into_inner(self) -> Inner - where Inner: From<>::Error> { - match self { - Compatible::Outer(o) => { - let i: Inner = o.try_into().unwrap_or_else(|e| Inner::from(e)); - i - } - Compatible::Inner(i) => i, - } - } -} - -#[cfg(test)] -mod tests { - use databend_common_meta_types::ForwardToLeader; - use databend_common_meta_types::MembershipNode; - use databend_common_meta_types::MetaAPIError; - use databend_common_meta_types::MetaError; - - use crate::compat_errors::Compatible; - use crate::kv_app_error::KVAppError; - - #[test] - fn test_read_api_err_from_api_err() -> anyhow::Result<()> { - let me = MetaAPIError::ForwardToLeader(ForwardToLeader { - leader_id: Some(1), - leader_node: Some(MembershipNode {}), - }); - let s = serde_json::to_string(&me)?; - - let ge: Compatible = serde_json::from_str(&s)?; - - if let MetaAPIError::ForwardToLeader(f) = ge.clone().into_inner() { - assert_eq!(Some(1), f.leader_id); - } else { - unreachable!("expect ForwardToLeader but: {:?}", ge); - } - - Ok(()) - } - - #[test] - fn test_read_api_err_from_kv_app_err() -> anyhow::Result<()> { - let me = KVAppError::MetaError(MetaError::APIError(MetaAPIError::ForwardToLeader( - ForwardToLeader { - leader_id: Some(1), - leader_node: Some(MembershipNode {}), - }, - ))); - let s = serde_json::to_string(&me)?; - - let ge: Compatible = serde_json::from_str(&s)?; - - if let MetaAPIError::ForwardToLeader(f) = ge.clone().into_inner() { - assert_eq!(Some(1), f.leader_id); - } else { - unreachable!("expect ForwardToLeader but: {:?}", ge); - } - - Ok(()) - } -} diff --git a/src/meta/api/src/lib.rs b/src/meta/api/src/lib.rs index 78c0cf5952ed..c489cc88dffe 100644 --- a/src/meta/api/src/lib.rs +++ b/src/meta/api/src/lib.rs @@ -22,7 +22,6 @@ mod background_api; mod background_api_impl; mod background_api_keys; mod background_api_test_suite; -pub mod compat_errors; mod data_mask_api; mod data_mask_api_impl; mod data_mask_api_keys; diff --git a/src/meta/api/src/reply.rs b/src/meta/api/src/reply.rs index 62967b56958c..fe61e6f99dd9 100644 --- a/src/meta/api/src/reply.rs +++ b/src/meta/api/src/reply.rs @@ -15,17 +15,10 @@ use databend_common_meta_types::protobuf::RaftReply; use databend_common_meta_types::InvalidReply; use databend_common_meta_types::MetaAPIError; -use databend_common_meta_types::MetaError; -use databend_common_meta_types::MetaNetworkError; use databend_common_meta_types::TxnOpResponse; use databend_common_meta_types::TxnReply; use serde::de::DeserializeOwned; -use crate::compat_errors::Compatible; -use crate::kv_app_error::KVAppError; - -/// Compatible layer: -/// Convert either KVAppError or MetaAPIError to MetaAPIError pub fn reply_to_api_result(msg: RaftReply) -> Result where T: DeserializeOwned { if !msg.data.is_empty() { @@ -33,33 +26,13 @@ where T: DeserializeOwned { .map_err(|e| InvalidReply::new("can not decode RaftReply.data", &e))?; Ok(res) } else { - let err: Compatible = serde_json::from_str(&msg.error) + let err: MetaAPIError = serde_json::from_str(&msg.error) .map_err(|e| InvalidReply::new("can not decode RaftReply.error", &e))?; - let err = err.into_inner(); Err(err) } } -/// Compatible layer: -/// Convert either KVAppError or MetaError to MetaError -pub fn reply_to_meta_result(raft_reply: RaftReply) -> Result -where T: DeserializeOwned { - if !raft_reply.data.is_empty() { - let res: T = serde_json::from_str(&raft_reply.data) - .map_err(|e| InvalidReply::new("can not decode RaftReply.data", &e))?; - Ok(res) - } else { - let err: Compatible = serde_json::from_str(&raft_reply.error) - .map_err(|e| InvalidReply::new("can not decode RaftReply.error", &e))?; - - let err = err.into_inner(); - - Err(err) - } -} - -/// Compatible layer: /// Convert txn response to `success` and a series of `TxnOpResponse`. pub fn txn_reply_to_api_result( txn_reply: TxnReply, @@ -67,11 +40,9 @@ pub fn txn_reply_to_api_result( if txn_reply.error.is_empty() { Ok((txn_reply.success, txn_reply.responses)) } else { - let err: Compatible = serde_json::from_str(&txn_reply.error) - .map_err(|e| { - MetaNetworkError::InvalidReply(InvalidReply::new("invalid TxnReply.error", &e)) - })?; - let err = err.into_inner(); + let err: MetaAPIError = serde_json::from_str(&txn_reply.error) + .map_err(|e| InvalidReply::new("invalid TxnReply.error", &e))?; + Err(err) } } diff --git a/src/meta/client/src/lib.rs b/src/meta/client/src/lib.rs index 80b8ce8babe3..0f1fd0d1f739 100644 --- a/src/meta/client/src/lib.rs +++ b/src/meta/client/src/lib.rs @@ -25,7 +25,6 @@ mod message; use std::sync::LazyLock; pub use databend_common_meta_api::reply::reply_to_api_result; -pub use databend_common_meta_api::reply::reply_to_meta_result; pub use grpc_action::MetaGrpcReadReq; pub use grpc_action::MetaGrpcReq; pub use grpc_action::RequestFor; @@ -83,6 +82,9 @@ pub static METACLI_COMMIT_SEMVER: LazyLock = LazyLock::new(|| { /// - 2023-12-16: since 1.2.258: /// Meta service: add: ttl to TxnPutRequest and Upsert /// +/// - 2024-01-02: since TODO: fill version when merged. +/// Meta client: remove `Compatible` for KVAppError and MetaAPIError, added in `2023-02-16: since 0.9.41` +/// /// Server feature set: /// ```yaml /// server_features: