From 08dd6789d6143b54748a7acecc51668946a48ffe Mon Sep 17 00:00:00 2001 From: just-an-engineer Date: Mon, 12 Aug 2024 14:34:41 -0400 Subject: [PATCH] Add ability to specify server timeout amount in ms from config file --- src/commands.rs | 2 +- src/config.rs | 33 +++++++++++++++++++++++++++++---- src/server.rs | 7 ++++--- tests/harness/mod.rs | 1 + tests/oauth.rs | 1 + 5 files changed, 36 insertions(+), 8 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index 189880ba6..d3ccf9073 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -631,7 +631,7 @@ pub fn run_command(cmd: Command) -> Result { // Config isn't required for all commands, but if it's broken then we should flag // it early and loudly. let config = &Config::load()?; - let startup_timeout = config.server_startup_timeout; + let startup_timeout = config.server_timing.startup_timeout; match cmd { Command::ShowStats(fmt, advanced) => { diff --git a/src/config.rs b/src/config.rs index 1b26bd6e1..2d4ceb4d8 100644 --- a/src/config.rs +++ b/src/config.rs @@ -555,10 +555,18 @@ impl Default for DistConfig { pub struct FileConfig { pub cache: CacheConfigs, pub dist: DistConfig, + // pub timing: ServerTimingConfig, pub server_startup_timeout_ms: Option, + pub server_shutdown_timeout_ms: Option, pub port: Option, } +// #[derive(Debug, Default, Serialize, Deserialize, Eq, PartialEq)] +// pub struct ServerTimingConfig { +// pub server_startup_timeout_ms: Option, +// pub server_shutdown_timeout_ms: Option, +// } + // If the file doesn't exist or we can't read it, log the issue and proceed. If the // config exists but doesn't parse then something is wrong - return an error. pub fn try_read_config_file(path: &Path) -> Result> { @@ -946,10 +954,16 @@ pub struct Config { pub cache: Option, pub fallback_cache: DiskCacheConfig, pub dist: DistConfig, - pub server_startup_timeout: Option, + pub server_timing: ServerTiming, pub port: Option, } +#[derive(Debug, Default, PartialEq, Eq)] +pub struct ServerTiming { + pub startup_timeout: Option, + pub shutdown_timeout: Option, +} + impl Config { pub fn load() -> Result { let env_conf = config_from_env()?; @@ -969,12 +983,20 @@ impl Config { cache, dist, server_startup_timeout_ms, + server_shutdown_timeout_ms, port, } = file_conf; conf_caches.merge(cache); let server_startup_timeout = server_startup_timeout_ms.map(std::time::Duration::from_millis); + let server_shutdown_timeout = + server_shutdown_timeout_ms.map(std::time::Duration::from_millis); + let server_timing = ServerTiming { + startup_timeout: server_startup_timeout, + shutdown_timeout: server_shutdown_timeout, + }; + let EnvConfig { cache } = env_conf; conf_caches.merge(cache); @@ -984,7 +1006,7 @@ impl Config { cache: caches, fallback_cache, dist, - server_startup_timeout, + server_timing, port, } } @@ -1285,6 +1307,7 @@ fn config_overrides() { }, dist: Default::default(), server_startup_timeout_ms: None, + server_shutdown_timeout_ms: None, port: None, }; @@ -1307,7 +1330,7 @@ fn config_overrides() { rw_mode: CacheModeConfig::ReadWrite, }, dist: Default::default(), - server_startup_timeout: None, + server_timing: Default::default(), port: None, } ); @@ -1583,7 +1606,8 @@ no_credentials = true toolchain_cache_size: 5368709120, rewrite_includes_only: false, }, - server_startup_timeout_ms: Some(10000), + server_startup_timeout_ms: Some(10_000), + server_shutdown_timeout_ms: None, port: None, } ) @@ -1600,6 +1624,7 @@ fn test_port_config() { cache: Default::default(), dist: Default::default(), server_startup_timeout_ms: None, + server_shutdown_timeout_ms: None, port: Some(8080), } ) diff --git a/src/server.rs b/src/server.rs index 0128a4adf..21dbeadda 100644 --- a/src/server.rs +++ b/src/server.rs @@ -658,11 +658,12 @@ impl SccacheServer { } })?; - const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(10); + let config = Config::load().unwrap_or_default(); + let shutdown_timeout: Duration = config.server_timing.shutdown_timeout.unwrap_or(Duration::from_secs(10)); info!( "moving into the shutdown phase now, waiting at most {} seconds \ for all client requests to complete", - SHUTDOWN_TIMEOUT.as_secs() + shutdown_timeout.as_secs() ); // Once our server has shut down either due to inactivity or a manual @@ -672,7 +673,7 @@ impl SccacheServer { // // Note that we cap the amount of time this can take, however, as we // don't want to wait *too* long. - runtime.block_on(async { time::timeout(SHUTDOWN_TIMEOUT, wait).await })?; + runtime.block_on(async { time::timeout(shutdown_timeout, wait).await })?; info!("ok, fully shutting down now"); diff --git a/tests/harness/mod.rs b/tests/harness/mod.rs index 3ed1f1ae7..767be3ce4 100644 --- a/tests/harness/mod.rs +++ b/tests/harness/mod.rs @@ -165,6 +165,7 @@ pub fn sccache_client_cfg( rewrite_includes_only: false, // TODO }, server_startup_timeout_ms: None, + server_shutdown_timeout_ms: None, port: None, } } diff --git a/tests/oauth.rs b/tests/oauth.rs index ba82a3afd..b33fa24fe 100755 --- a/tests/oauth.rs +++ b/tests/oauth.rs @@ -60,6 +60,7 @@ fn config_with_dist_auth( rewrite_includes_only: true, }, server_startup_timeout_ms: None, + server_shutdown_timeout_ms: None, port: None, } }