Merge pull request databendlabs#2734 from drmingdrmer/simplify-kv-action

[metasrv] refactor: simplify metasrv ActionHandler

common/meta/flight/Cargo.toml

@@ -24,6 +24,7 @@ common-tracing = {path = "../../tracing"}
 
 # Crates.io dependencies
 async-trait = "0.1"
+derive_more = "0.99.16"
 futures = "0.3"
 jwt-simple = "0.10.7"
 log = "0.4"

common/meta/flight/src/flight_action.rs

@@ -43,23 +43,8 @@ pub trait RequestFor {
     type Reply;
 }
 
-#[macro_export]
-macro_rules! action_declare {
-    ($req:ident, $reply:ty, $enum_ctor:expr) => {
-        impl RequestFor for $req {
-            type Reply = $reply;
-        }
-
-        impl From<$req> for MetaFlightAction {
-            fn from(act: $req) -> Self {
-                $enum_ctor(act)
-            }
-        }
-    };
-}
-
 // Action wrapper for do_action.
-#[derive(serde::Serialize, serde::Deserialize, Clone, Debug)]
+#[derive(serde::Serialize, serde::Deserialize, Clone, Debug, derive_more::From)]
 pub enum MetaFlightAction {
     // database meta
     CreateDatabase(CreateDatabaseAction),
@@ -129,15 +114,6 @@ impl RequestFor for GetKVAction {
     type Reply = GetKVActionReply;
 }
 
-// Explicitly defined the converter for MetaDoAction
-// It's implementations' choice, that they gonna using enum MetaDoAction as wrapper.
-// This can be simplified by using macro (see code below)
-impl From<GetKVAction> for MetaFlightAction {
-    fn from(act: GetKVAction) -> Self {
-        MetaFlightAction::GetKV(act)
-    }
-}
-
 // - MGetKV
 
 // Again, impl chooses to wrap it up
@@ -147,70 +123,66 @@ pub struct MGetKVAction {
 }
 
 // here we use a macro to simplify the declarations
-action_declare!(MGetKVAction, MGetKVActionReply, MetaFlightAction::MGetKV);
+impl RequestFor for MGetKVAction {
+    type Reply = MGetKVActionReply;
+}
 
 // - prefix list
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)]
 pub struct PrefixListReq(pub String);
-action_declare!(
-    PrefixListReq,
-    PrefixListReply,
-    MetaFlightAction::PrefixListKV
-);
-
-action_declare!(
-    UpsertKVAction,
-    UpsertKVActionReply,
-    MetaFlightAction::UpsertKV
-);
+impl RequestFor for PrefixListReq {
+    type Reply = PrefixListReply;
+}
+
+impl RequestFor for UpsertKVAction {
+    type Reply = UpsertKVActionReply;
+}
 
 // == database actions ==
 // - create database
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)]
 pub struct CreateDatabaseAction {
     pub plan: CreateDatabasePlan,
 }
-action_declare!(
-    CreateDatabaseAction,
-    CreateDatabaseReply,
-    MetaFlightAction::CreateDatabase
-);
+impl RequestFor for CreateDatabaseAction {
+    type Reply = CreateDatabaseReply;
+}
 
 // - get database
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)]
 pub struct GetDatabaseAction {
     pub db: String,
 }
-action_declare!(
-    GetDatabaseAction,
-    DatabaseInfo,
-    MetaFlightAction::GetDatabase
-);
+impl RequestFor for GetDatabaseAction {
+    type Reply = DatabaseInfo;
+}
 
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)]
 pub struct DropDatabaseAction {
     pub plan: DropDatabasePlan,
 }
-action_declare!(DropDatabaseAction, (), MetaFlightAction::DropDatabase);
+impl RequestFor for DropDatabaseAction {
+    type Reply = ();
+}
 
 // == table actions ==
 // - create table
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)]
 pub struct CreateTableAction {
     pub plan: CreateTablePlan,
 }
