From e31be3c9a31a4c9470aa5f3777805e126dcf8412 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sun, 19 Feb 2023 16:34:03 +0800 Subject: [PATCH 01/11] Enable sccache log Signed-off-by: Xuanwo --- tests/harness/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/harness/mod.rs b/tests/harness/mod.rs index 84ed36873..14e3618a6 100644 --- a/tests/harness/mod.rs +++ b/tests/harness/mod.rs @@ -46,8 +46,8 @@ pub fn start_local_daemon(cfg_path: &Path, cached_cfg_path: &Path) { let _ = sccache_command() .arg("--start-server") // Uncomment following lines to debug locally. - // .env("SCCACHE_LOG", "debug") - // .env("SCCACHE_ERROR_LOG", "/tmp/sccache_log.txt") + .env("SCCACHE_LOG", "debug") + .env("SCCACHE_ERROR_LOG", "sccache_log.txt") .env("SCCACHE_CONF", cfg_path) .env("SCCACHE_CACHED_CONF", cached_cfg_path) .status() From 130b13c76e510ab97f17c551a1f0f912fa1a0810 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sun, 19 Feb 2023 16:53:04 +0800 Subject: [PATCH 02/11] Try adding more log for test Signed-off-by: Xuanwo --- .github/workflows/ci.yml | 8 ++++---- tests/harness/mod.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 336e85cf7..d20203960 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -82,10 +82,10 @@ jobs: if: ${{ matrix.os == 'ubuntu-20.04' || matrix.os == 'ubuntu-22.04' }} - name: Build tests - run: cargo test --no-run --locked --all-targets --verbose ${{ matrix.extra_args }} + run: cargo test --no-run --locked --all-targets ${{ matrix.extra_args }} - name: Run tests - run: cargo test --locked --all-targets --verbose ${{ matrix.extra_args }} + run: cargo test --locked --all-targets ${{ matrix.extra_args }} - name: Upload failure if: failure() @@ -150,7 +150,7 @@ jobs: if: ${{ matrix.target == 'aarch64-unknown-linux-musl' }} - name: Build - run: cargo build --locked --release --verbose --bin ${{ matrix.binary || 'sccache' }} --target ${{ matrix.target }} --features=openssl/vendored ${{ matrix.extra_args }} + run: cargo build --locked --release --bin ${{ matrix.binary || 'sccache' }} --target ${{ matrix.target }} --features=openssl/vendored ${{ matrix.extra_args }} env: CARGO_TARGET_AARCH64_UNKNOWN_LINUX_MUSL_LINKER: aarch64-linux-musl-gcc MACOSX_DEPLOYMENT_TARGET: ${{ matrix.macosx_deployment_target }} @@ -211,7 +211,7 @@ jobs: run: cargo install grcov - name: Execute tests - run: cargo test --no-fail-fast --locked --all-targets --verbose ${{ matrix.extra_args }} + run: cargo test --no-fail-fast --locked --all-targets ${{ matrix.extra_args }} env: CARGO_INCREMENTAL: "0" RUSTC_WRAPPER: "" diff --git a/tests/harness/mod.rs b/tests/harness/mod.rs index 14e3618a6..6cf3b9963 100644 --- a/tests/harness/mod.rs +++ b/tests/harness/mod.rs @@ -264,7 +264,7 @@ impl DistSystem { "-e", "SCCACHE_NO_DAEMON=1", "-e", - "SCCACHE_LOG=sccache=trace", + "SCCACHE_LOG=debug", "-e", "RUST_BACKTRACE=1", "--network", @@ -332,7 +332,7 @@ impl DistSystem { "--name", &server_name, "-e", - "SCCACHE_LOG=sccache=trace", + "SCCACHE_LOG=debug", "-e", "RUST_BACKTRACE=1", "--network", From a58ccf7a763614ae471445d9fa3983515366e25d Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sun, 19 Feb 2023 17:40:56 +0800 Subject: [PATCH 03/11] Sleep to wait for server Signed-off-by: Xuanwo --- tests/dist.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/dist.rs b/tests/dist.rs index 516d0a179..377b4bc02 100644 --- a/tests/dist.rs +++ b/tests/dist.rs @@ -17,6 +17,7 @@ use sccache::dist::{ }; use std::ffi::OsStr; use std::path::Path; +use std::time::Duration; use sccache::errors::*; @@ -115,6 +116,8 @@ fn test_dist_restartedserver() { basic_compile(tmpdir, &sccache_cfg_path, &sccache_cached_cfg_path); system.restart_server(&server_handle); + // Give our server some time to get start. + std::thread::sleep(Duration::from_secs(1)); basic_compile(tmpdir, &sccache_cfg_path, &sccache_cached_cfg_path); get_stats(|info| { From 14eea2440563755702663aaecee052de7c4f2ed6 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sun, 19 Feb 2023 21:44:45 +0800 Subject: [PATCH 04/11] Upload sccache Signed-off-by: Xuanwo --- .github/actions/artifact_failure/action.yml | 4 +++- tests/dist.rs | 2 +- tests/harness/mod.rs | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.github/actions/artifact_failure/action.yml b/.github/actions/artifact_failure/action.yml index febe873ce..d83c76229 100644 --- a/.github/actions/artifact_failure/action.yml +++ b/.github/actions/artifact_failure/action.yml @@ -25,4 +25,6 @@ runs: - uses: actions/upload-artifact@v3 with: name: ${{ inputs.name }} - path: target/failure-${{ inputs.name }}.tar.gz + path: | + target/failure-${{ inputs.name }}.tar.gz + /tmp/sccache_*.txt diff --git a/tests/dist.rs b/tests/dist.rs index 377b4bc02..0e21dd735 100644 --- a/tests/dist.rs +++ b/tests/dist.rs @@ -26,7 +26,7 @@ mod harness; fn basic_compile(tmpdir: &Path, sccache_cfg_path: &Path, sccache_cached_cfg_path: &Path) { let envs: Vec<(_, &OsStr)> = vec![ ("RUST_BACKTRACE", "1".as_ref()), - ("SCCACHE_LOG", "trace".as_ref()), + ("SCCACHE_LOG", "debug".as_ref()), ("SCCACHE_CONF", sccache_cfg_path.as_ref()), ("SCCACHE_CACHED_CONF", sccache_cached_cfg_path.as_ref()), ]; diff --git a/tests/harness/mod.rs b/tests/harness/mod.rs index 6cf3b9963..0ed770452 100644 --- a/tests/harness/mod.rs +++ b/tests/harness/mod.rs @@ -47,13 +47,14 @@ pub fn start_local_daemon(cfg_path: &Path, cached_cfg_path: &Path) { .arg("--start-server") // Uncomment following lines to debug locally. .env("SCCACHE_LOG", "debug") - .env("SCCACHE_ERROR_LOG", "sccache_log.txt") + .env("SCCACHE_ERROR_LOG", "/tmp/sccache_local_daemon.txt") .env("SCCACHE_CONF", cfg_path) .env("SCCACHE_CACHED_CONF", cached_cfg_path) .status() .unwrap() .success(); } + pub fn stop_local_daemon() { trace!("sccache --stop-server"); drop( From 4233c883adfa7a47742203deb69ca100789f96c4 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sun, 19 Feb 2023 21:45:20 +0800 Subject: [PATCH 05/11] Remove sleep Signed-off-by: Xuanwo --- tests/dist.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/tests/dist.rs b/tests/dist.rs index 0e21dd735..4885e0451 100644 --- a/tests/dist.rs +++ b/tests/dist.rs @@ -17,7 +17,6 @@ use sccache::dist::{ }; use std::ffi::OsStr; use std::path::Path; -use std::time::Duration; use sccache::errors::*; @@ -116,8 +115,6 @@ fn test_dist_restartedserver() { basic_compile(tmpdir, &sccache_cfg_path, &sccache_cached_cfg_path); system.restart_server(&server_handle); - // Give our server some time to get start. - std::thread::sleep(Duration::from_secs(1)); basic_compile(tmpdir, &sccache_cfg_path, &sccache_cached_cfg_path); get_stats(|info| { From 8053df02f89311352e98ab74fa09180ef3957620 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sun, 19 Feb 2023 22:08:07 +0800 Subject: [PATCH 06/11] Disable connection pool for all http client Signed-off-by: Xuanwo --- src/bin/sccache-dist/token_check.rs | 6 +++--- src/dist/client_auth.rs | 3 ++- src/dist/http.rs | 7 ++++--- src/util.rs | 8 ++++++++ 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/bin/sccache-dist/token_check.rs b/src/bin/sccache-dist/token_check.rs index b22d61932..0f1c7b65b 100644 --- a/src/bin/sccache-dist/token_check.rs +++ b/src/bin/sccache-dist/token_check.rs @@ -1,7 +1,7 @@ use crate::jwt; use anyhow::{bail, Context, Result}; use sccache::dist::http::{ClientAuthCheck, ClientVisibleMsg}; -use sccache::util::BASE64_URL_SAFE_ENGINE; +use sccache::util::{new_reqwest_blocking_client, BASE64_URL_SAFE_ENGINE}; use std::collections::HashMap; use std::result::Result as StdResult; use std::sync::Mutex; @@ -97,7 +97,7 @@ impl MozillaCheck { pub fn new(required_groups: Vec) -> Self { Self { auth_cache: Mutex::new(HashMap::new()), - client: reqwest::blocking::Client::new(), + client: new_reqwest_blocking_client(), required_groups, } } @@ -266,7 +266,7 @@ impl ProxyTokenCheck { let maybe_auth_cache: Option, Duration)>> = cache_secs.map(|secs| Mutex::new((HashMap::new(), Duration::from_secs(secs)))); Self { - client: reqwest::blocking::Client::new(), + client: new_reqwest_blocking_client(), maybe_auth_cache, url, } diff --git a/src/dist/client_auth.rs b/src/dist/client_auth.rs index 685b5c423..099171361 100644 --- a/src/dist/client_auth.rs +++ b/src/dist/client_auth.rs @@ -84,6 +84,7 @@ mod code_grant_pkce { html_response, json_response, query_pairs, MIN_TOKEN_VALIDITY, MIN_TOKEN_VALIDITY_WARNING, REDIRECT_WITH_AUTH_JSON, }; + use crate::util::new_reqwest_blocking_client; use crate::util::BASE64_URL_SAFE_ENGINE; use futures::channel::oneshot; use hyper::{Body, Method, Request, Response, StatusCode}; @@ -242,7 +243,7 @@ mod code_grant_pkce { grant_type: GRANT_TYPE_PARAM_VALUE, redirect_uri, }; - let client = reqwest::blocking::Client::new(); + let client = new_reqwest_blocking_client(); let res = client.post(token_url).json(&token_request).send()?; if !res.status().is_success() { bail!( diff --git a/src/dist/http.rs b/src/dist/http.rs index b4e9f7e17..74505c596 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -250,6 +250,7 @@ pub mod urls { #[cfg(feature = "dist-server")] mod server { use crate::jwt; + use crate::util::new_reqwest_blocking_client; use byteorder::{BigEndian, ReadBytesExt}; use flate2::read::ZlibDecoder as ZlibReadDecoder; use rand::{rngs::OsRng, RngCore}; @@ -685,7 +686,7 @@ mod server { check_server_auth, } = self; let requester = SchedulerRequester { - client: Mutex::new(reqwest::blocking::Client::new()), + client: Mutex::new(new_reqwest_blocking_client()), }; macro_rules! check_server_auth_or_err { @@ -939,14 +940,14 @@ mod server { let job_authorizer = JWTJobAuthorizer::new(jwt_key); let heartbeat_url = urls::scheduler_heartbeat_server(&scheduler_url); let requester = ServerRequester { - client: reqwest::blocking::Client::new(), + client: new_reqwest_blocking_client(), scheduler_url, scheduler_auth: scheduler_auth.clone(), }; // TODO: detect if this panics thread::spawn(move || { - let client = reqwest::blocking::Client::new(); + let client = new_reqwest_blocking_client(); loop { trace!("Performing heartbeat"); match bincode_req( diff --git a/src/util.rs b/src/util.rs index b3441b3e3..fb2026d69 100644 --- a/src/util.rs +++ b/src/util.rs @@ -506,6 +506,14 @@ pub fn daemonize() -> Result<()> { Ok(()) } +/// Disable connection pool to avoid broken connection between runtime +pub fn new_reqwest_blocking_client() -> reqwest::blocking::Client { + reqwest::blocking::Client::builder() + .pool_max_idle_per_host(0) + .build() + .expect("http client must build with success") +} + #[cfg(test)] mod tests { use super::OsStrExt; From fc83f13546c261f66f999be87d7fb1829e28a97b Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Sun, 19 Feb 2023 22:43:36 +0800 Subject: [PATCH 07/11] Dsiable keep-alive Signed-off-by: Xuanwo --- src/dist/http.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/dist/http.rs b/src/dist/http.rs index 74505c596..73d9e4dc2 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -1153,6 +1153,8 @@ mod client { let timeout = Duration::new(REQUEST_TIMEOUT_SECS, 0); let new_client_async = client_async_builder .timeout(timeout) + // Disable keep-alive + .pool_max_idle_per_host(0) .build() .context("failed to create an async HTTP client")?; // Use the updated certificates From 52ad6a9876ad420536db34ec4752f9c50673127a Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Wed, 22 Feb 2023 09:18:01 +0800 Subject: [PATCH 08/11] Add comment for new_reqwest_blocking_client Signed-off-by: Xuanwo --- src/util.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/util.rs b/src/util.rs index fb2026d69..16ea53d8c 100644 --- a/src/util.rs +++ b/src/util.rs @@ -507,6 +507,15 @@ pub fn daemonize() -> Result<()> { } /// Disable connection pool to avoid broken connection between runtime +/// +/// # TODO +/// +/// We should refactor sccache current model to make sure that we only have +/// one tokio runtime and keep reqwest alive inside it. +/// +/// --- +/// +/// More details could be found at https://github.com/mozilla/sccache/pull/1563 pub fn new_reqwest_blocking_client() -> reqwest::blocking::Client { reqwest::blocking::Client::builder() .pool_max_idle_per_host(0) From 7a887320bea50b5442f3746eda93180e4d862a45 Mon Sep 17 00:00:00 2001 From: Xuanwo Date: Wed, 22 Feb 2023 15:11:29 +0800 Subject: [PATCH 09/11] Make build happy Signed-off-by: Xuanwo --- src/compiler/compiler.rs | 2 ++ src/dist/http.rs | 14 +++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index c515d3313..560026c3c 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -36,6 +36,8 @@ use fs_err as fs; use std::borrow::Cow; use std::ffi::{OsStr, OsString}; use std::fmt; +#[cfg(not(feature = "dist-client"))] +use std::fs; use std::future::Future; use std::io::prelude::*; use std::path::{Path, PathBuf}; diff --git a/src/dist/http.rs b/src/dist/http.rs index 7c987ab07..d854b5595 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -22,9 +22,6 @@ pub use self::server::{ mod common { use http::header; - use http::header::CONNECTION; - use http::header::CONTENT_LENGTH; - use http::header::CONTENT_TYPE; #[cfg(feature = "dist-server")] use std::collections::HashMap; use std::fmt; @@ -64,9 +61,12 @@ mod common { Ok(self.bytes(bytes)) } fn bytes(self, bytes: Vec) -> Self { - self.header(CONTENT_TYPE, mime::APPLICATION_OCTET_STREAM.to_string()) - .header(CONTENT_LENGTH, bytes.len()) - .body(bytes) + self.header( + header::CONTENT_TYPE, + mime::APPLICATION_OCTET_STREAM.to_string(), + ) + .header(header::CONTENT_LENGTH, bytes.len()) + .body(bytes) } fn bearer_auth(self, token: String) -> Self { self.bearer_auth(token) @@ -79,7 +79,7 @@ mod common { ) -> Result { // Work around tiny_http issue #151 by disabling HTTP pipeline with // `Connection: close`. - let res = req.header(CONNECTION, "close").send().await?; + let res = req.header(header::CONNECTION, "close").send().await?; let status = res.status(); let bytes = res.bytes().await?; From a426f30d5d04a00b0b28e9284e02e64f803dc7f3 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 22 Feb 2023 14:15:45 +0100 Subject: [PATCH 10/11] use fs_err Co-authored-by: Bernhard Schuster --- src/compiler/compiler.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 560026c3c..40d61c942 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -37,7 +37,7 @@ use std::borrow::Cow; use std::ffi::{OsStr, OsString}; use std::fmt; #[cfg(not(feature = "dist-client"))] -use std::fs; +use fs_err as fs; use std::future::Future; use std::io::prelude::*; use std::path::{Path, PathBuf}; From 79052164a871f167811bc6381ec4c19061b15998 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Wed, 22 Feb 2023 16:34:58 +0100 Subject: [PATCH 11/11] fix the rustfmt --- src/compiler/compiler.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 40d61c942..8fe962c29 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -31,13 +31,10 @@ use crate::mock_command::{exit_status, CommandChild, CommandCreatorSync, RunComm use crate::util::{fmt_duration_as_secs, ref_env, run_input_output}; use filetime::FileTime; use fs::File; -#[cfg(feature = "dist-client")] use fs_err as fs; use std::borrow::Cow; use std::ffi::{OsStr, OsString}; use std::fmt; -#[cfg(not(feature = "dist-client"))] -use fs_err as fs; use std::future::Future; use std::io::prelude::*; use std::path::{Path, PathBuf};