Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cli): fallback to env if CLI arg is absent #7658

Merged
merged 3 commits into from
Feb 2, 2023
Merged

feat(cli): fallback to env if CLI arg is absent #7658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
524abb2
cli fallback to env
Gun9niR Feb 2, 2023
5bf3412
use RW prefix
Gun9niR Feb 2, 2023
de2fe39
Merge branch 'main' into zhidong/env
mergify[bot] Feb 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 17 additions & 13 deletions src/compute/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,37 +38,41 @@ use risingwave_common_proc_macro::OverrideConfig;
#[derive(Parser, Clone, Debug)]
pub struct ComputeNodeOpts {
// TODO: rename to listen_address and separate out the port.
#[clap(long, default_value = "127.0.0.1:5688")]
#[clap(long, env = "RW_HOST", default_value = "127.0.0.1:5688")]
pub host: String,

/// The address of the compute node's meta client.
///
/// Optional, we will use listen_address if not specified.
#[clap(long)]
#[clap(long, env = "RW_CLIENT_ADDRESS")]
pub client_address: Option<String>,

#[clap(long, default_value = "127.0.0.1:1222")]
#[clap(
long,
env = "RW_PROMETHEUS_LISTENER_ADDR",
default_value = "127.0.0.1:1222"
)]
pub prometheus_listener_addr: String,

#[clap(long, default_value = "http://127.0.0.1:5690")]
#[clap(long, env = "RW_META_ADDRESS", default_value = "http://127.0.0.1:5690")]
pub meta_address: String,

/// Endpoint of the connector node
#[clap(long, env = "CONNECTOR_RPC_ENDPOINT")]
#[clap(long, env = "RW_CONNECTOR_RPC_ENDPOINT")]
pub connector_rpc_endpoint: Option<String>,

/// The path of `risingwave.toml` configuration file.
///
/// If empty, default configuration values will be used.
#[clap(long, default_value = "")]
#[clap(long, env = "RW_CONFIG_PATH", default_value = "")]
pub config_path: String,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All components will share the same ENV name for config_path, right? Not sure whether we need to distinguish them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can specify env for each process, so I think there's no need?

Copy link
Contributor

@StrikeW StrikeW Feb 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest adding a WARN log to prompt users we use the value from env variables. Since if there is an unexpected CONFIG_PATH in the user’s environment, its value will fill in. Same as other configs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a RW_ prefix, just like POSTGRES_ in pg.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure whether we need to distinguish them.

As long as we don't set it in some all-in-one process mode like the playground, it's fine to use the same name.

Let's add a RW_ prefix, just like POSTGRES_ in pg.

Good point!

Copy link
Contributor

@arkbriar arkbriar Feb 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we use the full name RISINGWAVE_ instead of RW_ to avoid confusion? @Gun9niR
Which one do you favor? cc. @fuyufjh @StrikeW


/// Total available memory in bytes, used by LRU Manager
#[clap(long, default_value_t = default_total_memory_bytes())]
#[clap(long, env = "RW_TOTAL_MEMORY_BYTES", default_value_t = default_total_memory_bytes())]
pub total_memory_bytes: usize,

/// The parallelism that the compute node will register to the scheduler of the meta service.
#[clap(long, default_value_t = default_parallelism())]
#[clap(long, env = "RW_PARALLELISM", default_value_t = default_parallelism())]
pub parallelism: usize,

