Merge remote-tracking branch 'upstream/raftstore-proxy-6.2' into migr…

…ate-pre-handle

components/proxy_server/src/config.rs

@@ -10,7 +10,7 @@ use itertools::Itertools;
 use online_config::OnlineConfig;
 use serde_derive::{Deserialize, Serialize};
 use serde_with::with_prefix;
-use tikv::config::TiKvConfig;
+use tikv::config::{TiKvConfig, LAST_CONFIG_FILE};
 use tikv_util::{config::ReadableDuration, crit};
 
 use crate::fatal;
@@ -138,3 +138,62 @@ pub fn address_proxy_config(config: &mut TiKvConfig) {
         .labels
         .insert(DEFAULT_ENGINE_LABEL_KEY.to_owned(), engine_name);
 }
+
+pub fn validate_and_persist_config(config: &mut TiKvConfig, persist: bool) {
+    config.compatible_adjust();
+    if let Err(e) = config.validate() {
+        fatal!("invalid configuration: {}", e);
+    }
+
+    if let Err(e) = check_critical_config(config) {
+        fatal!("critical config check failed: {}", e);
+    }
+
+    if persist {
+        if let Err(e) = tikv::config::persist_config(config) {
+            fatal!("persist critical config failed: {}", e);
+        }
+    }
+}
+
+/// Prevents launching with an incompatible configuration
+///
+/// Loads the previously-loaded configuration from `last_tikv.toml`,
+/// compares key configuration items and fails if they are not
+/// identical.
+pub fn check_critical_config(config: &TiKvConfig) -> Result<(), String> {
+    // Check current critical configurations with last time, if there are some
+    // changes, user must guarantee relevant works have been done.
+    if let Some(mut cfg) = get_last_config(&config.storage.data_dir) {
+        cfg.compatible_adjust();
+        if let Err(e) = cfg.validate() {
+            warn!("last_tikv.toml is invalid but ignored: {:?}", e);
+        }
+        config.check_critical_cfg_with(&cfg)?;
+    }
+    Ok(())
+}
+
+pub fn get_last_config(data_dir: &str) -> Option<TiKvConfig> {
+    let store_path = Path::new(data_dir);
+    let last_cfg_path = store_path.join(LAST_CONFIG_FILE);
+    let mut v: Vec<String> = vec![];
+    if last_cfg_path.exists() {
+        let s = TiKvConfig::from_file(&last_cfg_path, None).unwrap_or_else(|e| {
+            error!(
+                "invalid auto generated configuration file {}, err {}",
+                last_cfg_path.display(),
+                e
+            );
+            std::process::exit(1)
+        });
+        if !v.is_empty() {
+            info!("unrecognized in last config";
+                "config" => ?v,
+                "file" => last_cfg_path.display(),
+            );
+        }
+        return Some(s);
+    }
+    None
+}

components/proxy_server/src/proxy.rs

@@ -14,9 +14,7 @@ use tikv_util::config::ReadableDuration;
 
 use crate::{
     fatal,
-    setup::{
-        ensure_no_unrecognized_config, overwrite_config_with_cmd_args, validate_and_persist_config,
-    },
+    setup::{ensure_no_unrecognized_config, overwrite_config_with_cmd_args},
 };
 
 // Not the same as TiKV
@@ -33,8 +31,8 @@ pub fn setup_default_tikv_config(default: &mut TiKvConfig) {
     default.server.addr = TIFLASH_DEFAULT_LISTENING_ADDR.to_string();
     default.server.status_addr = TIFLASH_DEFAULT_STATUS_ADDR.to_string();
     default.server.advertise_status_addr = TIFLASH_DEFAULT_STATUS_ADDR.to_string();
-    default.raft_store.region_worker_tick_interval = 500;
-    let clean_stale_tick_max = (10_000 / default.raft_store.region_worker_tick_interval) as usize;
+    default.raft_store.region_worker_tick_interval = ReadableDuration::millis(500);
+    let clean_stale_tick_max = (10_000 / default.raft_store.region_worker_tick_interval.as_millis()) as usize;
     default.raft_store.clean_stale_tick_max = clean_stale_tick_max;
 }
 
