From 50b1fdc41f4d87bf20daaf0ff3b07e337f176020 Mon Sep 17 00:00:00 2001 From: Charles Eckman Date: Fri, 15 Mar 2024 12:43:06 -0400 Subject: [PATCH 1/2] Add skip-mode-check option to config and env In the config, `cache.assume_rw_mode`; in the environment, `SCCACHE_ASSUME_RW_MODE`. The "winning" value, if any is specified, will cause the server to skip the read/write `check` of the remote backend and assume that it operates in the given mode. --- docs/Configuration.md | 8 ++++++++ src/config.rs | 42 +++++++++++++++++++++++++++++++++++++++--- tests/harness/mod.rs | 1 + 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/docs/Configuration.md b/docs/Configuration.md index cfca72444..b915f30fb 100644 --- a/docs/Configuration.md +++ b/docs/Configuration.md @@ -19,6 +19,11 @@ cache_dir = "/home/user/.cache/sccache-dist-client" type = "token" token = "secrettoken" +[cache] +# If specified, skip the startup check for whether the backend is read-only/writeable; +# assume it has this mode instead. +# If unspecified, the server will check the mode at startup. +assume_rw_mode = "READ_WRITE" #[cache.azure] # does not work as it appears @@ -136,6 +141,9 @@ configuration variables ### cache configs +* `SCCACHE_ASSUME_RW_MODE` to skip checking whether the cache is correctly configured. + Instead, assume it is `READ_ONLY` or `READ_WRITE`- whichever this environment variable specifies. + #### disk (local) * `SCCACHE_DIR` local on disk artifact cache directory diff --git a/src/config.rs b/src/config.rs index 4173bcaac..f34b0e4f6 100644 --- a/src/config.rs +++ b/src/config.rs @@ -344,6 +344,10 @@ pub enum CacheType { #[derive(Debug, Default, Serialize, Deserialize, PartialEq, Eq)] #[serde(deny_unknown_fields)] pub struct CacheConfigs { + /// If set, assume that the configure cache has the given mode + /// (without checking at server startup). + pub assume_rw_mode: Option, + pub azure: Option, pub disk: Option, pub gcs: Option, @@ -358,8 +362,9 @@ pub struct CacheConfigs { impl CacheConfigs { /// Return cache type in an arbitrary but /// consistent ordering - fn into_fallback(self) -> (Option, DiskCacheConfig) { + fn into_fallback(self) -> (Option, Option, DiskCacheConfig) { let CacheConfigs { + assume_rw_mode, azure, disk, gcs, @@ -383,12 +388,13 @@ impl CacheConfigs { let fallback = disk.unwrap_or_default(); - (cache_type, fallback) + (assume_rw_mode, cache_type, fallback) } /// Override self with any existing fields from other fn merge(&mut self, other: Self) { let CacheConfigs { + assume_rw_mode, azure, disk, gcs, @@ -400,6 +406,10 @@ impl CacheConfigs { oss, } = other; + if assume_rw_mode.is_some() { + self.assume_rw_mode = assume_rw_mode; + } + if azure.is_some() { self.azure = azure } @@ -904,7 +914,21 @@ fn config_from_env() -> Result { None }; + let assume_rw_mode = match env::var("SCCACHE_ASSUME_RW_MODE") + .as_ref() + .map(String::as_str) + { + Ok("READ_ONLY") => Some(CacheModeConfig::ReadOnly), + Ok("READ_WRITE") => Some(CacheModeConfig::ReadWrite), + Ok(_) => { + warn!("Invalid SCCACHE_ASSUME_RW_MODE; ignoring value. The server may check the mode explicitly."); + None + } + _ => None, + }; + let cache = CacheConfigs { + assume_rw_mode, azure, disk, gcs, @@ -948,6 +972,10 @@ pub struct Config { pub fallback_cache: DiskCacheConfig, pub dist: DistConfig, pub server_startup_timeout: Option, + + /// If set, skip the check on `cache`'s RW/RO mode on server startup. + /// Assume that it has this mode instead. + pub assume_rw_mode: Option, } impl Config { @@ -978,12 +1006,13 @@ impl Config { let EnvConfig { cache } = env_conf; conf_caches.merge(cache); - let (caches, fallback_cache) = conf_caches.into_fallback(); + let (assume_rw_mode, caches, fallback_cache) = conf_caches.into_fallback(); Self { cache: caches, fallback_cache, dist, server_startup_timeout, + assume_rw_mode, } } } @@ -1234,6 +1263,7 @@ fn test_parse_size() { fn config_overrides() { let env_conf = EnvConfig { cache: CacheConfigs { + assume_rw_mode: Some(CacheModeConfig::ReadOnly), azure: Some(AzureCacheConfig { connection_string: String::new(), container: String::new(), @@ -1260,6 +1290,7 @@ fn config_overrides() { let file_conf = FileConfig { cache: CacheConfigs { + assume_rw_mode: Some(CacheModeConfig::ReadWrite), disk: Some(DiskCacheConfig { dir: "/file-cache".into(), size: 15, @@ -1287,6 +1318,7 @@ fn config_overrides() { assert_eq!( Config::from_env_and_file_configs(env_conf, file_conf), Config { + assume_rw_mode: Some(CacheModeConfig::ReadOnly), cache: Some(CacheType::Redis(RedisCacheConfig { endpoint: Some("myotherredisurl".to_owned()), ttl: 24 * 3600, @@ -1421,6 +1453,9 @@ fn full_toml_parse() { const CONFIG_STR: &str = r#" server_startup_timeout_ms = 10000 +[cache] +assume_rw_mode = "READ_ONLY" + [dist] # where to find the scheduler scheduler_url = "http://1.2.3.4:10600" @@ -1502,6 +1537,7 @@ no_credentials = true file_config, FileConfig { cache: CacheConfigs { + assume_rw_mode: Some(CacheModeConfig::ReadOnly), azure: None, // TODO not sure how to represent a unit struct in TOML Some(AzureCacheConfig), disk: Some(DiskCacheConfig { dir: PathBuf::from("/tmp/.cache/sccache"), diff --git a/tests/harness/mod.rs b/tests/harness/mod.rs index 16a039847..91032d7a3 100644 --- a/tests/harness/mod.rs +++ b/tests/harness/mod.rs @@ -146,6 +146,7 @@ pub fn sccache_client_cfg( }; sccache::config::FileConfig { cache: sccache::config::CacheConfigs { + assume_rw_mode: None, azure: None, disk: Some(disk_cache), gcs: None, From 06edc6ebd30c5eff97a6b9959ff573bd7700e775 Mon Sep 17 00:00:00 2001 From: Charles Eckman Date: Fri, 15 Mar 2024 12:55:21 -0400 Subject: [PATCH 2/2] Respect assume_rw_mode --- src/server.rs | 41 +++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/src/server.rs b/src/server.rs index 70dc4f859..f7b3ff162 100644 --- a/src/server.rs +++ b/src/server.rs @@ -443,23 +443,32 @@ pub fn start_server(config: &Config, port: u16) -> Result<()> { } }; - let cache_mode = runtime.block_on(async { - match raw_storage.check().await { - Ok(mode) => Ok(mode), - Err(err) => { - error!("storage check failed for: {err:?}"); - - notify_server_startup( - ¬ify, - ServerStartup::Err { - reason: err.to_string(), - }, - )?; - - Err(err) + let cache_mode = if let Some(rw_mode) = config.assume_rw_mode { + // We've been provided with a RW mode to assume this cache has. + // Use it. + Ok(rw_mode.into()) + } else { + // Before starting to use this cache (raw_storage), we need to check whether it's ReadOnly or ReadWrite- + // or if it's altogether unreachable. + runtime.block_on(async { + match raw_storage.check().await { + Ok(mode) => Ok(mode), + Err(err) => { + error!("storage check failed for: {err:?}"); + + notify_server_startup( + ¬ify, + ServerStartup::Err { + reason: err.to_string(), + }, + )?; + + Err(err) + } } - } - })?; + }) + }?; + info!("server has setup with {cache_mode:?}"); let storage = match cache_mode {