#[clap(flatten)]
Expand All @@ -84,30 +88,30 @@ struct OverrideConfigOpts {
/// `memory` or `memory-shared`.
/// 2. `in-memory`
/// 3. `sled://{path}`
#[clap(long)]
#[clap(long, env = "RW_STATE_STORE")]
#[override_opts(path = storage.state_store)]
pub state_store: Option<String>,

/// Used for control the metrics level, similar to log level.
/// 0 = close metrics
/// >0 = open metrics
#[clap(long)]
#[clap(long, env = "RW_METRICS_LEVEL")]
#[override_opts(path = server.metrics_level)]
pub metrics_level: Option<u32>,

/// Path to file cache data directory.
/// Left empty to disable file cache.
#[clap(long)]
#[clap(long, env = "RW_FILE_CACHE_DIR")]
#[override_opts(path = storage.file_cache.dir)]
pub file_cache_dir: Option<String>,

/// Enable reporting tracing information to jaeger.
#[clap(parse(from_flag = true_if_present), long)]
#[clap(long, env = "RW_ENABLE_JAEGER_TRACING", parse(from_flag = true_if_present))]
#[override_opts(path = streaming.enable_jaeger_tracing)]
pub enable_jaeger_tracing: Flag,

/// Enable async stack tracing for risectl.
#[clap(long, arg_enum)]
#[clap(long, env = "RW_ASYNC_STACK_TRACE", arg_enum)]
#[override_opts(path = streaming.async_stack_trace)]
pub async_stack_trace: Option<AsyncStackTraceOption>,
}
Expand Down
24 changes: 16 additions & 8 deletions src/frontend/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,24 +70,32 @@ use session::SessionManagerImpl;
#[derive(Parser, Clone, Debug)]
pub struct FrontendOpts {
// TODO: rename to listen_address and separate out the port.
#[clap(long, default_value = "127.0.0.1:4566")]
#[clap(long, env = "RW_HOST", default_value = "127.0.0.1:4566")]
pub host: String,

// Optional, we will use listen_address if not specified.
#[clap(long)]
#[clap(long, env = "RW_CLIENT_ADDRESS")]
pub client_address: Option<String>,

// TODO: This is currently unused.
#[clap(long)]
#[clap(long, env = "RW_PORT")]
pub port: Option<u16>,

#[clap(long, default_value = "http://127.0.0.1:5690")]
#[clap(long, env = "RW_META_ADDR", default_value = "http://127.0.0.1:5690")]
pub meta_addr: String,

#[clap(long, default_value = "127.0.0.1:2222")]
#[clap(
long,
env = "RW_PROMETHEUS_LISTENER_ADDR",
default_value = "127.0.0.1:2222"
)]
pub prometheus_listener_addr: String,

#[clap(long, default_value = "127.0.0.1:6786")]
#[clap(
long,
env = "RW_HEALTH_CHECK_LISTENER_ADDR",
default_value = "127.0.0.1:6786"
)]
pub health_check_listener_addr: String,

