refactor: add Configurable trait (#3917)

* refactor: add Configurable trait

* refactor: add merge_with_cli_options() to simplify load_options()

* docs: add comments

* fix: clippy errors

* fix: toml format

* fix: build error

* fix: clippy errors

* build: downgrade config-rs

* refactor: use '#[snafu(source(from()))'

* refactor: minor modification for load_layered_options() to make it clean

Cargo.toml

@@ -100,6 +100,7 @@ bytemuck = "1.12"
 bytes = { version = "1.5", features = ["serde"] }
 chrono = { version = "0.4", features = ["serde"] }
 clap = { version = "4.4", features = ["derive"] }
+config = "0.13.0"
 crossbeam-utils = "0.8"
 dashmap = "5.4"
 datafusion = { git = "https://github.com/apache/arrow-datafusion.git", rev = "34eda15b73a9e278af8844b30ed2f1c21c10359c" }

src/cmd/Cargo.toml

@@ -40,7 +40,6 @@ common-telemetry = { workspace = true, features = [
 common-time.workspace = true
 common-version.workspace = true
 common-wal.workspace = true
-config = "0.13"
 datanode.workspace = true
 datatypes.workspace = true
 either = "1.8"

src/cmd/src/datanode.rs

@@ -18,6 +18,7 @@ use std::time::Duration;
 use async_trait::async_trait;
 use catalog::kvbackend::MetaKvBackend;
 use clap::Parser;
+use common_config::Configurable;
 use common_telemetry::info;
 use common_telemetry::logging::TracingOptions;
 use common_wal::config::DatanodeWalConfig;
@@ -28,7 +29,9 @@ use meta_client::MetaClientOptions;
 use servers::Mode;
 use snafu::{OptionExt, ResultExt};
 
-use crate::error::{MissingConfigSnafu, Result, ShutdownDatanodeSnafu, StartDatanodeSnafu};
+use crate::error::{
+    LoadLayeredConfigSnafu, MissingConfigSnafu, Result, ShutdownDatanodeSnafu, StartDatanodeSnafu,
+};
 use crate::options::{GlobalOptions, Options};
 use crate::App;
 
@@ -133,12 +136,24 @@ struct StartCommand {
 
 impl StartCommand {
     fn load_options(&self, global_options: &GlobalOptions) -> Result<Options> {
-        let mut opts: DatanodeOptions = Options::load_layered_options(
-            self.config_file.as_deref(),
-            self.env_prefix.as_ref(),
-            DatanodeOptions::env_list_keys(),
-        )?;
+        Ok(Options::Datanode(Box::new(
+            self.merge_with_cli_options(
+                global_options,
+                DatanodeOptions::load_layered_options(
+                    self.config_file.as_deref(),
+                    self.env_prefix.as_ref(),
+                )
+                .context(LoadLayeredConfigSnafu)?,
+            )?,
+        )))
+    }
 
+    // The precedence order is: cli > config file > environment variables > default values.
+    fn merge_with_cli_options(
+        &self,
+        global_options: &GlobalOptions,
+        mut opts: DatanodeOptions,
+    ) -> Result<DatanodeOptions> {
         if let Some(dir) = &global_options.log_dir {
             opts.logging.dir.clone_from(dir);
         }
@@ -208,7 +223,7 @@ impl StartCommand {
         // Disable dashboard in datanode.
         opts.http.disable_dashboard = true;
 
-        Ok(Options::Datanode(Box::new(opts)))
+        Ok(opts)
     }
 
     async fn build(self, mut opts: DatanodeOptions) -> Result<Instance> {
@@ -259,13 +274,14 @@ mod tests {
     use std::io::Write;
     use std::time::Duration;
 
+    use common_config::ENV_VAR_SEP;
     use common_test_util::temp_dir::create_named_temp_file;
     use datanode::config::{FileConfig, GcsConfig, ObjectStoreConfig, S3Config};
     use servers::heartbeat_options::HeartbeatOptions;
     use servers::Mode;
 
     use super::*;
-    use crate::options::{GlobalOptions, ENV_VAR_SEP};
+    use crate::options::GlobalOptions;
 
     #[test]
     fn test_read_from_config_file() {

src/cmd/src/error.rs

@@ -17,7 +17,6 @@ use std::any::Any;
 use common_error::ext::{BoxedError, ErrorExt};
 use common_error::status_code::StatusCode;
 use common_macro::stack_trace_debug;
-use config::ConfigError;
 use rustyline::error::ReadlineError;
 use snafu::{Location, Snafu};
 
@@ -209,8 +208,8 @@ pub enum Error {
 
     #[snafu(display("Failed to load layered config"))]
     LoadLayeredConfig {
-        #[snafu(source)]
-        error: ConfigError,
+        #[snafu(source(from(common_config::error::Error, Box::new)))]
+        source: Box<common_config::error::Error>,
         #[snafu(implicit)]
         location: Location,
     },

src/cmd/src/frontend.rs

@@ -23,6 +23,7 @@ use cache::{
 use catalog::kvbackend::{CachedMetaKvBackendBuilder, KvBackendCatalogManager, MetaKvBackend};
 use clap::Parser;
 use client::client_manager::DatanodeClients;
+use common_config::Configurable;
 use common_meta::cache::{CacheRegistryBuilder, LayeredCacheRegistryBuilder};
 use common_meta::heartbeat::handler::parse_mailbox_message::ParseMailboxMessageHandler;
 use common_meta::heartbeat::handler::HandlerGroupExecutor;
@@ -40,7 +41,9 @@ use servers::tls::{TlsMode, TlsOption};
 use servers::Mode;
 use snafu::{OptionExt, ResultExt};
 
-use crate::error::{self, InitTimezoneSnafu, MissingConfigSnafu, Result, StartFrontendSnafu};
+use crate::error::{
+    self, InitTimezoneSnafu, LoadLayeredConfigSnafu, MissingConfigSnafu, Result, StartFrontendSnafu,
+};
 use crate::options::{GlobalOptions, Options};
 use crate::App;
 
@@ -153,12 +156,24 @@ pub struct StartCommand {
 
 impl StartCommand {
     fn load_options(&self, global_options: &GlobalOptions) -> Result<Options> {
-        let mut opts: FrontendOptions = Options::load_layered_options(
-            self.config_file.as_deref(),
-            self.env_prefix.as_ref(),
-            FrontendOptions::env_list_keys(),
-        )?;
+        Ok(Options::Frontend(Box::new(
+            self.merge_with_cli_options(
+                global_options,
+                FrontendOptions::load_layered_options(
+                    self.config_file.as_deref(),
+                    self.env_prefix.as_ref(),
+                )
+                .context(LoadLayeredConfigSnafu)?,
+            )?,
+        )))
+    }
 
+    // The precedence order is: cli > config file > environment variables > default values.
+    fn merge_with_cli_options(
+        &self,
+        global_options: &GlobalOptions,
+        mut opts: FrontendOptions,
+    ) -> Result<FrontendOptions> {
         if let Some(dir) = &global_options.log_dir {
             opts.logging.dir.clone_from(dir);
         }
@@ -220,7 +235,7 @@ impl StartCommand {
 
         opts.user_provider.clone_from(&self.user_provider);
 
-        Ok(Options::Frontend(Box::new(opts)))
+        Ok(opts)
     }
 
     async fn build(self, mut opts: FrontendOptions) -> Result<Instance> {
@@ -336,12 +351,13 @@ mod tests {
 
     use auth::{Identity, Password, UserProviderRef};
     use common_base::readable_size::ReadableSize;
+    use common_config::ENV_VAR_SEP;
     use common_test_util::temp_dir::create_named_temp_file;
     use frontend::service_config::GrpcOptions;
     use servers::http::HttpOptions;
 
     use super::*;
-    use crate::options::{GlobalOptions, ENV_VAR_SEP};
+    use crate::options::GlobalOptions;
 
     #[test]
     fn test_try_from_start_command() {

src/cmd/src/metasrv.rs

@@ -16,13 +16,14 @@ use std::time::Duration;
 
 use async_trait::async_trait;
 use clap::Parser;
+use common_config::Configurable;
 use common_telemetry::info;
 use common_telemetry::logging::TracingOptions;
 use meta_srv::bootstrap::MetasrvInstance;
 use meta_srv::metasrv::MetasrvOptions;
 use snafu::ResultExt;
 
-use crate::error::{self, Result, StartMetaServerSnafu};
+use crate::error::{self, LoadLayeredConfigSnafu, Result, StartMetaServerSnafu};
 use crate::options::{GlobalOptions, Options};
 use crate::App;
 
@@ -128,12 +129,24 @@ struct StartCommand {
 
 impl StartCommand {
     fn load_options(&self, global_options: &GlobalOptions) -> Result<Options> {
-        let mut opts: MetasrvOptions = Options::load_layered_options(
-            self.config_file.as_deref(),
-            self.env_prefix.as_ref(),
-            MetasrvOptions::env_list_keys(),
-        )?;
+        Ok(Options::Metasrv(Box::new(
+            self.merge_with_cli_options(
+                global_options,
+                MetasrvOptions::load_layered_options(
+                    self.config_file.as_deref(),
+                    self.env_prefix.as_ref(),
+                )
+                .context(LoadLayeredConfigSnafu)?,
+            )?,
+        )))
+    }
 
+    // The precedence order is: cli > config file > environment variables > default values.
+    fn merge_with_cli_options(
+        &self,
+        global_options: &GlobalOptions,
+        mut opts: MetasrvOptions,
+    ) -> Result<MetasrvOptions> {
         if let Some(dir) = &global_options.log_dir {
             opts.logging.dir.clone_from(dir);
         }
@@ -196,7 +209,7 @@ impl StartCommand {
         // Disable dashboard in metasrv.
         opts.http.disable_dashboard = true;
 
-        Ok(Options::Metasrv(Box::new(opts)))
+        Ok(opts)
     }
 
     async fn build(self, mut opts: MetasrvOptions) -> Result<Instance> {
@@ -225,11 +238,11 @@ mod tests {
     use std::io::Write;
 
     use common_base::readable_size::ReadableSize;
+    use common_config::ENV_VAR_SEP;
     use common_test_util::temp_dir::create_named_temp_file;
     use meta_srv::selector::SelectorType;
 
     use super::*;
-    use crate::options::ENV_VAR_SEP;
 
     #[test]
     fn test_read_from_cmd() {