@@ -302,7 +300,7 @@ pub unsafe fn run_proxy(
     config.logger_compatible_adjust();
 
     if is_config_check {
-        validate_and_persist_config(&mut config, false);
+        crate::config::validate_and_persist_config(&mut config, false);
         match crate::config::ensure_no_common_unrecognized_keys(
             &proxy_unrecognized_keys,
             &unrecognized_keys,

components/proxy_server/src/run.rs

@@ -596,7 +596,7 @@ impl<ER: RaftEngine> TiKvServer<ER> {
     fn init_config(mut config: TiKvConfig) -> ConfigController {
         // Add {label: {"engine": "tiflash"}} to Config
         crate::config::address_proxy_config(&mut config);
-        validate_and_persist_config(&mut config, true);
+        crate::config::validate_and_persist_config(&mut config, true);
 
         ensure_dir_exist(&config.storage.data_dir).unwrap();
         if !config.rocksdb.wal_dir.is_empty() {

components/proxy_server/src/setup.rs

@@ -10,13 +10,11 @@ use std::{
 use chrono::Local;
 use clap::ArgMatches;
 use collections::HashMap;
-pub use server::setup::{
-    ensure_no_unrecognized_config, initial_logger, initial_metric, validate_and_persist_config,
-};
-use tikv::config::{check_critical_config, persist_config, MetricConfig, TiKvConfig};
+pub use server::setup::{ensure_no_unrecognized_config, initial_logger, initial_metric};
+use tikv::config::{MetricConfig, TiKvConfig};
 use tikv_util::{self, config, logger};
 
-use crate::config::ProxyConfig;
+use crate::config::{validate_and_persist_config, ProxyConfig};
 pub use crate::fatal;
 
 #[allow(dead_code)]

components/raftstore/src/engine_store_ffi/observer.rs

@@ -702,7 +702,7 @@ impl ApplySnapshotObserver for TiFlashObserver {
         peer_id: u64,
         snap_key: &crate::store::SnapKey,
         snap: Option<&crate::store::Snapshot>,
-    ) -> std::result::Result<(), coprocessor::error::Error> {
+    ) -> std::result::Result<(), coprocessor::Error> {
         fail::fail_point!("on_ob_post_apply_snapshot", |_| {
             return Err(box_err!("on_ob_post_apply_snapshot"));
         });

components/raftstore/src/store/worker/region.rs

@@ -1419,7 +1419,7 @@ mod tests {
             peer_id: u64,
             key: &crate::store::SnapKey,
             snapshot: Option<&crate::store::Snapshot>,
-        ) -> std::result::Result<(), crate::coprocessor::error::Error> {
+        ) -> std::result::Result<(), crate::coprocessor::Error> {
             let code = snapshot.unwrap().total_size().unwrap()
                 + key.term
                 + key.region_id

src/config.rs

@@ -91,7 +91,6 @@ const LOCKCF_MIN_MEM: usize = 256 * MIB as usize;
 const LOCKCF_MAX_MEM: usize = GIB as usize;
 const RAFT_MIN_MEM: usize = 256 * MIB as usize;
 const RAFT_MAX_MEM: usize = 2 * GIB as usize;
-/// Exported for extra checking.
 pub const LAST_CONFIG_FILE: &str = "last_tikv.toml";
 const TMP_CONFIG_FILE: &str = "tmp_tikv.toml";
 const MAX_BLOCK_SIZE: usize = 32 * MIB as usize;
@@ -3282,10 +3281,13 @@ pub fn check_critical_config(config: &TiKvConfig) -> Result<(), String> {
     // changes, user must guarantee relevant works have been done.
     if let Some(mut cfg) = get_last_config(&config.storage.data_dir) {
         cfg.compatible_adjust();
+        info!("check_critical_config finished compatible_adjust");
         if let Err(e) = cfg.validate() {
             warn!("last_tikv.toml is invalid but ignored: {:?}", e);
         }
+        info!("check_critical_config finished validate");
         config.check_critical_cfg_with(&cfg)?;
+        info!("check_critical_config finished check_critical_cfg_with");
     }
     Ok(())
 }

tests/proxy/normal.rs

@@ -5,6 +5,7 @@ use std::{
     io::{self, Read, Write},
     ops::{Deref, DerefMut},
     path::{Path, PathBuf},
+    str::FromStr,
     sync::{
         atomic::{AtomicUsize, Ordering},
         mpsc, Arc, Once, RwLock,
@@ -34,7 +35,10 @@ use new_mock_engine_store::{
 };
 use pd_client::PdClient;
 use proxy_server::{
-    config::{address_proxy_config, ensure_no_common_unrecognized_keys},
+    config::{
+        address_proxy_config, ensure_no_common_unrecognized_keys, get_last_config,
+        validate_and_persist_config,
+    },
     proxy::{
         gen_tikv_config, setup_default_tikv_config, TIFLASH_DEFAULT_LISTENING_ADDR,
         TIFLASH_DEFAULT_STATUS_ADDR,
@@ -48,11 +52,10 @@ use raftstore::{
     engine_store_ffi::{KVGetStatus, RaftStoreProxyFFI},
     store::util::find_peer,
 };
-use server::setup::validate_and_persist_config;
 use sst_importer::SstImporter;
 pub use test_raftstore::{must_get_equal, must_get_none, new_peer};
 use test_raftstore::{new_node_cluster, new_tikv_config};
-use tikv::config::TiKvConfig;
+use tikv::config::{TiKvConfig, LAST_CONFIG_FILE};
 use tikv_util::{
     config::{LogFormat, ReadableDuration, ReadableSize},
     time::Duration,
@@ -246,12 +249,11 @@ mod config {
 
         let mut unrecognized_keys = Vec::new();
         let mut config = TiKvConfig::from_file(path, Some(&mut unrecognized_keys)).unwrap();
-        // Otherwise we have no default addr for TiKv.
+        // Othersize we have no default addr for TiKv.
         setup_default_tikv_config(&mut config);
         assert_eq!(config.memory_usage_high_water, 0.65);
         assert_eq!(config.rocksdb.max_open_files, 111);
         assert_eq!(config.server.addr, TIFLASH_DEFAULT_LISTENING_ADDR);
-        assert_eq!(config.raft_store.snap_handle_pool_size, 4);
         assert_eq!(unrecognized_keys.len(), 3);
 
         let mut proxy_unrecognized_keys = Vec::new();
@@ -279,13 +281,31 @@ mod config {
         assert_eq!(unknown.unwrap_err(), "nosense, rocksdb.z");
 
         // Need run this test with ENGINE_LABEL_VALUE=tiflash, otherwise will fatal exit.
-        server::setup::validate_and_persist_config(&mut config, true);
+        std::fs::remove_file(
+            PathBuf::from_str(&config.storage.data_dir)
+                .unwrap()
+                .join(LAST_CONFIG_FILE),
+        );
+        validate_and_persist_config(&mut config, true);
 
         // Will not override ProxyConfig
         let proxy_config_new = ProxyConfig::from_file(path, None).unwrap();
         assert_eq!(proxy_config_new.raft_store.snap_handle_pool_size, 4);
     }
 
+    #[test]
+    fn test_validate_config() {
+        let mut file = tempfile::NamedTempFile::new().unwrap();
+        let text = "memory-usage-high-water=0.65\n[raftstore.aaa]\nbbb=2\n[server]\nengine-addr=\"1.2.3.4:5\"\n[raftstore]\nsnap-handle-pool-size=4\n[nosense]\nfoo=2\n[rocksdb]\nmax-open-files = 111\nz=1";
+        write!(file, "{}", text).unwrap();
+        let path = file.path();
+        let tmp_store_folder = tempfile::TempDir::new().unwrap();
+        let tmp_last_config_path = tmp_store_folder.path().join(LAST_CONFIG_FILE);
+        std::fs::copy(path, tmp_last_config_path.as_path()).unwrap();
+        std::fs::copy(path, "./last_ttikv.toml").unwrap();
+        get_last_config(tmp_store_folder.path().to_str().unwrap());
+    }
+
     #[test]
     fn test_config_default_addr() {
         let mut file = tempfile::NamedTempFile::new().unwrap();

tests/proxy/proxy.rs

@@ -30,15 +30,14 @@ use new_mock_engine_store::{
     Cluster, ProxyConfig, Simulator, TestPdClient,
 };
 use pd_client::PdClient;
-use proxy_server::config::ensure_no_common_unrecognized_keys;
+use proxy_server::config::{ensure_no_common_unrecognized_keys, validate_and_persist_config};
 use raft::eraftpb::MessageType;
 use raftstore::{
     coprocessor::{ConsistencyCheckMethod, Coprocessor},
     engine_store_ffi,
     engine_store_ffi::{KVGetStatus, RaftStoreProxyFFI},
     store::util::find_peer,
 };
-use server::setup::validate_and_persist_config;
 use sst_importer::SstImporter;
 pub use test_raftstore::{must_get_equal, must_get_none, new_peer};
 use tikv::config::TiKvConfig;