/// The path of `risingwave.toml` configuration file.
Expand All @@ -96,7 +104,7 @@ pub struct FrontendOpts {
///
/// Note that internal system parameters should be defined in the configuration file at
/// [`risingwave_common::config`] instead of command line arguments.
#[clap(long, default_value = "")]
#[clap(long, env = "RW_CONFIG_PATH", default_value = "")]
pub config_path: String,

#[clap(flatten)]
Expand All @@ -109,7 +117,7 @@ struct OverrideConfigOpts {
/// Used for control the metrics level, similar to log level.
/// 0 = close metrics
/// >0 = open metrics
#[clap(long)]
#[clap(long, env = "RW_METRICS_LEVEL")]
#[override_opts(path = server.metrics_level)]
pub metrics_level: Option<u32>,
}
Expand Down
30 changes: 14 additions & 16 deletions src/meta/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,57 +58,55 @@ use crate::rpc::server::{rpc_serve, AddressInfo, MetaStoreBackend};
#[derive(Debug, Clone, Parser)]
pub struct MetaNodeOpts {
// TODO: rename to listen_address and separate out the port.
#[clap(long, default_value = "127.0.0.1:5690")]
#[clap(long, env = "RW_LISTEN_ADDR", default_value = "127.0.0.1:5690")]
listen_addr: String,

/// Deprecated. But we keep it for backward compatibility.
#[clap(long)]
#[clap(long, env = "RW_HOST")]
host: Option<String>,

/// The endpoint for this meta node, which also serves as its unique identifier in cluster
/// membership and leader election.
#[clap(long)]
#[clap(long, env = "RW_META_ENDPOINT")]
meta_endpoint: Option<String>,

#[clap(long)]
#[clap(long, env = "RW_DASHBOARD_HOST")]
dashboard_host: Option<String>,

#[clap(long)]
#[clap(long, env = "RW_PROMETHEUS_HOST")]
prometheus_host: Option<String>,

#[clap(long, default_value_t = String::from(""))]
#[clap(long, env = "RW_ETCD_ENDPOINTS", default_value_t = String::from(""))]
etcd_endpoints: String,

/// Enable authentication with etcd. By default disabled.
#[clap(long)]
#[clap(long, env = "RW_ETCD_AUTH")]
etcd_auth: bool,

/// Username of etcd, required when --etcd-auth is enabled.
/// Default value is read from the 'ETCD_USERNAME' environment variable.
#[clap(long, env = "ETCD_USERNAME", default_value = "")]
#[clap(long, env = "RW_ETCD_USERNAME", default_value = "")]
etcd_username: String,

/// Password of etcd, required when --etcd-auth is enabled.
/// Default value is read from the 'ETCD_PASSWORD' environment variable.
#[clap(long, env = "ETCD_PASSWORD", default_value = "")]
#[clap(long, env = "RW_ETCD_PASSWORD", default_value = "")]
etcd_password: String,

#[clap(long)]
#[clap(long, env = "RW_DASHBOARD_UI_PATH")]
dashboard_ui_path: Option<String>,

/// For dashboard service to fetch cluster info.
#[clap(long)]
#[clap(long, env = "RW_PROMETHEUS_ENDPOINT")]
prometheus_endpoint: Option<String>,

/// Endpoint of the connector node, there will be a sidecar connector node
/// colocated with Meta node in the cloud environment
#[clap(long, env = "META_CONNECTOR_RPC_ENDPOINT")]
#[clap(long, env = "RW_CONNECTOR_RPC_ENDPOINT")]
pub connector_rpc_endpoint: Option<String>,

/// The path of `risingwave.toml` configuration file.
///
/// If empty, default configuration values will be used.
#[clap(long, default_value = "")]
#[clap(long, env = "RW_CONFIG_PATH", default_value = "RW_CONFIG_PATH")]
pub config_path: String,

#[clap(flatten)]
Expand All @@ -118,7 +116,7 @@ pub struct MetaNodeOpts {
/// Command-line arguments for compute-node that overrides the config file.
#[derive(Parser, Clone, Debug, OverrideConfig)]
pub struct OverrideConfigOpts {
#[clap(long, arg_enum)]
#[clap(long, env = "RW_BACKEND", arg_enum)]
#[override_opts(path = meta.backend)]
backend: Option<MetaBackend>,
}
Expand Down
24 changes: 14 additions & 10 deletions src/storage/compactor/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,30 +25,34 @@ use crate::server::compactor_serve;
#[derive(Parser, Clone, Debug)]
pub struct CompactorOpts {
// TODO: rename to listen_address and separate out the port.
#[clap(long, default_value = "127.0.0.1:6660")]
#[clap(long, env = "RW_HOST", default_value = "127.0.0.1:6660")]
pub host: String,

// Optional, we will use listen_address if not specified.
#[clap(long)]
#[clap(long, env = "RW_CLIENT_ADDRESS")]
pub client_address: Option<String>,

// TODO: This is currently unused.
#[clap(long)]
#[clap(long, env = "RW_PORT")]
pub port: Option<u16>,

#[clap(long, default_value = "127.0.0.1:1260")]
#[clap(
long,
env = "RW_PROMETHEUS_LISTENER_ADDR",
default_value = "127.0.0.1:1260"
)]
pub prometheus_listener_addr: String,

#[clap(long, default_value = "http://127.0.0.1:5690")]
#[clap(long, env = "RW_META_ADDRESS", default_value = "http://127.0.0.1:5690")]
pub meta_address: String,

#[clap(long)]
#[clap(long, env = "RW_COMPACTION_WORKER_THREADS_NUMBER")]
pub compaction_worker_threads_number: Option<usize>,

/// The path of `risingwave.toml` configuration file.
///
/// If empty, default configuration values will be used.
#[clap(long, default_value = "")]
#[clap(long, env = "RW_CONFIG_PATH", default_value = "")]
pub config_path: String,

#[clap(flatten)]
Expand All @@ -61,19 +65,19 @@ struct OverrideConfigOpts {
/// Of the form `hummock+{object_store}` where `object_store`
/// is one of `s3://{path}`, `s3-compatible://{path}`, `minio://{path}`, `disk://{path}`,
/// `memory` or `memory-shared`.
#[clap(long)]
#[clap(long, env = "RW_STATE_STORE")]
#[override_opts(path = storage.state_store)]
pub state_store: Option<String>,

/// Used for control the metrics level, similar to log level.
/// 0 = close metrics
/// >0 = open metrics
#[clap(long)]
#[clap(long, env = "RW_METRICS_LEVEL")]
#[override_opts(path = server.metrics_level)]
pub metrics_level: Option<u32>,

/// It's a hint used by meta node.
#[clap(long)]
#[clap(long, env = "RW_MAX_CONCURRENT_TASK_NUMBER")]
#[override_opts(path = storage.max_concurrent_compaction_task_number)]
pub max_concurrent_task_number: Option<u64>,
}
Expand Down