From 0dffe07b179be4c0702b8bae04156dc2dbae35f2 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 28 Nov 2022 11:01:11 +0100 Subject: [PATCH 1/4] Add configurable server timeout --- src/commands.rs | 33 ++++++++++++++++++--------------- src/config.rs | 25 ++++++++++++++++++++----- tests/harness/mod.rs | 1 + tests/oauth.rs | 1 + 4 files changed, 40 insertions(+), 20 deletions(-) diff --git a/src/commands.rs b/src/commands.rs index a07f77674..8f271d986 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -32,6 +32,7 @@ use std::io::{self, Write}; use std::os::unix::process::ExitStatusExt; use std::path::Path; use std::process; +use std::time::Duration; use strip_ansi_escapes::Writer; use tokio::io::AsyncReadExt; use tokio::runtime::Runtime; @@ -43,7 +44,7 @@ use crate::errors::*; pub const DEFAULT_PORT: u16 = 4226; /// The number of milliseconds to wait for server startup. -const SERVER_STARTUP_TIMEOUT_MS: u32 = 10000; +const SERVER_STARTUP_TIMEOUT: Duration = Duration::from_millis(10000); /// Get the port on which the server should listen. fn get_port() -> u16 { @@ -78,9 +79,7 @@ async fn read_server_startup_status( /// Re-execute the current executable as a background server, and wait /// for it to start up. #[cfg(not(windows))] -fn run_server_process() -> Result { - use std::time::Duration; - +fn run_server_process(startup_timeout: Option) -> Result { trace!("run_server_process"); let tempdir = tempfile::Builder::new().prefix("sccache").tempdir()?; let socket_path = tempdir.path().join("sock"); @@ -101,7 +100,7 @@ fn run_server_process() -> Result { read_server_startup_status(socket).await }; - let timeout = Duration::from_millis(SERVER_STARTUP_TIMEOUT_MS.into()); + let timeout = startup_timeout.unwrap_or(SERVER_STARTUP_TIMEOUT); runtime.block_on(async move { match tokio::time::timeout(timeout, startup).await { Ok(result) => result, @@ -159,12 +158,11 @@ fn redirect_error_log(f: File) -> Result<()> { /// Re-execute the current executable as a background server. #[cfg(windows)] -fn run_server_process() -> Result { +fn run_server_process(startup_timeout: Option) -> Result { use futures::StreamExt; use std::mem; use std::os::windows::ffi::OsStrExt; use std::ptr; - use std::time::Duration; use uuid::Uuid; use winapi::shared::minwindef::{DWORD, FALSE, LPVOID, TRUE}; use winapi::um::handleapi::CloseHandle; @@ -256,7 +254,7 @@ fn run_server_process() -> Result { read_server_startup_status(socket?).await }; - let timeout = Duration::from_millis(SERVER_STARTUP_TIMEOUT_MS.into()); + let timeout = startup_timeout.unwrap_or(SERVER_STARTUP_TIMEOUT); runtime.block_on(async move { match tokio::time::timeout(timeout, startup).await { Ok(result) => result, @@ -266,7 +264,10 @@ fn run_server_process() -> Result { } /// Attempt to connect to an sccache server listening on `port`, or start one if no server is running. -fn connect_or_start_server(port: u16) -> Result { +fn connect_or_start_server( + port: u16, + startup_timeout: Option, +) -> Result { trace!("connect_or_start_server({})", port); match connect_to_server(port) { Ok(server) => Ok(server), @@ -276,7 +277,7 @@ fn connect_or_start_server(port: u16) -> Result { { // If the connection was refused we probably need to start // the server. - match run_server_process()? { + match run_server_process(startup_timeout)? { ServerStartup::Ok { port: actualport } => { if port != actualport { // bail as the next connect_with_retry will fail @@ -578,11 +579,12 @@ 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; match cmd { Command::ShowStats(fmt) => { trace!("Command::ShowStats({:?})", fmt); - let srv = connect_or_start_server(get_port())?; + let srv = connect_or_start_server(get_port(), startup_timeout)?; let stats = request_stats(srv).context("failed to get stats from server")?; match fmt { StatsFormat::Text => stats.print(), @@ -605,7 +607,8 @@ pub fn run_command(cmd: Command) -> Result { Command::StartServer => { trace!("Command::StartServer"); println!("sccache: Starting the server..."); - let startup = run_server_process().context("failed to start server process")?; + let startup = + run_server_process(startup_timeout).context("failed to start server process")?; match startup { ServerStartup::Ok { port } => { if port != DEFAULT_PORT { @@ -626,7 +629,7 @@ pub fn run_command(cmd: Command) -> Result { } Command::ZeroStats => { trace!("Command::ZeroStats"); - let conn = connect_or_start_server(get_port())?; + let conn = connect_or_start_server(get_port(), startup_timeout)?; let stats = request_zero_stats(conn).context("couldn't zero stats on server")?; stats.print(); } @@ -688,7 +691,7 @@ pub fn run_command(cmd: Command) -> Result { ), Command::DistStatus => { trace!("Command::DistStatus"); - let srv = connect_or_start_server(get_port())?; + let srv = connect_or_start_server(get_port(), startup_timeout)?; let status = request_dist_status(srv).context("failed to get dist-status from server")?; serde_json::to_writer(&mut io::stdout(), &status)?; @@ -726,7 +729,7 @@ pub fn run_command(cmd: Command) -> Result { } => { trace!("Command::Compile {{ {:?}, {:?}, {:?} }}", exe, cmdline, cwd); let jobserver = unsafe { Client::new() }; - let conn = connect_or_start_server(get_port())?; + let conn = connect_or_start_server(get_port(), startup_timeout)?; let mut runtime = Runtime::new()?; let res = do_compile( ProcessCommandCreator::new(&jobserver), diff --git a/src/config.rs b/src/config.rs index 43310f311..2a247db81 100644 --- a/src/config.rs +++ b/src/config.rs @@ -431,6 +431,7 @@ impl Default for DistConfig { pub struct FileConfig { pub cache: CacheConfigs, pub dist: DistConfig, + pub server_startup_timeout_ms: Option, } // If the file doesn't exist or we can't read it, log the issue and proceed. If the @@ -657,10 +658,11 @@ pub struct Config { pub caches: Vec, pub fallback_cache: DiskCacheConfig, pub dist: DistConfig, + pub server_startup_timeout: Option, } impl Config { - pub fn load() -> Result { + pub fn load() -> Result { let env_conf = config_from_env()?; let file_conf_path = config_file("SCCACHE_CONF", "config"); @@ -668,23 +670,31 @@ impl Config { .context("Failed to load config file")? .unwrap_or_default(); - Ok(Config::from_env_and_file_configs(env_conf, file_conf)) + Ok(Self::from_env_and_file_configs(env_conf, file_conf)) } - fn from_env_and_file_configs(env_conf: EnvConfig, file_conf: FileConfig) -> Config { + fn from_env_and_file_configs(env_conf: EnvConfig, file_conf: FileConfig) -> Self { let mut conf_caches: CacheConfigs = Default::default(); - let FileConfig { cache, dist } = file_conf; + let FileConfig { + cache, + dist, + server_startup_timeout_ms, + } = file_conf; conf_caches.merge(cache); + let server_startup_timeout = + server_startup_timeout_ms.map(std::time::Duration::from_millis); + let EnvConfig { cache } = env_conf; conf_caches.merge(cache); let (caches, fallback_cache) = conf_caches.into_vec_and_fallback(); - Config { + Self { caches, fallback_cache, dist, + server_startup_timeout, } } } @@ -926,6 +936,7 @@ fn config_overrides() { ..Default::default() }, dist: Default::default(), + server_startup_timeout_ms: None, }; assert_eq!( @@ -947,6 +958,7 @@ fn config_overrides() { size: 5, }, dist: Default::default(), + server_startup_timeout: None, } ); } @@ -1024,6 +1036,8 @@ fn test_gcs_oauth_url() { #[test] fn full_toml_parse() { const CONFIG_STR: &str = r#" +server_startup_timeout_ms = 10000 + [dist] # where to find the scheduler scheduler_url = "http://1.2.3.4:10600" @@ -1131,6 +1145,7 @@ no_credentials = true toolchain_cache_size: 5368709120, rewrite_includes_only: false, }, + server_startup_timeout_ms: Some(10000), } ) } diff --git a/tests/harness/mod.rs b/tests/harness/mod.rs index 9df61a6dd..d8b5e11c0 100644 --- a/tests/harness/mod.rs +++ b/tests/harness/mod.rs @@ -154,6 +154,7 @@ pub fn sccache_client_cfg(tmpdir: &Path) -> sccache::config::FileConfig { toolchain_cache_size: TC_CACHE_SIZE, rewrite_includes_only: false, // TODO }, + server_startup_timeout_ms: None, } } #[cfg(feature = "dist-server")] diff --git a/tests/oauth.rs b/tests/oauth.rs index 59ae9549c..2093d34ce 100755 --- a/tests/oauth.rs +++ b/tests/oauth.rs @@ -59,6 +59,7 @@ fn config_with_dist_auth( toolchain_cache_size: 0, rewrite_includes_only: true, }, + server_startup_timeout_ms: None, } } From 7c71ce7e1577d2ced708508a6a8e7e644fab15d7 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 28 Nov 2022 11:25:00 +0100 Subject: [PATCH 2/4] Remove openssl dependency when enabling GCS --- Cargo.lock | 31 ++++++++++++++----------------- Cargo.toml | 4 ++-- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index fe40ba153..4be695367 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1377,19 +1377,6 @@ dependencies = [ "tokio-rustls", ] -[[package]] -name = "hyper-tls" -version = "0.5.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d6183ddfa99b85da61a140bea0efc93fdf56ceaa041b37d553518030827f9905" -dependencies = [ - "bytes", - "hyper", - "native-tls", - "tokio", - "tokio-native-tls", -] - [[package]] name = "hyperx" version = "1.4.0" @@ -2198,27 +2185,28 @@ dependencies = [ "http", "http-body", "hyper", - "hyper-tls", + "hyper-rustls", "ipnet", "js-sys", "log", "mime", - "mime_guess", - "native-tls", "once_cell", "percent-encoding", "pin-project-lite", + "rustls", + "rustls-pemfile", "serde", "serde_json", "serde_urlencoded", "tokio", - "tokio-native-tls", + "tokio-rustls", "tokio-util", "tower-service", "url", "wasm-bindgen", "wasm-bindgen-futures", "web-sys", + "webpki-roots", "winreg", ] @@ -3465,6 +3453,15 @@ dependencies = [ "untrusted", ] +[[package]] +name = "webpki-roots" +version = "0.22.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "368bfe657969fb01238bb756d351dcade285e0f6fcbd36dcb23359a5169975be" +dependencies = [ + "webpki", +] + [[package]] name = "which" version = "4.3.0" diff --git a/Cargo.toml b/Cargo.toml index a9c52a972..8bef8842c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,7 +56,7 @@ openssl = { version = "0.10.9", optional = true } percent-encoding = { version = "2", optional = true } rand = "0.8.4" regex = "1.7.0" -reqwest = { version = "0.11", features = ["json", "blocking", "stream"], optional = true } +reqwest = { version = "0.11", default-features = false, features = ["blocking", "json", "rustls-tls", "stream"], optional = true } retry = "2" ring = { version = "0.16", optional = true, features = ["std"] } rredis = { package = "redis", version = "0.21", optional = true, default-features = false, features = ["aio", "tokio-comp", "tokio-native-tls-comp"] } @@ -101,7 +101,7 @@ cc = "1.0" chrono = "0.4.22" itertools = "0.10" predicates = "=2.1.2" -thirtyfour_sync = "0.27" +thirtyfour_sync = { version = "0.27", default-features = false, features = ["reqwest-rustls-tls"] } once_cell = "1.16" serial_test = "0.9" wiremock = "0.5.15" From 85ea494cb48eb9bc493a739e066d284c9b8e48b7 Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Fri, 2 Dec 2022 08:58:24 +0100 Subject: [PATCH 3/4] Update lockfile --- Cargo.lock | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 4be695367..00c793a2f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1377,6 +1377,19 @@ dependencies = [ "tokio-rustls", ] +[[package]] +name = "hyper-tls" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6183ddfa99b85da61a140bea0efc93fdf56ceaa041b37d553518030827f9905" +dependencies = [ + "bytes", + "hyper", + "native-tls", + "tokio", + "tokio-native-tls", +] + [[package]] name = "hyperx" version = "1.4.0" @@ -2186,10 +2199,13 @@ dependencies = [ "http-body", "hyper", "hyper-rustls", + "hyper-tls", "ipnet", "js-sys", "log", "mime", + "mime_guess", + "native-tls", "once_cell", "percent-encoding", "pin-project-lite", @@ -2199,6 +2215,7 @@ dependencies = [ "serde_json", "serde_urlencoded", "tokio", + "tokio-native-tls", "tokio-rustls", "tokio-util", "tower-service", From 0658dadfe0270bc2bac6a7a30980587158e724fe Mon Sep 17 00:00:00 2001 From: Jake Shadle Date: Mon, 5 Dec 2022 12:19:02 +0100 Subject: [PATCH 4/4] Revert changes to cargo toml --- Cargo.lock | 14 -------------- Cargo.toml | 4 ++-- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 00c793a2f..fe40ba153 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2198,7 +2198,6 @@ dependencies = [ "http", "http-body", "hyper", - "hyper-rustls", "hyper-tls", "ipnet", "js-sys", @@ -2209,21 +2208,17 @@ dependencies = [ "once_cell", "percent-encoding", "pin-project-lite", - "rustls", - "rustls-pemfile", "serde", "serde_json", "serde_urlencoded", "tokio", "tokio-native-tls", - "tokio-rustls", "tokio-util", "tower-service", "url", "wasm-bindgen", "wasm-bindgen-futures", "web-sys", - "webpki-roots", "winreg", ] @@ -3470,15 +3465,6 @@ dependencies = [ "untrusted", ] -[[package]] -name = "webpki-roots" -version = "0.22.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "368bfe657969fb01238bb756d351dcade285e0f6fcbd36dcb23359a5169975be" -dependencies = [ - "webpki", -] - [[package]] name = "which" version = "4.3.0" diff --git a/Cargo.toml b/Cargo.toml index 8bef8842c..a9c52a972 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -56,7 +56,7 @@ openssl = { version = "0.10.9", optional = true } percent-encoding = { version = "2", optional = true } rand = "0.8.4" regex = "1.7.0" -reqwest = { version = "0.11", default-features = false, features = ["blocking", "json", "rustls-tls", "stream"], optional = true } +reqwest = { version = "0.11", features = ["json", "blocking", "stream"], optional = true } retry = "2" ring = { version = "0.16", optional = true, features = ["std"] } rredis = { package = "redis", version = "0.21", optional = true, default-features = false, features = ["aio", "tokio-comp", "tokio-native-tls-comp"] } @@ -101,7 +101,7 @@ cc = "1.0" chrono = "0.4.22" itertools = "0.10" predicates = "=2.1.2" -thirtyfour_sync = { version = "0.27", default-features = false, features = ["reqwest-rustls-tls"] } +thirtyfour_sync = "0.27" once_cell = "1.16" serial_test = "0.9" wiremock = "0.5.15"