diff --git a/src/query/config/src/config.rs b/src/query/config/src/config.rs index 3b3766acb323d..555bce8f74942 100644 --- a/src/query/config/src/config.rs +++ b/src/query/config/src/config.rs @@ -15,7 +15,6 @@ use std::collections::BTreeMap; use std::collections::HashMap; use std::env; -use std::ffi::OsString; use std::fmt; use std::fmt::Debug; use std::fmt::Formatter; @@ -2958,7 +2957,7 @@ pub struct DiskCacheConfig { pub struct SpillConfig { /// Path of spill to local disk. disable if it's empty. #[clap(long, value_name = "VALUE", default_value = "")] - pub spill_local_disk_path: OsString, + pub spill_local_disk_path: String, #[clap(long, value_name = "VALUE", default_value = "30")] /// Percentage of reserve disk space that won't be used for spill to local disk. @@ -3039,7 +3038,11 @@ mod cache_config_converters { { spill.spill_local_disk_path = PathBuf::from(&cache.disk_cache_config.path) .join("temp/_query_spill") - .into(); + .into_os_string() + .into_string() + .map_err(|s| { + ErrorCode::Internal(format!("failed to convert os string to string: {s:?}")) + })? }; Ok(InnerConfig { @@ -3120,20 +3123,20 @@ mod cache_config_converters { fn try_from(value: SpillConfig) -> std::result::Result { let SpillConfig { spill_local_disk_path, - spill_local_disk_reserved_space_percentage: spill_local_disk_max_space_percentage, + spill_local_disk_reserved_space_percentage: reserved, spill_local_disk_max_bytes, } = value; - if !spill_local_disk_max_space_percentage.is_normal() - || spill_local_disk_max_space_percentage.is_sign_negative() - || spill_local_disk_max_space_percentage > OrderedFloat(100.0) + if !reserved.is_normal() + || reserved.is_sign_negative() + || reserved > OrderedFloat(100.0) { - return Err(ErrorCode::InvalidArgument( - "invalid spill_local_disk_max_space_percentage", - )); + Err(ErrorCode::InvalidArgument(format!( + "invalid spill_local_disk_reserved_space_percentage: {reserved}" + )))?; } Ok(Self { path: spill_local_disk_path, - reserved_disk_ratio: spill_local_disk_max_space_percentage / 100.0, + reserved_disk_ratio: reserved / 100.0, global_bytes_limit: spill_local_disk_max_bytes, }) } diff --git a/src/query/config/src/inner.rs b/src/query/config/src/inner.rs index 829f7abd338d9..cfb0d55800fd5 100644 --- a/src/query/config/src/inner.rs +++ b/src/query/config/src/inner.rs @@ -13,7 +13,6 @@ // limitations under the License. use std::collections::HashMap; -use std::ffi::OsString; use std::fmt; use std::fmt::Debug; use std::fmt::Display; @@ -719,7 +718,7 @@ impl Default for CacheConfig { #[derive(Clone, Debug, PartialEq, Eq)] pub struct SpillConfig { /// Path of spill to local disk. disable if it's empty. - pub path: OsString, + pub path: String, /// Ratio of the reserve of the disk space. pub reserved_disk_ratio: OrderedFloat, @@ -731,7 +730,7 @@ pub struct SpillConfig { impl Default for SpillConfig { fn default() -> Self { Self { - path: OsString::from(""), + path: "".to_string(), reserved_disk_ratio: OrderedFloat(0.3), global_bytes_limit: u64::MAX, } diff --git a/src/query/service/tests/it/configs.rs b/src/query/service/tests/it/configs.rs index 9366d2f22f4b8..f9b2ee37de0a3 100644 --- a/src/query/service/tests/it/configs.rs +++ b/src/query/service/tests/it/configs.rs @@ -926,6 +926,38 @@ protocol = "binary" Ok(()) } +#[test] +fn test_spill_config() -> Result<()> { + let file_path = temp_dir().join("databend_test_spill_config.toml"); + + let mut f = fs::File::create(&file_path)?; + f.write_all( + r#" +[spill] +spill_local_disk_path = "/data/spill" +spill_local_disk_reserved_space_percentage = 0.5 +"# + .as_bytes(), + )?; + + // Make sure all data flushed. + f.flush()?; + + temp_env::with_vars( + vec![("CONFIG_FILE", Some(file_path.to_string_lossy().as_ref()))], + || { + let cfg = InnerConfig::load_for_test().expect("config load failed"); + + assert_eq!(cfg.spill.path, "/data/spill"); + }, + ); + + // remove temp file + fs::remove_file(file_path)?; + + Ok(()) +} + /// Test new hive catalog #[test] fn test_override_config_new_hive_catalog() -> Result<()> { diff --git a/src/query/service/tests/it/storages/testdata/configs_table_basic.txt b/src/query/service/tests/it/storages/testdata/configs_table_basic.txt index 35ea94d3b6d3e..20762066b467a 100644 --- a/src/query/service/tests/it/storages/testdata/configs_table_basic.txt +++ b/src/query/service/tests/it/storages/testdata/configs_table_basic.txt @@ -142,6 +142,9 @@ DB.Table: 'system'.'configs', Table: configs-table_id:1, ver:0, Engine: SystemCo | 'query' | 'udf_server_allow_list' | '' | '' | | 'query' | 'udfs' | '{"name":"test_builtin_ping","definition":"CREATE OR REPLACE FUNCTION test_builtin_ping (STRING)\n RETURNS STRING\n LANGUAGE python\nHANDLER = 'ping'\nADDRESS = 'https://databend.com';"}' | '' | | 'query' | 'users' | '{"name":"root","auth_type":"no_password","auth_string":null}' | '' | +| 'spill' | 'spill_local_disk_max_bytes' | '18446744073709551615' | '' | +| 'spill' | 'spill_local_disk_path' | '' | '' | +| 'spill' | 'spill_local_disk_reserved_space_percentage' | '30.0' | '' | | 'storage' | 'allow_insecure' | 'true' | '' | | 'storage' | 'azblob.account_key' | '' | '' | | 'storage' | 'azblob.account_name' | '' | '' | diff --git a/src/query/storages/common/cache/src/temp_dir.rs b/src/query/storages/common/cache/src/temp_dir.rs index 54f0465e0c94d..ab80d83e4f52f 100644 --- a/src/query/storages/common/cache/src/temp_dir.rs +++ b/src/query/storages/common/cache/src/temp_dir.rs @@ -62,7 +62,7 @@ impl TempDirManager { if let Err(e) = remove_dir_all(&path) { if !matches!(e.kind(), ErrorKind::NotFound) { return Err(ErrorCode::StorageUnavailable(format!( - "can't clean temp dir: {e}", + "can't clean temp dir {path:?}: {e}", ))); } } @@ -373,7 +373,6 @@ impl Drop for InnerPath { #[cfg(test)] mod tests { use std::assert_matches::assert_matches; - use std::ffi::OsString; use std::fs; use std::sync::atomic::Ordering; @@ -385,7 +384,7 @@ mod tests { GlobalInstance::init_testing(thread.name().unwrap()); let config = SpillConfig { - path: OsString::from("test_data"), + path: "test_data".to_string(), reserved_disk_ratio: 0.01.into(), global_bytes_limit: 1 << 30, }; @@ -425,7 +424,7 @@ mod tests { GlobalInstance::init_testing(thread.name().unwrap()); let config = SpillConfig { - path: OsString::from("test_data2"), + path: "test_data2".to_string(), reserved_disk_ratio: 0.99.into(), global_bytes_limit: 1 << 30, }; diff --git a/src/query/storages/system/src/configs_table.rs b/src/query/storages/system/src/configs_table.rs index fc26e51486812..8d38d10153eb2 100644 --- a/src/query/storages/system/src/configs_table.rs +++ b/src/query/storages/system/src/configs_table.rs @@ -104,6 +104,17 @@ impl SyncSystemTable for ConfigsTable { cache_config_value, ); + let spill_config = config.spill; + let spill_config_value = serde_json::to_value(spill_config)?; + ConfigsTable::extract_config( + &mut names, + &mut values, + &mut groups, + &mut descs, + "spill".to_string(), + spill_config_value, + ); + let storage_config = config.storage; let storage_config_value = serde_json::to_value(storage_config)?; ConfigsTable::extract_config(