From 06d273b75a76fb12661cde23bfca9acfdce2ea9b Mon Sep 17 00:00:00 2001 From: Niwaka <61189782+NiwakaDev@users.noreply.github.com> Date: Wed, 8 Nov 2023 12:22:19 +0900 Subject: [PATCH] fix: `GET: /config` return all configurations when running standalone (#2630) * fix: config api return all configurations when running standalone * chore: follow same style * fix: avoid panic --- Cargo.lock | 1 + src/cmd/Cargo.toml | 2 +- src/cmd/src/frontend.rs | 2 +- src/cmd/src/options.rs | 16 ++++- src/cmd/src/standalone.rs | 12 ++-- src/frontend/src/error.rs | 9 ++- src/frontend/src/frontend.rs | 12 ++++ src/frontend/src/instance.rs | 7 ++- src/frontend/src/server.rs | 15 +++-- tests-integration/Cargo.toml | 1 + tests-integration/src/standalone.rs | 20 +++++- tests-integration/src/test_util.rs | 3 +- tests-integration/tests/http.rs | 97 +++++++++++++++++++++++++---- 13 files changed, 163 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2b55b6db8d05..acd943183fa2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9263,6 +9263,7 @@ dependencies = [ "catalog", "chrono", "client", + "cmd", "common-base", "common-catalog", "common-config", diff --git a/src/cmd/Cargo.toml b/src/cmd/Cargo.toml index c112f8a209d5..cd2df6f523ae 100644 --- a/src/cmd/Cargo.toml +++ b/src/cmd/Cargo.toml @@ -61,6 +61,7 @@ snafu.workspace = true substrait = { workspace = true } table = { workspace = true } tokio.workspace = true +toml.workspace = true [target.'cfg(not(windows))'.dependencies] tikv-jemallocator = "0.5" @@ -69,7 +70,6 @@ tikv-jemallocator = "0.5" common-test-util = { workspace = true } serde.workspace = true temp-env = "0.3" -toml.workspace = true [target.'cfg(not(windows))'.dev-dependencies] rexpect = "0.5" diff --git a/src/cmd/src/frontend.rs b/src/cmd/src/frontend.rs index 429a16d85782..e869517b719e 100644 --- a/src/cmd/src/frontend.rs +++ b/src/cmd/src/frontend.rs @@ -201,7 +201,7 @@ impl StartCommand { .context(StartFrontendSnafu)?; instance - .build_servers(&opts) + .build_servers(opts) .await .context(StartFrontendSnafu)?; diff --git a/src/cmd/src/options.rs b/src/cmd/src/options.rs index b48b57c237f9..3908eda60fc8 100644 --- a/src/cmd/src/options.rs +++ b/src/cmd/src/options.rs @@ -16,7 +16,8 @@ use common_config::KvBackendConfig; use common_telemetry::logging::LoggingOptions; use config::{Config, Environment, File, FileFormat}; use datanode::config::{DatanodeOptions, ProcedureConfig}; -use frontend::frontend::FrontendOptions; +use frontend::error::{Result as FeResult, TomlFormatSnafu}; +use frontend::frontend::{FrontendOptions, TomlSerializable}; use meta_srv::metasrv::MetaSrvOptions; use serde::{Deserialize, Serialize}; use snafu::ResultExt; @@ -27,6 +28,7 @@ pub const ENV_VAR_SEP: &str = "__"; pub const ENV_LIST_SEP: &str = ","; /// Options mixed up from datanode, frontend and metasrv. +#[derive(Serialize)] pub struct MixOptions { pub data_home: String, pub procedure: ProcedureConfig, @@ -36,6 +38,18 @@ pub struct MixOptions { pub logging: LoggingOptions, } +impl From for FrontendOptions { + fn from(value: MixOptions) -> Self { + value.frontend + } +} + +impl TomlSerializable for MixOptions { + fn to_toml(&self) -> FeResult { + toml::to_string(self).context(TomlFormatSnafu) + } +} + pub enum Options { Datanode(Box), Frontend(Box), diff --git a/src/cmd/src/standalone.rs b/src/cmd/src/standalone.rs index 7d23702ee328..ac4ced397e59 100644 --- a/src/cmd/src/standalone.rs +++ b/src/cmd/src/standalone.rs @@ -316,13 +316,13 @@ impl StartCommand { #[allow(unused_variables)] #[allow(clippy::diverging_sub_expression)] async fn build(self, opts: MixOptions) -> Result { - let mut fe_opts = opts.frontend; #[allow(clippy::unnecessary_mut_passed)] - let fe_plugins = plugins::setup_frontend_plugins(&mut fe_opts) + let fe_opts = opts.frontend.clone(); + let fe_plugins = plugins::setup_frontend_plugins(&fe_opts) .await .context(StartFrontendSnafu)?; - let dn_opts = opts.datanode; + let dn_opts = opts.datanode.clone(); info!("Standalone start command: {:#?}", self); info!( @@ -338,8 +338,8 @@ impl StartCommand { let metadata_dir = metadata_store_dir(&opts.data_home); let (kv_backend, procedure_manager) = FeInstance::try_build_standalone_components( metadata_dir, - opts.metadata_store, - opts.procedure, + opts.metadata_store.clone(), + opts.procedure.clone(), ) .await .context(StartFrontendSnafu)?; @@ -377,7 +377,7 @@ impl StartCommand { .await?; frontend - .build_servers(&fe_opts) + .build_servers(opts) .await .context(StartFrontendSnafu)?; diff --git a/src/frontend/src/error.rs b/src/frontend/src/error.rs index 63c009db51c4..5f851768b2b8 100644 --- a/src/frontend/src/error.rs +++ b/src/frontend/src/error.rs @@ -272,6 +272,12 @@ pub enum Error { #[snafu(display("Invalid auth config"))] IllegalAuthConfig { source: auth::error::Error }, + + #[snafu(display("Failed to serialize options to TOML"))] + TomlFormat { + #[snafu(source)] + error: toml::ser::Error, + }, } pub type Result = std::result::Result; @@ -279,7 +285,8 @@ pub type Result = std::result::Result; impl ErrorExt for Error { fn status_code(&self) -> StatusCode { match self { - Error::ParseAddr { .. } + Error::TomlFormat { .. } + | Error::ParseAddr { .. } | Error::InvalidSql { .. } | Error::InvalidInsertRequest { .. } | Error::InvalidDeleteRequest { .. } diff --git a/src/frontend/src/frontend.rs b/src/frontend/src/frontend.rs index cade13de921b..d43e3816fbf5 100644 --- a/src/frontend/src/frontend.rs +++ b/src/frontend/src/frontend.rs @@ -18,7 +18,9 @@ use serde::{Deserialize, Serialize}; use servers::heartbeat_options::HeartbeatOptions; use servers::http::HttpOptions; use servers::Mode; +use snafu::prelude::*; +use crate::error::{Result, TomlFormatSnafu}; use crate::service_config::{ DatanodeOptions, GrpcOptions, InfluxdbOptions, MysqlOptions, OpentsdbOptions, OtlpOptions, PostgresOptions, PromStoreOptions, @@ -76,6 +78,16 @@ impl FrontendOptions { } } +pub trait TomlSerializable { + fn to_toml(&self) -> Result; +} + +impl TomlSerializable for FrontendOptions { + fn to_toml(&self) -> Result { + toml::to_string(&self).context(TomlFormatSnafu) + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/src/frontend/src/instance.rs b/src/frontend/src/instance.rs index 2e5ef085f806..2fbdbfc02233 100644 --- a/src/frontend/src/instance.rs +++ b/src/frontend/src/instance.rs @@ -88,7 +88,7 @@ use crate::error::{ ParseSqlSnafu, PermissionSnafu, PlanStatementSnafu, Result, SqlExecInterceptedSnafu, TableOperationSnafu, }; -use crate::frontend::FrontendOptions; +use crate::frontend::{FrontendOptions, TomlSerializable}; use crate::heartbeat::handler::invalidate_table_cache::InvalidateTableCacheHandler; use crate::heartbeat::HeartbeatTask; use crate::metrics; @@ -358,7 +358,10 @@ impl Instance { }) } - pub async fn build_servers(&mut self, opts: &FrontendOptions) -> Result<()> { + pub async fn build_servers( + &mut self, + opts: impl Into + TomlSerializable, + ) -> Result<()> { let servers = Services::build(opts, Arc::new(self.clone()), self.plugins.clone()).await?; self.servers = Arc::new(servers); diff --git a/src/frontend/src/server.rs b/src/frontend/src/server.rs index 5105c1d14e60..c29f2bf45387 100644 --- a/src/frontend/src/server.rs +++ b/src/frontend/src/server.rs @@ -33,7 +33,7 @@ use servers::server::Server; use snafu::ResultExt; use crate::error::{self, Result, StartServerSnafu}; -use crate::frontend::FrontendOptions; +use crate::frontend::{FrontendOptions, TomlSerializable}; use crate::instance::FrontendInstance; pub(crate) struct Services; @@ -43,14 +43,17 @@ pub type ServerHandlers = HashMap; pub type ServerHandler = (Box, SocketAddr); impl Services { - pub(crate) async fn build( - opts: &FrontendOptions, - instance: Arc, + pub(crate) async fn build( + opts: T, + instance: Arc, plugins: Plugins, ) -> Result where - T: FrontendInstance, + T: Into + TomlSerializable, + U: FrontendInstance, { + let toml = opts.to_toml()?; + let opts: FrontendOptions = opts.into(); let mut result = Vec::::with_capacity(plugins.len()); let user_provider = plugins.get::(); @@ -120,7 +123,7 @@ impl Services { .with_metrics_handler(MetricsHandler) .with_script_handler(instance.clone()) .with_plugins(plugins) - .with_greptime_config_options(opts.to_toml_string()) + .with_greptime_config_options(toml) .build(); result.push((Box::new(http_server), http_addr)); } diff --git a/tests-integration/Cargo.toml b/tests-integration/Cargo.toml index 78a6984dc559..0c84550de2d1 100644 --- a/tests-integration/Cargo.toml +++ b/tests-integration/Cargo.toml @@ -16,6 +16,7 @@ axum-test-helper = { git = "https://github.com/sunng87/axum-test-helper.git", br catalog = { workspace = true } chrono.workspace = true client = { workspace = true, features = ["testing"] } +cmd.workspace = true common-base = { workspace = true } common-catalog = { workspace = true } common-config = { workspace = true } diff --git a/tests-integration/src/standalone.rs b/tests-integration/src/standalone.rs index c8c66cbd3cc3..153ad46e0608 100644 --- a/tests-integration/src/standalone.rs +++ b/tests-integration/src/standalone.rs @@ -15,12 +15,15 @@ use std::sync::Arc; use catalog::kvbackend::KvBackendCatalogManager; +use cmd::options::MixOptions; use common_base::Plugins; use common_config::KvBackendConfig; use common_meta::cache_invalidator::DummyKvCacheInvalidator; use common_procedure::options::ProcedureConfig; +use common_telemetry::logging::LoggingOptions; use datanode::config::DatanodeOptions; use datanode::datanode::DatanodeBuilder; +use frontend::frontend::FrontendOptions; use frontend::instance::{FrontendInstance, Instance, StandaloneDatanodeManager}; use crate::test_util::{self, create_tmp_dir_and_datanode_opts, StorageType, TestGuard}; @@ -28,6 +31,7 @@ use crate::test_util::{self, create_tmp_dir_and_datanode_opts, StorageType, Test pub struct GreptimeDbStandalone { pub instance: Arc, pub datanode_opts: DatanodeOptions, + pub mix_options: MixOptions, pub guard: TestGuard, } @@ -66,10 +70,12 @@ impl GreptimeDbStandaloneBuilder { let (opts, guard) = create_tmp_dir_and_datanode_opts(store_type, &self.instance_name); + let procedure_config = ProcedureConfig::default(); + let kv_backend_config = KvBackendConfig::default(); let (kv_backend, procedure_manager) = Instance::try_build_standalone_components( format!("{}/kv", &opts.storage.data_home), - KvBackendConfig::default(), - ProcedureConfig::default(), + kv_backend_config.clone(), + procedure_config.clone(), ) .await .unwrap(); @@ -110,7 +116,15 @@ impl GreptimeDbStandaloneBuilder { GreptimeDbStandalone { instance: Arc::new(instance), - datanode_opts: opts, + datanode_opts: opts.clone(), + mix_options: MixOptions { + data_home: opts.storage.data_home.to_string(), + procedure: procedure_config, + metadata_store: kv_backend_config, + frontend: FrontendOptions::default(), + datanode: opts, + logging: LoggingOptions::default(), + }, guard, } } diff --git a/tests-integration/src/test_util.rs b/tests-integration/src/test_util.rs index fb72430806ad..ef47b3c2b974 100644 --- a/tests-integration/src/test_util.rs +++ b/tests-integration/src/test_util.rs @@ -32,6 +32,7 @@ use datanode::config::{ AzblobConfig, DatanodeOptions, FileConfig, GcsConfig, ObjectStoreConfig, OssConfig, S3Config, StorageConfig, }; +use frontend::frontend::TomlSerializable; use frontend::instance::Instance; use frontend::service_config::{MysqlOptions, PostgresOptions}; use object_store::services::{Azblob, Gcs, Oss, S3}; @@ -376,7 +377,7 @@ pub async fn setup_test_http_app_with_frontend_and_user_provider( instance.instance.clone(), )) .with_script_handler(instance.instance.clone()) - .with_greptime_config_options(instance.datanode_opts.to_toml_string()); + .with_greptime_config_options(instance.mix_options.to_toml().unwrap()); if let Some(user_provider) = user_provider { http_server.with_user_provider(user_provider); diff --git a/tests-integration/tests/http.rs b/tests-integration/tests/http.rs index 1b355d4877a6..cb697a7661a2 100644 --- a/tests-integration/tests/http.rs +++ b/tests-integration/tests/http.rs @@ -604,7 +604,77 @@ pub async fn test_config_api(store_type: StorageType) { let res_get = client.get("/config").send().await; assert_eq!(res_get.status(), StatusCode::OK); let expected_toml_str = format!( - r#"mode = "standalone" + r#" +[procedure] +max_retry_times = 3 +retry_delay = "500ms" + +[metadata_store] +file_size = "256MiB" +purge_threshold = "4GiB" + +[frontend] +mode = "standalone" + +[frontend.heartbeat] +interval = "18s" +retry_interval = "3s" + +[frontend.http] +addr = "127.0.0.1:4000" +timeout = "30s" +body_limit = "64MiB" + +[frontend.grpc] +addr = "127.0.0.1:4001" +runtime_size = 8 +max_recv_message_size = "512MiB" +max_send_message_size = "512MiB" + +[frontend.mysql] +enable = true +addr = "127.0.0.1:4002" +runtime_size = 2 + +[frontend.mysql.tls] +mode = "disable" +cert_path = "" +key_path = "" + +[frontend.postgres] +enable = true +addr = "127.0.0.1:4003" +runtime_size = 2 + +[frontend.postgres.tls] +mode = "disable" +cert_path = "" +key_path = "" + +[frontend.opentsdb] +enable = true +addr = "127.0.0.1:4242" +runtime_size = 2 + +[frontend.influxdb] +enable = true + +[frontend.prom_store] +enable = true + +[frontend.otlp] +enable = true + +[frontend.logging] +enable_jaeger_tracing = false + +[frontend.datanode.client] +timeout = "10s" +connect_timeout = "1s" +tcp_nodelay = true + +[datanode] +mode = "standalone" node_id = 0 require_lease_before_startup = true rpc_addr = "127.0.0.1:3001" @@ -613,45 +683,45 @@ rpc_max_recv_message_size = "512MiB" rpc_max_send_message_size = "512MiB" enable_telemetry = true -[heartbeat] +[datanode.heartbeat] interval = "3s" retry_interval = "3s" -[http] +[datanode.http] addr = "127.0.0.1:4000" timeout = "30s" body_limit = "64MiB" -[wal] +[datanode.wal] file_size = "256MiB" purge_threshold = "4GiB" purge_interval = "10m" read_batch_size = 128 sync_write = false -[storage] +[datanode.storage] type = "{}" -[storage.compaction] +[datanode.storage.compaction] max_inflight_tasks = 4 max_files_in_level0 = 8 max_purge_tasks = 32 sst_write_buffer_size = "8MiB" -[storage.manifest] +[datanode.storage.manifest] checkpoint_margin = 10 gc_duration = "10m" compress = false -[storage.flush] +[datanode.storage.flush] max_flush_tasks = 8 region_write_buffer_size = "32MiB" picker_schedule_interval = "5m" auto_flush_interval = "1h" -[[region_engine]] +[[datanode.region_engine]] -[region_engine.mito] +[datanode.region_engine.mito] num_workers = 1 worker_channel_size = 128 worker_request_batch_size = 64 @@ -664,9 +734,12 @@ global_write_buffer_reject_size = "2GiB" sst_meta_cache_size = "128MiB" vector_cache_size = "512MiB" -[[region_engine]] +[[datanode.region_engine]] + +[datanode.region_engine.file] -[region_engine.file] +[datanode.logging] +enable_jaeger_tracing = false [logging] enable_jaeger_tracing = false"#,