-action_declare!(
-    CreateTableAction,
-    CreateTableReply,
-    MetaFlightAction::CreateTable
-);
+impl RequestFor for CreateTableAction {
+    type Reply = CreateTableReply;
+}
 
 // - drop table
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug)]
 pub struct DropTableAction {
     pub plan: DropTablePlan,
 }
-action_declare!(DropTableAction, (), MetaFlightAction::DropTable);
+impl RequestFor for DropTableAction {
+    type Reply = ();
+}
 
 // - get table
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug, Eq, PartialEq)]
@@ -219,13 +191,17 @@ pub struct GetTableAction {
     pub table: String,
 }
 
-action_declare!(GetTableAction, TableInfo, MetaFlightAction::GetTable);
+impl RequestFor for GetTableAction {
+    type Reply = TableInfo;
+}
 
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug, Eq, PartialEq)]
 pub struct GetTableExtReq {
     pub tbl_id: MetaId,
 }
-action_declare!(GetTableExtReq, TableInfo, MetaFlightAction::GetTableExt);
+impl RequestFor for GetTableExtReq {
+    type Reply = TableInfo;
+}
 
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug, Eq, PartialEq)]
 pub struct UpsertTableOptionReq {
@@ -234,31 +210,25 @@ pub struct UpsertTableOptionReq {
     pub option_key: String,
     pub option_value: String,
 }
-action_declare!(
-    UpsertTableOptionReq,
-    UpsertTableOptionReply,
-    MetaFlightAction::CommitTable
-);
+impl RequestFor for UpsertTableOptionReq {
+    type Reply = UpsertTableOptionReply;
+}
 
 // - get tables
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug, Eq, PartialEq)]
 pub struct GetTablesAction {
     pub db: String,
 }
 
-action_declare!(
-    GetTablesAction,
-    Vec<Arc<TableInfo>>,
-    MetaFlightAction::GetTables
-);
+impl RequestFor for GetTablesAction {
+    type Reply = Vec<Arc<TableInfo>>;
+}
 
 // -get databases
 
 #[derive(serde::Serialize, serde::Deserialize, Clone, Debug, PartialEq)]
 pub struct GetDatabasesAction;
 
-action_declare!(
-    GetDatabasesAction,
-    Vec<Arc<DatabaseInfo>>,
-    MetaFlightAction::GetDatabases
-);
+impl RequestFor for GetDatabasesAction {
+    type Reply = Vec<Arc<DatabaseInfo>>;
+}

metasrv/src/executor/action_handler.rs

@@ -15,6 +15,7 @@
 use std::sync::Arc;
 
 use common_exception::ErrorCode;
+use common_meta_api::KVApi;
 use common_meta_flight::MetaFlightAction;
 use common_meta_flight::RequestFor;
 use serde::Serialize;
@@ -55,10 +56,12 @@ impl ActionHandler {
         // To keep the code IDE-friendly, we manually expand the enum variants and dispatch them one by one
 
         match action {
-            MetaFlightAction::UpsertKV(a) => s.serialize(self.handle(a).await?),
-            MetaFlightAction::GetKV(a) => s.serialize(self.handle(a).await?),
-            MetaFlightAction::MGetKV(a) => s.serialize(self.handle(a).await?),
-            MetaFlightAction::PrefixListKV(a) => s.serialize(self.handle(a).await?),
+            MetaFlightAction::UpsertKV(a) => s.serialize(self.meta_node.upsert_kv(a).await?),
+            MetaFlightAction::GetKV(a) => s.serialize(self.meta_node.get_kv(&a.key).await?),
+            MetaFlightAction::MGetKV(a) => s.serialize(self.meta_node.mget_kv(&a.keys).await?),
+            MetaFlightAction::PrefixListKV(a) => {
+                s.serialize(self.meta_node.prefix_list_kv(&a.0).await?)
+            }
 
             // database
             MetaFlightAction::CreateDatabase(a) => s.serialize(self.handle(a).await?),

metasrv/src/executor/mod.rs

@@ -13,7 +13,6 @@
 // limitations under the License.
 
 mod action_handler;
-mod kv_handlers;
 mod meta_handlers;
 
 pub use action_handler::ActionHandler;