Specify extra store labels by param --labels (#420) (release-8.1) (#422)

* Specify extra store labels by param `--labels` (#420)

ref pingcap/tiflash#9750

* Remove param `--engine-role-label`
* Specify extra store labels by param `--labels`
  * If there are duplicated key in `server.labels` and `--labels`, use the one in `server.labels`

Signed-off-by: JaySon-Huang <[email protected]>
Signed-off-by: Calvin Neo <[email protected]>

---------

Signed-off-by: JaySon-Huang <[email protected]>
Signed-off-by: Calvin Neo <[email protected]>
Co-authored-by: Calvin Neo <[email protected]>

Author: JaySon-Huang

proxy_components/proxy_server/src/proxy.rs

@@ -201,22 +201,6 @@ pub unsafe fn run_proxy(
                 .help("Sets PD endpoints")
                 .long_help("Set the PD endpoints to use. Use `,` to separate multiple PDs"),
         )
-        .arg(
-            Arg::with_name("labels")
-                .long("labels")
-                .alias("label")
-                .takes_value(true)
-                .value_name("KEY=VALUE")
-                .multiple(true)
-                .use_delimiter(true)
-                .require_delimiter(true)
-                .value_delimiter(",")
-                .help("Sets server labels")
-                .long_help(
-                    "Set the server labels. Uses `,` to separate kv pairs, like \
-                     `zone=cn,disk=ssd`",
-                ),
-        )
         .arg(
             Arg::with_name("print-sample-config")
                 .long("print-sample-config")
@@ -263,6 +247,22 @@ pub unsafe fn run_proxy(
                 .required(false)
                 .takes_value(true),
         )
+        .arg(
+            Arg::with_name("labels")
+                .long("labels")
+                .alias("label")
+                .takes_value(true)
+                .value_name("KEY=VALUE")
+                .multiple(true)
+                .use_delimiter(true)
+                .require_delimiter(true)
+                .value_delimiter(",")
+                .help("Sets server labels")
+                .long_help(
+                    "Set the server labels. Uses `,` to separate kv pairs, like \
+                     `zone=cn,disk=ssd`",
+                ),
+        )
         .arg(
             Arg::with_name("engine-label")
                 .long("engine-label")
@@ -275,12 +275,6 @@ pub unsafe fn run_proxy(
                 .long("only-decryption")
                 .help("Only do decryption in Proxy"),
         )
-        .arg(
-            Arg::with_name("engine-role-label")
-                .long("engine-role-label")
-                .help("Set engine role label")
-                .takes_value(true),
-        )
         .arg(
             Arg::with_name("memory-limit-size")
                 .long("memory-limit-size")

proxy_components/proxy_server/src/setup.rs

@@ -2,7 +2,6 @@
 use std::borrow::ToOwned;
 
 use clap::ArgMatches;
-use collections::HashMap;
 pub use server::setup::initial_logger;
 use tikv::config::{MetricConfig, TikvConfig, MEMORY_USAGE_LIMIT_RATE};
 use tikv_util::{self, config::ReadableSize, logger, sys::SysQuota};
@@ -106,7 +105,7 @@ pub fn overwrite_config_with_cmd_args(
     }
 
     if let Some(labels_vec) = matches.values_of("labels") {
-        let mut labels = HashMap::default();
+        // labels_vec is a vector of string like ["k1=v1", "k1=v2", ...]
         for label in labels_vec {
             let mut parts = label.split('=');
             let key = parts.next().unwrap().to_owned();
@@ -117,22 +116,18 @@ pub fn overwrite_config_with_cmd_args(
             if parts.next().is_some() {
                 fatal!("invalid label: {}", label);
             }
-            labels.insert(key, value);
+            if config.server.labels.contains_key(&key) {
+                warn!(
+                    "label is ignored due to duplicated key defined in `server.labels`, key={} value={}",
+                    key, value
+                );
+                continue; // ignore duplicated label
+            }
+            // only set the label when `server.labels` does not contain the label with the
+            // same key
+            config.server.labels.insert(key, value);
         }
-        config.server.labels = labels;
-    }
-
-    if let Some(capacity_str) = matches.value_of("capacity") {
-        let capacity = capacity_str.parse().unwrap_or_else(|e| {
-            fatal!("invalid capacity: {}", e);
-        });
-        config.raft_store.capacity = capacity;
-    }
-
-    if matches.value_of("metrics-addr").is_some() {
-        warn!("metrics push is not supported any more.");
     }
-
     // User can specify engine label, because we need to distinguish TiFlash role
     // (tiflash-compute or tiflash-storage) in the disaggregated architecture.
     // If no engine label is specified, we use 'ENGINE_LABEL_VALUE'(env variable
@@ -147,13 +142,18 @@ pub fn overwrite_config_with_cmd_args(
                 .unwrap(),
         ),
     );
-    const DEFAULT_ENGINE_ROLE_LABEL_KEY: &str = "engine_role";
-    if let Some(engine_role_value) = matches.value_of("engine-role-label") {
-        config.server.labels.insert(
-            DEFAULT_ENGINE_ROLE_LABEL_KEY.to_owned(),
-            String::from(engine_role_value),
-        );
+
+    if let Some(capacity_str) = matches.value_of("capacity") {
+        let capacity = capacity_str.parse().unwrap_or_else(|e| {
+            fatal!("invalid capacity: {}", e);
+        });
+        config.raft_store.capacity = capacity;
+    }
+
+    if matches.value_of("metrics-addr").is_some() {
+        warn!("metrics push is not supported any more.");
     }
+
     if let Some(unips_enabled_str) = matches.value_of("unips-enabled") {
         let enabled: u64 = unips_enabled_str.parse().unwrap_or_else(|e| {
             fatal!("invalid unips-enabled: {}", e);

proxy_scripts/ci_check.sh

@@ -6,8 +6,13 @@ echo "PATH=", $PATH
 
 if [[ $M == "fmt" ]]; then
     pwd
+    clang --version
+    rustup toolchain list
+    rustup show
     git rev-parse --show-toplevel
+    cargo --version -v
     make gen_proxy_ffi
+    git diff
     git status -s .
     GIT_STATUS=$(git status -s .) && if [[ ${GIT_STATUS} ]]; then echo "Error: found illegal git status"; echo ${GIT_STATUS}; [[ -z ${GIT_STATUS} ]]; fi
     cargo fmt -- --check