From fe8c6d8d8628e471eb5dac5927db5c59b8e8c3ad Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 19 Jun 2026 15:25:12 -0600 Subject: [PATCH 01/43] Don't panic when a finished compile has neither code nor signal get_signal did `status.signal().expect("must have signal")`, assuming the Unix invariant that an ExitStatus with no exit code was terminated by a signal. That does not always hold: an ExitStatus reconstructed for a distributed compile (or an abnormal wait status such as WIFSTOPPED) can report neither a code nor a signal. When that happened the expect() panicked the compile task, which the server surfaced as a misleading "Failed to bind socket" and, under load, repeatedly fell back to local compilation. Return Option from get_signal and assign it straight into res.signal, so a compile that reports neither code nor signal leaves res.signal unset instead of crashing the in-flight task. The Windows arm returns None rather than panicking; ExitStatus::code() is always Some there, so the signal branch is never reached anyway. Add a unit test covering a real terminating signal (SIGKILL) and the neither-code-nor-signal case (WIFSTOPPED via from_raw), which previously panicked. Signed-off-by: Javier Tia --- src/server.rs | 39 +++++++++++++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 6 deletions(-) diff --git a/src/server.rs b/src/server.rs index 143ea6375..227b95318 100644 --- a/src/server.rs +++ b/src/server.rs @@ -133,13 +133,20 @@ fn notify_server_startup(name: Option<&OsString>, status: ServerStartup) -> Resu } #[cfg(unix)] -fn get_signal(status: ExitStatus) -> i32 { +fn get_signal(status: ExitStatus) -> Option { use std::os::unix::prelude::*; - status.signal().expect("must have signal") + // None when the process produced neither an exit code nor a terminating + // signal - e.g. an ExitStatus synthesized from a distributed-compile result, + // or an abnormal wait status. Previously this was `.expect("must have + // signal")`, which panicked the compile task (surfacing as a misleading + // "Failed to bind socket") and could be tripped repeatedly under load. + status.signal() } #[cfg(windows)] -fn get_signal(_status: ExitStatus) -> i32 { - panic!("no signals on windows") +fn get_signal(_status: ExitStatus) -> Option { + // On Windows ExitStatus::code() is always Some, so the signal branch is + // never reached; return None rather than panicking. + None } pub struct DistClientContainer { @@ -1573,7 +1580,7 @@ where match status.code() { Some(code) => res.retcode = Some(code), - None => res.signal = Some(get_signal(status)), + None => res.signal = get_signal(status), } res.stdout = stdout; @@ -1589,7 +1596,7 @@ where match output.status.code() { Some(code) => res.retcode = Some(code), - None => res.signal = Some(get_signal(output.status)), + None => res.signal = get_signal(output.status), } res.stdout = output.stdout; res.stderr = output.stderr; @@ -2444,6 +2451,26 @@ fn waits_until_zero() { mod tests { use super::*; + #[cfg(unix)] + #[test] + fn test_get_signal_handles_signal_and_abnormal_status() { + use std::os::unix::process::ExitStatusExt; + + // Terminated by SIGKILL: a real terminating signal is reported. + let killed = ExitStatus::from_raw(9); + assert_eq!(killed.code(), None); + assert_eq!(get_signal(killed), Some(9)); + + // A wait status that is neither a normal exit (code) nor a terminating + // signal - here WIFSTOPPED (low byte 0x7f). The same neither-code-nor- + // signal shape can arise from an ExitStatus synthesized for a + // distributed compile. get_signal must return None, not panic + // "must have signal" (which used to crash the in-flight compile task). + let abnormal = ExitStatus::from_raw(0x7f); + assert_eq!(abnormal.code(), None); + assert_eq!(get_signal(abnormal), None); + } + struct StringWriter { buffer: String, } From 75a38941694ce5c409f12f94ba78f492f5985f5c Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 19 Jun 2026 17:38:39 -0600 Subject: [PATCH 02/43] Bundle relocated interpreter's libdir into the dist toolchain package sccache-dist packages a toolchain's shared libraries by parsing `ldd` output, which resolves NEEDED libraries against the host's dynamic loader. Yocto/OpenEmbedded "uninative" cross toolchains ship a relocated glibc whose loader has a built-in search path pointing at its own sysroot. For those binaries `ldd` reports host paths (e.g. /usr/lib/libm.so.6), yet inside the build sandbox the relocated loader searches its own sysroot lib dir, where those libraries were never packaged. The remote compile then dies with "libm.so.6: cannot open shared object file" and silently falls back to local compilation, so distribution never actually runs. When a packaged executable's PT_INTERP lives outside the standard host loader directories, also bundle the interpreter's own directory. That directory holds the libc/libm the relocated loader resolves against, so they land at the absolute path the loader searches inside the sandbox. Standard host toolchains are untouched: their interpreter is under /lib, /lib64, or /usr/lib, so the existing ldd-only path is preserved. Signed-off-by: Javier Tia --- src/dist/pkg.rs | 87 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/src/dist/pkg.rs b/src/dist/pkg.rs index 8bc1744a4..e7cab535c 100644 --- a/src/dist/pkg.rs +++ b/src/dist/pkg.rs @@ -98,6 +98,20 @@ mod toolchain_imp { } pub fn add_executable_and_deps(&mut self, executable: PathBuf) -> Result<()> { + // A relocated program interpreter (e.g. a Yocto/OE uninative + // toolchain) resolves its libc/libm against its own sysroot, not the + // host paths ldd reports. Bundle the interpreter's directory so those + // libraries exist where the loader looks inside the build sandbox. + if let Some(libdir) = read_elf_interpreter(&executable) + .and_then(|interp| relocated_interpreter_libdir(&interp)) + { + self.add_dir_contents(&libdir).with_context(|| { + format!( + "Failed to bundle relocated interpreter libdir {}", + libdir.display() + ) + })?; + } let mut remaining = vec![executable]; while let Some(obj_path) = remaining.pop() { assert!(obj_path.is_absolute()); @@ -350,6 +364,50 @@ mod toolchain_imp { libs } + /// Read a binary's ELF program interpreter (PT_INTERP), if it has one. + /// + /// Returns `None` for static binaries, non-ELF or non-64-bit files, and + /// anything that fails to parse -- callers treat that as "nothing special + /// to bundle", preserving the default ldd-only behaviour. + fn read_elf_interpreter(executable: &Path) -> Option { + use object::Endianness; + use object::read::elf::{ElfFile64, ProgramHeader}; + + let data = fs::read(executable).ok()?; + let elf = ElfFile64::::parse(data.as_slice()).ok()?; + let endian = elf.endian(); + for header in elf.elf_program_headers() { + if let Ok(Some(interp)) = header.interpreter(endian, elf.data()) { + return str::from_utf8(interp).ok().map(PathBuf::from); + } + } + None + } + + /// If `interp` is a relocated program interpreter -- one living outside the + /// standard host loader directories -- return the directory that should be + /// bundled alongside the toolchain. + /// + /// Yocto/OpenEmbedded "uninative" cross toolchains ship their own glibc and + /// loader, and the loader's built-in search path points at its own sysroot + /// lib dir rather than the host's. `ldd` resolves a binary's NEEDED + /// libraries against the *host* loader, so for these binaries it reports + /// host paths (e.g. /usr/lib/libm.so.6) that do not exist where the + /// relocated loader actually searches at runtime. Bundling the + /// interpreter's own directory -- which holds the matching libc/libm and + /// the loader itself -- makes the toolchain resolvable inside the sandbox. + fn relocated_interpreter_libdir(interp: &Path) -> Option { + const STANDARD_LOADER_PREFIXES: &[&str] = &["/lib/", "/lib64/", "/usr/lib/", "/usr/lib64/"]; + let interp_str = interp.to_str()?; + if STANDARD_LOADER_PREFIXES + .iter() + .any(|prefix| interp_str.starts_with(prefix)) + { + return None; + } + interp.parent().map(Path::to_path_buf) + } + #[test] fn test_ldd_parse() { let ubuntu_ls_output = "\tlinux-vdso.so.1 => (0x00007fffcfffe000) @@ -409,6 +467,35 @@ mod toolchain_imp { ] ); } + + #[test] + fn test_relocated_interpreter_libdir() { + // Standard host loaders are left to ldd's host resolution. + for standard in [ + "/lib64/ld-linux-x86-64.so.2", + "/usr/lib64/ld-linux-x86-64.so.2", + "/usr/lib/ld-linux-aarch64.so.1", + "/lib/x86_64-linux-gnu/ld-linux-x86-64.so.2", + ] { + assert_eq!( + relocated_interpreter_libdir(Path::new(standard)), + None, + "{standard} is a standard host loader and must not trigger bundling" + ); + } + + // A relocated loader (e.g. a Yocto/OE uninative toolchain) resolves its + // libc/libm against its own sysroot lib dir, which ldd does not report. + // Return that directory so it gets bundled into the toolchain package. + assert_eq!( + relocated_interpreter_libdir(Path::new( + "/home/u/build/tmp/sysroots-uninative/x86_64-linux/lib/ld-linux-x86-64.so.2" + )), + Some(PathBuf::from( + "/home/u/build/tmp/sysroots-uninative/x86_64-linux/lib" + )) + ); + } } pub fn make_tar_header(src: &Path, dest: &str) -> io::Result { From a6cc588d9c1635a5c959cfbdc33ab94b8e063da6 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Tue, 23 Jun 2026 13:02:45 -0600 Subject: [PATCH 03/43] compiler/gcc: fix distributed compile of relative input paths sccache-dist ships the preprocessed input through the inputs packager keyed on the absolute, simplified path cwd.join(input) (CInputsPackager), but the distributed compile command referenced the raw parsed_args.input. For out-of-tree builds the input is relative (e.g. OpenEmbedded's ../sources/foo.c), so the command and the packaged input disagreed and the build-server compiled a path the inputs were never placed at, failing with "cc1: fatal error: ... No such file or directory". Transform the same absolute, simplified path in the dist command so it matches the packaged input. An absolute input is unchanged, since cwd.join of an absolute path returns it verbatim. Signed-off-by: Javier Tia --- src/compiler/gcc.rs | 69 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 68 insertions(+), 1 deletion(-) diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index 84ddbd3c9..e6ed63c7e 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -1029,7 +1029,14 @@ where } arguments.extend(vec![ parsed_args.compilation_flag.clone().into_string().ok()?, - path_transformer.as_dist(&parsed_args.input)?, + // Match the path the inputs packager ships the preprocessed + // content at (CInputsPackager uses cwd.join(input) + simplify_path). + // parsed_args.input is relative for out-of-tree builds (e.g. OE's + // `../sources/foo.c`); passing it raw made the server look for the + // input at a path the package never placed it, failing with + // "No such file or directory". Absolutize+simplify so both agree. + path_transformer + .as_dist(&dist::pkg::simplify_path(&cwd.join(&parsed_args.input)).ok()?)?, "-o".into(), path_transformer.as_dist(out_file)?, ]); @@ -2486,6 +2493,66 @@ mod test { assert_eq!(0, creator.lock().unwrap().children.len()); } + #[test] + #[cfg(feature = "dist-client")] + fn test_compile_relative_input_dist_command_is_absolute() { + // Regression: the dist command must reference the same absolute, simplified + // input path the inputs packager ships the preprocessed content at + // (CInputsPackager uses cwd.join(input) + simplify_path). For out-of-tree + // builds the input is relative (e.g. OE's `../sources/foo.c`); passing it + // raw made the build-server look where the input was never placed, failing + // with "No such file or directory" (OE xz/glibc out-of-tree compiles). + let f = TestFixture::new(); + let parsed_args = ParsedArguments { + input: "../foo.c".into(), + double_dash_input: false, + language: Language::C, + compilation_flag: "-c".into(), + depfile: None, + outputs: vec![( + "obj", + ArtifactDescriptor { + path: "foo.o".into(), + optional: false, + }, + )] + .into_iter() + .collect(), + dependency_args: vec![], + preprocessor_args: vec![], + common_args: vec![], + arch_args: vec![], + unhashed_args: vec![], + extra_dist_files: vec![], + extra_hash_files: vec![], + msvc_show_includes: false, + profile_generate: false, + color_mode: ColorMode::Auto, + suppress_rewrite_includes_only: false, + too_hard_for_preprocessor_cache_mode: None, + }; + let mut path_transformer = dist::PathTransformer::new(); + let (_command, dist_command, _cacheable) = generate_compile_commands( + &mut path_transformer, + &f.bins[0], + &parsed_args, + f.tempdir.path(), + &[], + CCompilerKind::Gcc, + false, + language_to_gcc_arg, + ) + .unwrap(); + let dist_command = dist_command.expect("relative input must still produce a dist command"); + // No argument may carry an unresolved `..`: the input must be the simplified + // absolute path the packager ships, not the raw relative one. + assert!( + !dist_command.arguments.iter().any(|a| a.contains("..")), + "dist command still references a relative input: {:?}", + dist_command.arguments + ); + } + #[test] fn test_compile_simple_verbose_short() { let creator = new_creator(); From b94b293e2c73eda14bb6dbc85d5c2c52109ac4e8 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Tue, 23 Jun 2026 14:09:55 -0600 Subject: [PATCH 04/43] compiler: log distributed-compile decisions at info level sccache logged the distribute-vs-local decision only at debug ("Compiling locally", "Attempting distributed compilation"), while an infrastructure fallback warned. At info a successful distribution and a local compile were both silent, so the only visible dist signal was the failure path - leaving no way to see the distribute/fallback ratio without the full debug firehose. Diagnosing why a distributed build under-distributes meant guessing. Promote the decision points to info and add a log on successful distribution naming the server and exit code, so SCCACHE_LOG=info gives a per-compile dist trace (attempt then distributed-on-server, compiled-locally, or falling-back-with-reason). sccache only emits logs when SCCACHE_LOG is set, so default runs are unaffected. Signed-off-by: Javier Tia --- src/compiler/compiler.rs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 78bb5a433..77eea183e 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -877,7 +877,10 @@ where let dist_client = match dist_compile_cmd.clone().and(dist_client) { Some(dc) => dc, None => { - debug!("[{}]: Compiling locally", out_pretty); + info!( + "[{}]: Compiling locally (not eligible for distributed compilation)", + out_pretty + ); return compile_cmd .execute(service, &creator) .await @@ -885,7 +888,7 @@ where } }; - debug!("[{}]: Attempting distributed compilation", out_pretty); + info!("[{}]: Attempting distributed compilation", out_pretty); let out_pretty2 = out_pretty.clone(); let local_executable = compile_cmd.get_executable(); @@ -1044,6 +1047,12 @@ where .splice(0..0, server_info.as_bytes().to_vec()); } + info!( + "[{}]: Distributed compilation finished on {} (exit code {})", + out_pretty, + server_id.addr(), + jc.output.code + ); Ok((DistType::Ok(server_id), jc.output.into())) }; From ef83af8f875fad5d5de4112ae7cab26df605c093 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Tue, 23 Jun 2026 15:03:30 -0600 Subject: [PATCH 05/43] compiler: fall back to local on distributed-compile failure A distributed compile the build-server rejects is often a distribution artifact, not a genuine compiler error: an object that .incbin's a binary the inputs packager does not ship - the kernel's vdso, embedded-config, and dtb wrappers - cannot be assembled remotely and returns non-zero, which failed the whole build with no recourse and forced the kernel to be excluded wholesale. Treat a non-zero remote result as a fallback trigger rather than a terminal error: recompile locally, which either succeeds (confirming a dist-only artifact) or reproduces the genuine error. A remote failure can no longer break a build that would compile locally. Only failing dist compiles are affected - successful ones are returned unchanged - so the kernel now distributes (1124/1128 compiles), with its handful of .incbin objects falling back to local. Signed-off-by: Javier Tia --- src/compiler/compiler.rs | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 77eea183e..d3ceee25a 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -973,7 +973,7 @@ where ) })?; - let mut jc = match jres { + let jc = match jres { dist::RunJobResult::Complete(jc) => jc, dist::RunJobResult::JobNotFound => bail!("Job {} not found on server", job_id), }; @@ -1039,19 +1039,26 @@ where ); if jc.output.code != 0 { - // Add server info to help diagnose host-specific failures, e.g. due to flaky hardware. - // Failed builds are not cached so this tampering should not cause too much trouble. - let server_info = format!("sccache: Job failed on server {}:\n", server_id.addr()); - jc.output - .stderr - .splice(0..0, server_info.as_bytes().to_vec()); + // A non-zero remote result is frequently a distribution artifact + // rather than a genuine compiler error: e.g. an object that + // .incbin's a binary the inputs packager did not ship (the kernel's + // vdso/dtb/embedded-config wrappers), which the build-server cannot + // assemble. Fall back to a local recompile via the or_else below - it + // either succeeds (confirming a dist-only artifact) or reproduces the + // real error locally, so a remote failure never breaks a build that + // would compile fine locally. This only affects failing dist + // compiles; successful ones are returned unchanged above. + bail!( + "distributed compile on {} returned exit code {}; recompiling locally", + server_id.addr(), + jc.output.code + ); } info!( - "[{}]: Distributed compilation finished on {} (exit code {})", + "[{}]: Distributed compilation finished on {}", out_pretty, - server_id.addr(), - jc.output.code + server_id.addr() ); Ok((DistType::Ok(server_id), jc.output.into())) }; From 8bd7e752be676a284c41aeb59a9ab2180a8154ff Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Wed, 24 Jun 2026 16:50:12 -0600 Subject: [PATCH 06/43] compiler: fall back to local when a dist compile drops an output A distributed compile can return a successful (exit 0) result yet omit a declared output. glibc's ldconfig.o and sprof.o compile fine on the build-server, but it does not return their `.o.dt` dependency file, so zipping the compiler outputs fails fatally ("failed to open file ...o.dt: No such file") with no recourse. One dropped output among 6427 forced glibc to be excluded from distribution wholesale, even though 6425 of its compiles distribute cleanly. Capture the declared output paths before the compilation is moved into the packagers, and after a successful remote compile verify each one exists on disk. A missing output is a distribution artifact, not a compiler error, so bail into the existing local-recompile fallback (the same one that already salvages non-zero remote results), which reproduces the full output set. Only compiles whose dist output set is incomplete are affected; complete ones are returned unchanged. glibc now distributes, with ldconfig.o and sprof.o falling back to local. Signed-off-by: Javier Tia --- src/compiler/compiler.rs | 132 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index d3ceee25a..eb5ec5195 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -903,6 +903,14 @@ where .map(|output| path_transformer.as_dist_abs(&cwd.join(output.path))) .collect::>() .context("Failed to adapt an output path for distributed compile")?; + // The local paths every declared output must occupy once the compile + // finishes. Captured before `compilation` is moved into the packagers + // below so a remote compile that drops a declared output can be detected + // and salvaged (see the missing-output check after the run completes). + let expected_output_paths: Vec = compilation + .outputs() + .map(|output| cwd.join(output.path)) + .collect(); let (inputs_packager, toolchain_packager, outputs_rewriter) = compilation.into_dist_packagers(path_transformer)?; @@ -1055,6 +1063,21 @@ where ); } + // The remote compile returned exit 0 but may have dropped a declared + // output: glibc's ldconfig.o/sprof.o omit their `.o.dt` dep file, which + // the build-server does not return. Zipping the outputs later would then + // fail fatally with no recourse. Treat a missing declared output as a + // distribution artifact and fall back to a local recompile via the + // or_else below, which reproduces the full output set. Compiles whose + // dist output set is complete are unaffected. + if let Some(missing) = expected_output_paths.iter().find(|p| !p.exists()) { + bail!( + "distributed compile on {} did not return expected output {}; recompiling locally", + server_id.addr(), + missing.display() + ); + } + info!( "[{}]: Distributed compilation finished on {}", out_pretty, @@ -3225,6 +3248,7 @@ LLVM version: 6.0", test_dist::ErrorAllocJobClient::new(), test_dist::ErrorSubmitToolchainClient::new(), test_dist::ErrorRunJobClient::new(), + test_dist::IncompleteOutputsClient::new(), ]; // Write a dummy input file so the preprocessor cache mode can work std::fs::write(f.tempdir.path().join("foo.c"), "whatever").unwrap(); @@ -3693,4 +3717,112 @@ mod test_dist { None } } + + /// A dist client whose remote compile succeeds (exit 0) but whose returned + /// output set drops a declared output - the glibc `.o.dt` failure mode, + /// where the build-server runs the compile fine yet does not return every + /// declared output. The client must detect the missing output and fall back + /// to a local recompile rather than failing the build. + pub struct IncompleteOutputsClient { + has_started: AtomicBool, + tc: Toolchain, + output: ProcessOutput, + } + + impl IncompleteOutputsClient { + #[allow(clippy::new_ret_no_self)] + pub fn new() -> Arc { + Arc::new(Self { + has_started: AtomicBool::default(), + tc: Toolchain { + archive_id: "somearchiveid".to_owned(), + }, + output: ProcessOutput::fake_output(0, vec![], vec![]), + }) + } + } + + #[async_trait] + impl dist::Client for IncompleteOutputsClient { + async fn do_alloc_job(&self, tc: Toolchain) -> Result { + assert!( + !self + .has_started + .swap(true, std::sync::atomic::Ordering::AcqRel) + ); + assert_eq!(self.tc, tc); + + Ok(AllocJobResult::Success { + job_alloc: JobAlloc { + auth: "abcd".to_owned(), + job_id: JobId(0), + server_id: ServerId::new(([0, 0, 0, 0], 1).into()), + }, + need_toolchain: true, + }) + } + async fn do_get_status(&self) -> Result { + unreachable!("fn do_get_status is not used for this test. qed") + } + async fn do_submit_toolchain( + &self, + job_alloc: JobAlloc, + tc: Toolchain, + ) -> Result { + assert_eq!(job_alloc.job_id, JobId(0)); + assert_eq!(self.tc, tc); + + Ok(SubmitToolchainResult::Success) + } + async fn do_run_job( + &self, + job_alloc: JobAlloc, + command: CompileCommand, + outputs: Vec, + inputs_packager: Box, + ) -> Result<(RunJobResult, PathTransformer)> { + assert_eq!(job_alloc.job_id, JobId(0)); + assert_eq!(command.executable, "/overridden/compiler"); + + let mut inputs = vec![]; + let path_transformer = inputs_packager.write_inputs(&mut inputs).unwrap(); + // Drop one declared output to mimic a build-server that returned a + // successful compile with an incomplete output set. + let mut outputs = outputs; + outputs.pop(); + let outputs = outputs + .into_iter() + .map(|name| { + let data = format!("some data in {}", name); + let data = OutputData::try_from_reader(data.as_bytes()).unwrap(); + (name, data) + }) + .collect(); + let result = RunJobResult::Complete(JobComplete { + output: self.output.clone(), + outputs, + }); + Ok((result, path_transformer)) + } + async fn put_toolchain( + &self, + _: PathBuf, + _: String, + _: Box, + ) -> Result<(Toolchain, Option<(String, PathBuf)>)> { + Ok(( + self.tc.clone(), + Some(( + "/overridden/compiler".to_owned(), + PathBuf::from("somearchiveid"), + )), + )) + } + fn rewrite_includes_only(&self) -> bool { + false + } + fn get_custom_toolchain(&self, _exe: &Path) -> Option { + None + } + } } From 42b5bb032d006b029e3875ca4391ae6994c6805f Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 25 Jun 2026 11:46:23 -0600 Subject: [PATCH 07/43] compiler: concatenate strings without relying on deref coercion The lib test target links the thirtyfour dev-dependency, whose 0.36 release adds several `impl Add<_> for String`. With more than one `Add` impl for String in scope, the compiler no longer performs the &String -> &str and &Cow -> &str deref coercion that `s + &count` and `s + &p.to_string_lossy()` relied on, so `cargo test` fails to compile the lib target with E0277 in files unrelated to the change at hand. Spell the right-hand sides as &str explicitly (count.as_str(), to_string_lossy().as_ref()). The result is identical but no longer depends on coercion, so the test target builds regardless of which extra Add impls a dev-dependency happens to introduce. Signed-off-by: Javier Tia --- src/compiler/nvcc.rs | 2 +- src/compiler/rust.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/nvcc.rs b/src/compiler/nvcc.rs index e6f72a7bf..8ec211cb7 100644 --- a/src/compiler/nvcc.rs +++ b/src/compiler/nvcc.rs @@ -1183,7 +1183,7 @@ fn remap_generated_filenames( // Don't use the count as the first character of the file name, because the file name // may be used as an identifier (via the __FILE__ macro) and identifiers with leading // digits are not valid in C/C++, i.e. `x_0.cudafe1.cpp` instead of `0.cudafe1.cpp`. - .join("x_".to_owned() + &count + extension) + .join("x_".to_owned() + count.as_str() + extension) .to_string_lossy() .to_string() }) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index d6c2c7192..3382616a4 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -2262,7 +2262,7 @@ impl OutputsRewriter for RustOutputsRewriter { if let Some(dep_info) = self.dep_info { let extra_input_str = extra_inputs .iter() - .fold(String::new(), |s, p| s + " " + &p.to_string_lossy()); + .fold(String::new(), |s, p| s + " " + p.to_string_lossy().as_ref()); for dep_info_local_path in output_paths { trace!("Comparing with {}", dep_info_local_path.display()); if dep_info == *dep_info_local_path { From 911d48c36b19a4e20f4ca68124905c9eead32589 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 25 Jun 2026 11:46:13 -0600 Subject: [PATCH 08/43] compiler: package the compile task's toolchain, not the daemon's When sccache packages a C/C++ toolchain for distributed compilation it asks the compiler for its sub-tools with -print-prog-name=as (and objcopy, cc1, ...). A gcc that cannot find a bundled tool returns the bare name `as`, which write_pkg resolved with which::which against the sccache daemon's PATH. For an OpenEmbedded native or cross build the daemon's PATH points at the build host, so the packaged assembler was the host's /usr/bin/as rather than the recipe's binutils -- a silent wrong-toolchain hazard that is the reason native, cross, and crosssdk classes have to be excluded from distribution. Thread the compile task's environment into CToolchainPackager and use it both when invoking the compiler for -print-*-name and when resolving a bare program name (which_in against the task's PATH instead of which::which against the daemon's). The dist-compile path supplies the real task env via into_dist_packagers; the standalone --package-toolchain command has no compilation env and keeps its prior daemon-PATH behavior. Signed-off-by: Javier Tia --- src/compiler/c.rs | 77 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 70 insertions(+), 7 deletions(-) diff --git a/src/compiler/c.rs b/src/compiler/c.rs index 7cf84b505..e38676bfe 100644 --- a/src/compiler/c.rs +++ b/src/compiler/c.rs @@ -307,6 +307,11 @@ impl Compiler for CCompiler { Box::new(CToolchainPackager { executable: self.executable.clone(), kind: self.compiler.kind(), + // `CCompiler` carries no compile environment; the standalone + // `--package-toolchain` path keeps resolving against the daemon's + // PATH. The dist-compile path threads the task env via + // `into_dist_packagers`. + env_vars: Vec::new(), }) } fn parse_arguments( @@ -1217,6 +1222,7 @@ impl Compilation for CCompilation preprocessed_input, executable, compiler, + env_vars, .. } = *self; trace!("Dist inputs: {:?}", parsed_args.input); @@ -1232,6 +1238,7 @@ impl Compilation for CCompilation let toolchain_packager = Box::new(CToolchainPackager { executable, kind: compiler.kind(), + env_vars, }); let outputs_rewriter = Box::new(NoopOutputsRewriter); Ok((inputs_packager, toolchain_packager, outputs_rewriter)) @@ -1323,11 +1330,29 @@ impl pkg::InputsPackager for CInputsPackager { } } +/// Resolve a program path reported by `-print-prog-name`/`-print-file-name`. +/// Absolute paths are returned unchanged; a bare program name is resolved +/// against `path_env` (the compile task's PATH) when supplied, so an OE native +/// or cross gcc that reports a bare `as` packages the recipe's binutils rather +/// than the build host's. Falls back to the daemon's PATH when no compile PATH +/// is available. +#[cfg(feature = "dist-client")] +fn resolve_prog_in_path(raw: PathBuf, path_env: Option<&OsStr>) -> Option { + if raw.is_absolute() { + Some(raw) + } else if let Some(path) = path_env { + which::which_in(&raw, Some(path), std::env::current_dir().ok()?).ok() + } else { + which::which(raw).ok() + } +} + #[cfg(feature = "dist-client")] #[allow(unused)] struct CToolchainPackager { executable: PathBuf, kind: CCompilerKind, + env_vars: Vec<(OsString, OsString)>, } #[cfg(feature = "dist-client")] @@ -1344,11 +1369,21 @@ impl pkg::ToolchainPackager for CToolchainPackager { package_builder.add_common()?; package_builder.add_executable_and_deps(self.executable.clone())?; + // PATH from the compile task's environment. Resolving sub-tools against + // it (rather than the daemon's PATH) is what keeps an OE native or cross + // toolchain from packaging the build host's assembler. + let path_env = self + .env_vars + .iter() + .find(|(k, _)| k == OsStr::new("PATH")) + .map(|(_, v)| v.clone()); + // Helper to use -print-file-name and -print-prog-name to look up // files by path. let named_file = |kind: &str, name: &str| -> Option { let mut output = process::Command::new(&self.executable) .arg(format!("-print-{}-name={}", kind, name)) + .envs(self.env_vars.iter().cloned()) .output() .ok()?; debug!( @@ -1368,14 +1403,11 @@ impl pkg::ToolchainPackager for CToolchainPackager { output.stdout.pop(); } - // Create our PathBuf from the raw bytes. Assume that relative - // paths can be found via PATH. + // Create our PathBuf from the raw bytes. Relative paths are + // resolved against the compile task's PATH, falling back to the + // daemon's PATH when none was captured. let path: PathBuf = OsString::from_vec(output.stdout).into(); - if path.is_absolute() { - Some(path) - } else { - which::which(path).ok() - } + resolve_prog_in_path(path, path_env.as_deref()) }; // Helper to add a named file/program by to the package. @@ -1597,6 +1629,37 @@ mod test { use super::*; + #[test] + #[cfg(feature = "dist-client")] + fn test_resolve_prog_in_path_honors_supplied_path() { + use crate::test::utils::mk_bin; + let dir = tempfile::Builder::new() + .prefix("sccache_tc") + .tempdir() + .unwrap(); + let prog = mk_bin(dir.path(), "sccache-fake-as").unwrap(); + let path_env = std::env::join_paths([dir.path()]).unwrap(); + + // A bare program name resolves against the supplied compile PATH, not + // the daemon's PATH. This is the toolchain-packaging bug: an OE native + // or cross gcc reports a bare `as`, which must resolve to the recipe's + // binutils on the compile PATH, not the build host's /usr/bin/as. + let resolved = + resolve_prog_in_path(PathBuf::from("sccache-fake-as"), Some(path_env.as_os_str())); + assert_eq!( + resolved.map(|p| p.canonicalize().unwrap()), + Some(prog.clone()) + ); + + // Without the compile PATH, the daemon PATH cannot find this program. + let unresolved = resolve_prog_in_path(PathBuf::from("sccache-fake-as"), None); + assert_eq!(unresolved, None); + + // An absolute path passes through untouched. + let absolute = resolve_prog_in_path(prog.clone(), Some(path_env.as_os_str())); + assert_eq!(absolute, Some(prog)); + } + #[test] fn test_same_content() { let args = ovec!["a", "b", "c"]; From 8b1062c0f7bf0ecc06d0b4706bea94874baf2317 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 25 Jun 2026 11:51:30 -0600 Subject: [PATCH 09/43] dist: report a per-server breakdown in scheduler status SchedulerStatusResult only carried cluster aggregates (num_servers, num_cpus, in_progress), so a client could see total capacity but not how it was distributed: a two-server cluster reporting 64 cpus gave no way to tell whether one node carried every job or the load was balanced. Add a `servers` array of {id, num_cpus, in_progress}, populated from the scheduler's live server map in handle_status. The bakar cluster-info preflight already reads `servers` defensively via .get(), so this fills in the per-node detail it renders while existing aggregate consumers stay unaffected. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 12 ++++++++-- src/dist/mod.rs | 12 ++++++++++ src/dist/test.rs | 43 ++++++++++++++++++++++++++++++++++++ 3 files changed, 65 insertions(+), 2 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index eb85d3125..35c720fe4 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -11,8 +11,8 @@ use sccache::dist::{ self, AllocJobResult, AssignJobResult, BuilderIncoming, CompileCommand, HeartbeatServerResult, InputsReader, JobAlloc, JobAuthorizer, JobComplete, JobId, JobState, RunJobResult, SchedulerIncoming, SchedulerOutgoing, SchedulerStatusResult, ServerId, ServerIncoming, - ServerNonce, ServerOutgoing, SubmitToolchainResult, TcCache, Toolchain, ToolchainReader, - UpdateJobStateResult, + ServerNonce, ServerOutgoing, ServerStatusResult, SubmitToolchainResult, TcCache, Toolchain, + ToolchainReader, UpdateJobStateResult, }; use sccache::util::BASE64_URL_SAFE_ENGINE; use sccache::util::daemonize; @@ -746,6 +746,14 @@ impl SchedulerIncoming for Scheduler { num_servers: servers.len(), num_cpus: servers.values().map(|v| v.num_cpus).sum(), in_progress: jobs.len(), + servers: servers + .iter() + .map(|(server_id, details)| ServerStatusResult { + id: server_id.addr().to_string(), + num_cpus: details.num_cpus, + in_progress: details.jobs_assigned.len(), + }) + .collect(), }) } } diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 6bc1024aa..192c6b2f8 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -587,6 +587,18 @@ pub struct SchedulerStatusResult { pub num_servers: usize, pub num_cpus: usize, pub in_progress: usize, + /// Per-server breakdown so a client can see how capacity and load are + /// distributed, not just the cluster aggregate. + pub servers: Vec, +} + +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct ServerStatusResult { + /// The server's socket address, as the scheduler knows it. + pub id: String, + pub num_cpus: usize, + pub in_progress: usize, } // SubmitToolchain diff --git a/src/dist/test.rs b/src/dist/test.rs index 8b1378917..3c158c0c7 100644 --- a/src/dist/test.rs +++ b/src/dist/test.rs @@ -1 +1,44 @@ +use super::*; +/// The JSON shape emitted by `sccache --dist-status` is a contract consumed by +/// the bakar cluster-info preflight (`_parse_cluster_status`): a top-level +/// `servers` array of `{id, num_cpus, in_progress}` objects alongside the +/// aggregate counts. +#[test] +fn scheduler_status_serializes_per_server_breakdown() { + let status = SchedulerStatusResult { + num_servers: 2, + num_cpus: 64, + in_progress: 3, + servers: vec![ + ServerStatusResult { + id: "10.42.0.1:10501".to_string(), + num_cpus: 32, + in_progress: 2, + }, + ServerStatusResult { + id: "10.42.0.2:10501".to_string(), + num_cpus: 32, + in_progress: 1, + }, + ], + }; + + let json = serde_json::to_value(&status).unwrap(); + assert_eq!(json["num_servers"], 2); + assert_eq!(json["num_cpus"], 64); + assert_eq!(json["in_progress"], 3); + let servers = json["servers"].as_array().unwrap(); + assert_eq!(servers.len(), 2); + assert_eq!(servers[0]["id"], "10.42.0.1:10501"); + assert_eq!(servers[0]["num_cpus"], 32); + assert_eq!(servers[0]["in_progress"], 2); + assert_eq!(servers[1]["id"], "10.42.0.2:10501"); + assert_eq!(servers[1]["in_progress"], 1); + + // The aggregate in_progress is independent of the per-server sum: a job + // can be queued at the scheduler before assignment to any server. + let round: SchedulerStatusResult = serde_json::from_value(json).unwrap(); + assert_eq!(round.servers.len(), 2); + assert_eq!(round.servers[0].id, "10.42.0.1:10501"); +} From 277813158c2aec381f0d45e85677ebea93ca9551 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 25 Jun 2026 17:01:38 -0600 Subject: [PATCH 10/43] logging: keep wrapper log output off the compiler's stderr When sccache runs as a compiler wrapper (`sccache ...`) it shares stderr with the wrapped compiler. A build's feature probes treat any unexpected stderr as a compiler failure: libtool's -fPIC check (_LT_COMPILER_OPTION) sets pic_flag="" whenever the probe compile emits stderr, even on exit 0. With SCCACHE_LOG set, the client's own env_logger records ("Attempting to read config file", "Server sent CompileStarted") landed on that shared stderr, so an autotools package configured with pic_flag="" and produced non-PIC objects that failed to link into a shared library (relocation R_X86_64_32 ... recompile with -fPIC). It looked concurrency-dependent because the client's startup logging varies with server-connection timing, so the probe stderr only sometimes diverged from libtool's expected boilerplate. Initialize logging after parsing the command so the compile-wrapper case can choose its target: route sccache's own records to SCCACHE_ERROR_LOG when set, keep stderr only when it is an interactive terminal (so `SCCACHE_LOG=debug sccache gcc ...` at a prompt still shows logs), and discard them when stderr is captured by a build. The wrapped compiler's forwarded stderr is untouched. Signed-off-by: Javier Tia --- src/lib.rs | 37 ++++++++++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d5aa6a603..f6084803e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -62,8 +62,6 @@ pub const VERSION: &str = env!("CARGO_PKG_VERSION"); pub const LOGGING_ENV: &str = "SCCACHE_LOG"; pub fn main() { - init_logging(); - let command = match cmdline::try_parse() { Ok(cmd) => cmd, Err(e) => match e.downcast::() { @@ -79,6 +77,10 @@ pub fn main() { }, }; + // Logging is initialized after parsing so the compile-wrapper case can keep + // sccache's own log records off the wrapped compiler's stderr. + init_logging(&command); + std::process::exit(match commands::run_command(command) { Ok(s) => s, Err(e) => { @@ -91,7 +93,7 @@ pub fn main() { }); } -fn init_logging() { +fn init_logging(command: &cmdline::Command) { if env::var(LOGGING_ENV).is_ok() { let mut builder = env_logger::Builder::from_env(LOGGING_ENV); @@ -100,6 +102,35 @@ fn init_logging() { builder.format_timestamp_millis(); } + // When sccache runs as a compiler wrapper (`sccache ...`) it + // shares stderr with the wrapped compiler. Build tools treat any + // unexpected stderr on a feature probe as a compiler failure: libtool's + // `-fPIC` check sets `pic_flag=""` on non-empty stderr, producing non-PIC + // objects that then fail to link into a shared library. sccache's own log + // records must never reach that stderr. Send them to SCCACHE_ERROR_LOG + // when set; otherwise keep stderr only for an interactive terminal (so + // `SCCACHE_LOG=debug sccache gcc ...` at a prompt still shows logs) and + // discard them when stderr is captured by a build. + if matches!(command, cmdline::Command::Compile { .. }) { + use std::io::IsTerminal; + let target: Option> = + match env::var("SCCACHE_ERROR_LOG") { + Ok(path) if !path.is_empty() => Some( + std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&path) + .map(|f| Box::new(f) as Box) + .unwrap_or_else(|_| Box::new(std::io::sink())), + ), + _ if !std::io::stderr().is_terminal() => Some(Box::new(std::io::sink())), + _ => None, + }; + if let Some(target) = target { + builder.target(env_logger::Target::Pipe(target)); + } + } + match builder.try_init() { Ok(_) => (), Err(e) => panic!("Failed to initialize logging: {:?}", e), From 6c3b3730ce577e501ec1e5e0557d1c6bc462395a Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 25 Jun 2026 17:09:18 -0600 Subject: [PATCH 11/43] tests: match the scheduler-status servers field in dist harness e7df1d04 added the per-server `servers` array to SchedulerStatusResult but did not update the dist/system integration harness, whose `matches!` patterns enumerate every field explicitly. The test targets then failed to compile (E0027: pattern does not mention field `servers`), so `cargo build --all-targets` and the clippy pre-commit hook broke on a pre-existing, unrelated change. Add `servers: _` to both startup-wait patterns. Test-only; no effect on the binary. Signed-off-by: Javier Tia --- tests/harness/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/harness/mod.rs b/tests/harness/mod.rs index d04def707..35d8c48bf 100644 --- a/tests/harness/mod.rs +++ b/tests/harness/mod.rs @@ -366,7 +366,8 @@ impl DistSystem { SchedulerStatusResult { num_servers: 0, num_cpus: _, - in_progress: 0 + in_progress: 0, + servers: _ } ) { Ok(()) @@ -540,7 +541,8 @@ impl DistSystem { SchedulerStatusResult { num_servers: 1, num_cpus: _, - in_progress: 0 + in_progress: 0, + servers: _ } ) { Ok(()) From 074c6382aeb03be4b8d0d9ec09ca6920463bb89c Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 26 Jun 2026 10:49:56 -0600 Subject: [PATCH 12/43] compiler/gcc: never distribute precompiled-header generation A distributed PCH build produced a .gch that broke every consumer of the header. erlang's BEAM JIT (asmjit) failed do_compile with "missing binary operator before token (" at libstdc++'s `#elif ... && __has_builtin(...)` once the asmjit.hpp.gch built under sccache-dist was force-included: the .gch had been compiled on a remote builder from -fdirectives-only preprocessed source, where __has_builtin evaluates differently under -fpreprocessed than in a native build, so the cached macro state diverged and poisoned the headers parsed after the PCH was restored. A locally built .gch (original source, integrated cpp) compiles the same units cleanly. Treat a header-language input as precompiled-header generation and force it local (dist_command = None), alongside the existing -v/--verbose and ClangCUDA local-only cases. distcc refuses PCH for the same reason. The PCH is one cheap compile per recipe and its many consumers still distribute, so the throughput cost is nil while correctness is restored for every PCH-using recipe, not just erlang. --- src/compiler/gcc.rs | 76 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index e6ed63c7e..a455020c4 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -1008,7 +1008,16 @@ where // output is parsed by tools like CMake and must reflect the local toolchain // 2. ClangCUDA cannot be dist-compiled because Clang has separate host and // device preprocessor outputs and cannot compile preprocessed CUDA files. - let dist_command = if has_verbose_flag || parsed_args.language == Language::Cuda { + // 3. Precompiled-header generation (the input is itself a header, e.g. + // `gcc -c asmjit.hpp -o asmjit.hpp.gch`) must run locally. A .gch built + // from preprocessed source on a remote builder captures a macro state + // (notably __has_builtin, evaluated differently under -fpreprocessed) + // that diverges from a native build and then breaks every consumer that + // -includes the header. distcc refuses PCH for the same reason. + let dist_command = if has_verbose_flag + || parsed_args.language == Language::Cuda + || parsed_args.language.is_c_like_header() + { None } else { (|| { @@ -2671,6 +2680,71 @@ mod test { assert_eq!(0, creator.lock().unwrap().children.len()); } + #[test] + fn test_compile_pch_generation_never_dist() { + // Compiling a header (precompiled-header generation, e.g. + // `gcc -c asmjit.hpp -o asmjit.hpp.gch`) must never be distributed: a + // .gch built from preprocessed source / a remote toolchain captures a + // macro state (notably __has_builtin) that differs from a local build + // and then poisons every consumer that -includes the header. distcc + // refuses PCH for the same reason; we build it locally. + let creator = new_creator(); + let f = TestFixture::new(); + let parsed_args = ParsedArguments { + input: "asmjit.hpp".into(), + double_dash_input: false, + language: Language::CxxHeader, + compilation_flag: "-c".into(), + depfile: None, + outputs: vec![( + "obj", + ArtifactDescriptor { + path: "asmjit.hpp.gch".into(), + optional: false, + }, + )] + .into_iter() + .collect(), + dependency_args: vec![], + preprocessor_args: vec![], + common_args: vec![], + arch_args: vec![], + unhashed_args: vec![], + extra_dist_files: vec![], + extra_hash_files: vec![], + msvc_show_includes: false, + profile_generate: false, + color_mode: ColorMode::Auto, + suppress_rewrite_includes_only: false, + too_hard_for_preprocessor_cache_mode: None, + }; + let runtime = single_threaded_runtime(); + let storage = MockStorage::new(None, false); + let storage: std::sync::Arc = std::sync::Arc::new(storage); + let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); + let compiler = &f.bins[0]; + // Compiler invocation. + next_command(&creator, Ok(MockChild::new(exit_status(0), "", ""))); + let mut path_transformer = dist::PathTransformer::new(); + let (command, dist_command, cacheable) = generate_compile_commands( + &mut path_transformer, + compiler, + &parsed_args, + f.tempdir.path(), + &[], + CCompilerKind::Gcc, + // rewrite_includes_only=true: dist is enabled, yet a header still + // must not produce a dist command. + true, + language_to_gcc_arg, + ) + .unwrap(); + assert!(dist_command.is_none()); + let _ = command.execute(&service, &creator).wait(); + assert_eq!(Cacheable::Yes, cacheable); + assert_eq!(0, creator.lock().unwrap().children.len()); + } + #[test] fn test_compile_double_dash_input() { let args = stringvec!["-c", "-o", "foo.o", "--", "foo.c"]; From 4c7faa541ef4137abbd4d0b4925e7aa92b13da79 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 26 Jun 2026 12:07:57 -0600 Subject: [PATCH 13/43] dist/scheduler: drop a stopped server promptly A build server that stops is only removed once prune_servers sees its last_seen exceed the 90s heartbeat timeout, and prune runs only on the heartbeat and status paths. So --dist-status reports a stopped node as live for up to two minutes. Worse, handle_alloc_job picks servers purely by load and never checks last_seen, so it keeps dispatching compiles to a node that is gone; those fail and fall back to a local recompile, which silently skews dist timing measurements. Add a graceful deregister: the server installs a SIGTERM/SIGINT handler that flags a watcher thread to POST /api/v1/scheduler/deregister_server before exit, so a clean systemctl stop removes the node from the scheduler at once. The handler itself only does an async-signal-safe atomic store; the HTTP call runs on the watcher thread. The 90s timeout stays as the backstop for an ungraceful death (crash / power loss). Independently, skip any server stale beyond the heartbeat timeout in handle_alloc_job, so a job is never assigned to a node that has not been pruned yet - covering the crash case a deregister cannot. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 164 +++++++++++++++++++++++++++++++++-- src/dist/http.rs | 70 ++++++++++++++- src/dist/mod.rs | 13 +++ 3 files changed, 239 insertions(+), 8 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 35c720fe4..49849b5ce 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -8,11 +8,11 @@ use sccache::config::{ INSECURE_DIST_CLIENT_TOKEN, scheduler as scheduler_config, server as server_config, }; use sccache::dist::{ - self, AllocJobResult, AssignJobResult, BuilderIncoming, CompileCommand, HeartbeatServerResult, - InputsReader, JobAlloc, JobAuthorizer, JobComplete, JobId, JobState, RunJobResult, - SchedulerIncoming, SchedulerOutgoing, SchedulerStatusResult, ServerId, ServerIncoming, - ServerNonce, ServerOutgoing, ServerStatusResult, SubmitToolchainResult, TcCache, Toolchain, - ToolchainReader, UpdateJobStateResult, + self, AllocJobResult, AssignJobResult, BuilderIncoming, CompileCommand, DeregisterServerResult, + HeartbeatServerResult, InputsReader, JobAlloc, JobAuthorizer, JobComplete, JobId, JobState, + RunJobResult, SchedulerIncoming, SchedulerOutgoing, SchedulerStatusResult, ServerId, + ServerIncoming, ServerNonce, ServerOutgoing, ServerStatusResult, SubmitToolchainResult, + TcCache, Toolchain, ToolchainReader, UpdateJobStateResult, }; use sccache::util::BASE64_URL_SAFE_ENGINE; use sccache::util::daemonize; @@ -449,6 +449,14 @@ impl SchedulerIncoming for Scheduler { let mut best_load: f64 = MAX_PER_CORE_LOAD; let now = Instant::now(); for (&server_id, details) in servers.iter_mut() { + // A server silent past the heartbeat timeout is dead but + // is only removed by prune_servers (heartbeat / status + // paths), not here. Skip it so a job is never dispatched + // to a node that is gone - which would fail and fall back + // local, silently skewing dist measurements. + if now.duration_since(details.last_seen) > dist::http::HEARTBEAT_TIMEOUT { + continue; + } let load = load_weight(details.jobs_assigned.len(), details.num_cpus); if let Some(last_error) = details.last_error { @@ -682,6 +690,26 @@ impl SchedulerIncoming for Scheduler { Ok(HeartbeatServerResult { is_new: true }) } + fn handle_deregister_server(&self, server_id: ServerId) -> Result { + // LOCKS (jobs before servers, matching the alphabetical lock order + // used across the scheduler to avoid deadlock). + let mut jobs = self.jobs.lock().unwrap(); + let mut servers = self.servers.lock().unwrap(); + + if let Some(details) = servers.remove(&server_id) { + info!( + "Server {} deregistered on graceful shutdown", + server_id.addr() + ); + // Drop any jobs the departing server still held, as prune_servers + // does, so the scheduler does not track work no node will finish. + for job_id in details.jobs_assigned { + jobs.remove(&job_id); + } + } + Ok(DeregisterServerResult { success: true }) + } + fn handle_update_job_state( &self, job_id: JobId, @@ -861,3 +889,129 @@ impl ServerIncoming for Server { res } } + +#[cfg(test)] +mod scheduler_tests { + use super::*; + use std::net::SocketAddr; + + struct NoopAuthorizer; + impl JobAuthorizer for NoopAuthorizer { + fn generate_token(&self, _job_id: JobId) -> Result { + Ok(String::new()) + } + fn verify_token(&self, _job_id: JobId, _token: &str) -> Result<()> { + Ok(()) + } + } + + struct PanicOnAssign; + impl SchedulerOutgoing for PanicOnAssign { + fn do_assign_job( + &self, + _server_id: ServerId, + _job_id: JobId, + _tc: Toolchain, + _auth: String, + ) -> Result { + panic!("a job must never be assigned to a stale server"); + } + } + + struct AssignOk; + impl SchedulerOutgoing for AssignOk { + fn do_assign_job( + &self, + _server_id: ServerId, + _job_id: JobId, + _tc: Toolchain, + _auth: String, + ) -> Result { + Ok(AssignJobResult { + state: JobState::Ready, + need_toolchain: false, + }) + } + } + + fn insert_server(scheduler: &Scheduler, addr: &str, last_seen: Instant) { + let server_id = ServerId::new(addr.parse::().unwrap()); + let mut servers = scheduler.servers.lock().unwrap(); + servers.insert( + server_id, + ServerDetails { + last_seen, + last_error: None, + jobs_assigned: HashSet::new(), + jobs_unclaimed: HashMap::new(), + num_cpus: 32, + server_nonce: ServerNonce::new(), + job_authorizer: Box::new(NoopAuthorizer), + }, + ); + } + + fn a_toolchain() -> Toolchain { + Toolchain { + archive_id: "deadbeef".to_owned(), + } + } + + // A server silent past HEARTBEAT_TIMEOUT is dead but is only removed by + // prune_servers, which runs on the heartbeat and status paths - not on + // alloc_job. Until then it must never receive a job, or the scheduler + // dispatches to a node that is gone and the compile falls back local, + // silently skewing a dist measurement. + #[test] + fn test_alloc_job_skips_server_stale_beyond_heartbeat_timeout() { + let scheduler = Scheduler::new(); + let stale = Instant::now() - (dist::http::HEARTBEAT_TIMEOUT + Duration::from_secs(10)); + insert_server(&scheduler, "10.42.0.2:10501", stale); + + let res = scheduler + .handle_alloc_job(&PanicOnAssign, a_toolchain()) + .expect("handle_alloc_job should not error"); + + assert!( + matches!(res, AllocJobResult::Fail { .. }), + "a stale server must not be selected for assignment" + ); + } + + // Guard against over-filtering: a server seen within the timeout is alive + // and must remain a valid assignment target. + #[test] + fn test_alloc_job_uses_fresh_server() { + let scheduler = Scheduler::new(); + insert_server(&scheduler, "10.42.0.1:10501", Instant::now()); + + let res = scheduler + .handle_alloc_job(&AssignOk, a_toolchain()) + .expect("handle_alloc_job should not error"); + + assert!( + matches!(res, AllocJobResult::Success { .. }), + "a fresh server must be selected for assignment" + ); + } + + // A graceful deregister must drop the server at once, without waiting for + // the heartbeat timeout - even though its last_seen is current. + #[test] + fn test_deregister_server_removes_it_immediately() { + let scheduler = Scheduler::new(); + insert_server(&scheduler, "10.42.0.2:10501", Instant::now()); + + let server_id = ServerId::new("10.42.0.2:10501".parse::().unwrap()); + let res = scheduler + .handle_deregister_server(server_id) + .expect("handle_deregister_server should not error"); + assert!(res.success); + + let status = scheduler.handle_status().expect("status"); + assert_eq!( + status.num_servers, 0, + "a deregistered server must be gone immediately" + ); + } +} diff --git a/src/dist/http.rs b/src/dist/http.rs index 743a5ca69..4a55c52be 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -208,6 +208,11 @@ pub mod urls { .join("/api/v1/scheduler/status") .expect("failed to create alloc job url") } + pub fn scheduler_deregister_server(scheduler_url: &reqwest::Url) -> reqwest::Url { + scheduler_url + .join("/api/v1/scheduler/deregister_server") + .expect("failed to create deregister url") + } pub fn server_assign_job(server_id: ServerId, job_id: JobId) -> reqwest::Url { let url = format!( @@ -259,9 +264,10 @@ mod server { }; use super::urls; use crate::dist::{ - self, AllocJobResult, AssignJobResult, HeartbeatServerResult, InputsReader, JobAuthorizer, - JobId, JobState, RunJobResult, SchedulerStatusResult, ServerId, ServerNonce, - SubmitToolchainResult, Toolchain, ToolchainReader, UpdateJobStateResult, + self, AllocJobResult, AssignJobResult, DeregisterServerResult, HeartbeatServerResult, + InputsReader, JobAuthorizer, JobId, JobState, RunJobResult, SchedulerStatusResult, + ServerId, ServerNonce, SubmitToolchainResult, Toolchain, ToolchainReader, + UpdateJobStateResult, }; use crate::errors::*; @@ -833,6 +839,13 @@ mod server { let res: SchedulerStatusResult = try_or_500_log!(req_id, handler.handle_status()); prepare_response(request, &res) }, + (POST) (/api/v1/scheduler/deregister_server) => { + let server_id = check_server_auth_or_err!(request); + trace!("Req {}: deregister_server: {:?}", req_id, server_id); + + let res: DeregisterServerResult = try_or_500_log!(req_id, handler.handle_deregister_server(server_id)); + prepare_response(request, &res) + }, _ => { warn!("Unknown request {:?}", request); rouille::Response::empty_404() @@ -871,6 +884,24 @@ mod server { } } + static SHUTDOWN_REQUESTED: atomic::AtomicBool = atomic::AtomicBool::new(false); + + extern "C" fn handle_shutdown_signal(_sig: libc::c_int) { + // Async-signal-safe: only an atomic store. The deregister + exit run + // on a normal worker thread that polls this flag. + SHUTDOWN_REQUESTED.store(true, atomic::Ordering::SeqCst); + } + + fn install_shutdown_handler() { + // SAFETY: the handler performs only an async-signal-safe atomic store, + // which is sound to run from a signal handler. + let handler = handle_shutdown_signal as extern "C" fn(libc::c_int) as libc::sighandler_t; + unsafe { + libc::signal(libc::SIGTERM, handler); + libc::signal(libc::SIGINT, handler); + } + } + pub struct Server { bind_address: SocketAddr, scheduler_url: reqwest::Url, @@ -926,6 +957,39 @@ mod server { server_nonce, handler, } = self; + // Graceful shutdown: on SIGTERM/SIGINT, deregister from the + // scheduler before exiting so it drops us at once instead of + // waiting out the 90s heartbeat timeout. The timeout stays as the + // backstop for an ungraceful death (crash / power loss). + let deregister_url = urls::scheduler_deregister_server(&scheduler_url); + let deregister_auth = scheduler_auth.clone(); + install_shutdown_handler(); + thread::spawn(move || { + let client = reqwest::blocking::Client::builder() + .pool_max_idle_per_host(0) + .timeout(Duration::from_secs(5)) + .build() + .expect("deregister http client must build"); + loop { + if SHUTDOWN_REQUESTED.load(atomic::Ordering::SeqCst) { + info!("Shutdown signal received; deregistering from scheduler"); + let res: Result = bincode_req( + client + .post(deregister_url.clone()) + .bearer_auth(deregister_auth.clone()) + .bincode(&()) + .expect("failed to serialize deregister"), + ); + match res { + Ok(_) => info!("Deregistered from scheduler"), + Err(e) => error!("Failed to deregister from scheduler: {}", e), + } + std::process::exit(0); + } + thread::sleep(Duration::from_millis(200)); + } + }); + let heartbeat_req = HeartbeatServerHttpRequest { num_cpus: num_cpus(), jwt_key: jwt_key.clone(), diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 192c6b2f8..6d4021759 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -564,6 +564,14 @@ pub struct HeartbeatServerResult { pub is_new: bool, } +// DeregisterServer + +#[derive(Clone, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct DeregisterServerResult { + pub success: bool, +} + // RunJob #[derive(Clone, Serialize, Deserialize)] @@ -686,6 +694,11 @@ pub trait SchedulerIncoming: Send + Sync { job_authorizer: Box, ) -> ExtResult; // From Server + fn handle_deregister_server( + &self, + server_id: ServerId, + ) -> ExtResult; + // From Server fn handle_update_job_state( &self, job_id: JobId, From a1bcb0acad2444d6a165953acaf919da7bf116a7 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 26 Jun 2026 16:18:30 -0600 Subject: [PATCH 14/43] compiler/gcc: keep configure feature-probes local avocado-distro's do_configure phase ships ~900 autoconf conftest and CMake try_compile feature-probe compiles to the build-server. Each is a tiny translation unit and many are designed to fail (detecting an absent feature), so the server returns no object and the client falls back to a local recompile. Those round-trips contend with real compiles for the build-server slots and bury the dist_errors metric, all for a result identical to a sub-second local compile. Force configure feature-probes local (dist_command = None), alongside the existing verbose, ClangCUDA, and precompiled-header cases. Detect autoconf by the conftest. input name and CMake try_compile by the CMakeScratch/CMakeTmp/TryCompile-* scratch directory rather than the generated probe source names (CheckSymbolExists.c, src.c, ...), which are not stable across CMake versions. A false positive only makes a compile run locally, still cached, so the detection errs safe. Signed-off-by: Javier Tia --- src/compiler/gcc.rs | 126 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 126 insertions(+) diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index a455020c4..d91740178 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -927,6 +927,37 @@ where run_input_output(cmd, None).await } +/// Autoconf and CMake configure-time feature probes must run locally, not +/// distributed. autoconf compiles `conftest.`; CMake's `try_compile()` +/// compiles generated sources inside a `CMakeScratch`/`CMakeTmp`/`TryCompile-*` +/// scratch directory. These probes are tiny, run by the hundreds during +/// `do_configure`, and are frequently *designed* to fail (detecting the absence +/// of a feature). Distributing them ships a network round-trip per probe and, +/// for the intentionally-failing ones, a dropped-output fallback - hundreds of +/// wasted dispatches contending with real compiles for build-server slots, for +/// no benefit: a probe's pass/fail result is identical whether it compiles +/// locally or remotely, and a sub-second local compile beats the round-trip. +/// distcc/icecc tolerate this only because their dispatch is cheaper. Detection +/// keys on the autoconf input name and the CMake scratch directory rather than +/// the generated probe source names (CheckSymbolExists.c, src.c, ...), which are +/// not stable across CMake versions. A false positive is harmless: the compile +/// merely runs locally (still cached), exactly as it would for any non-eligible +/// recipe. +#[cfg(feature = "dist-client")] +fn is_configure_probe(input: &Path, cwd: &Path) -> bool { + if input + .file_stem() + .and_then(|stem| stem.to_str()) + .is_some_and(|stem| stem == "conftest") + { + return true; + } + cwd.components().any(|component| { + let part = component.as_os_str().to_str().unwrap_or_default(); + part == "CMakeScratch" || part == "CMakeTmp" || part.starts_with("TryCompile-") + }) +} + #[allow(clippy::too_many_arguments)] pub fn generate_compile_commands( path_transformer: &mut dist::PathTransformer, @@ -1014,9 +1045,16 @@ where // (notably __has_builtin, evaluated differently under -fpreprocessed) // that diverges from a native build and then breaks every consumer that // -includes the header. distcc refuses PCH for the same reason. + // 4. Autoconf/CMake configure feature-probes (conftest.c, CMake try_compile + // sources under a CMakeScratch/CMakeTmp/TryCompile-* dir) run locally: + // they are tiny, run by the hundreds during do_configure, and are often + // designed to fail, so distributing them only wastes build-server slots + // on round-trips whose result is identical to a local compile. See + // is_configure_probe. let dist_command = if has_verbose_flag || parsed_args.language == Language::Cuda || parsed_args.language.is_c_like_header() + || is_configure_probe(&parsed_args.input, cwd) { None } else { @@ -2745,6 +2783,94 @@ mod test { assert_eq!(0, creator.lock().unwrap().children.len()); } + #[test] + fn test_compile_conftest_never_dist() { + // Autoconf compiles `conftest.c` to probe for a feature; the probe is + // tiny and often designed to fail. It must run locally, never + // distributed, regardless of rewrite_includes_only (dist enabled). + let creator = new_creator(); + let f = TestFixture::new(); + let parsed_args = ParsedArguments { + input: "conftest.c".into(), + double_dash_input: false, + language: Language::C, + compilation_flag: "-c".into(), + depfile: None, + outputs: vec![( + "obj", + ArtifactDescriptor { + path: "conftest.o".into(), + optional: false, + }, + )] + .into_iter() + .collect(), + dependency_args: vec![], + preprocessor_args: vec![], + common_args: vec![], + arch_args: vec![], + unhashed_args: vec![], + extra_dist_files: vec![], + extra_hash_files: vec![], + msvc_show_includes: false, + profile_generate: false, + color_mode: ColorMode::Auto, + suppress_rewrite_includes_only: false, + too_hard_for_preprocessor_cache_mode: None, + }; + let runtime = single_threaded_runtime(); + let storage = MockStorage::new(None, false); + let storage: std::sync::Arc = std::sync::Arc::new(storage); + let service = server::SccacheService::mock_with_storage(storage, runtime.handle().clone()); + let compiler = &f.bins[0]; + next_command(&creator, Ok(MockChild::new(exit_status(0), "", ""))); + let mut path_transformer = dist::PathTransformer::new(); + let (command, dist_command, cacheable) = generate_compile_commands( + &mut path_transformer, + compiler, + &parsed_args, + f.tempdir.path(), + &[], + CCompilerKind::Gcc, + true, + language_to_gcc_arg, + ) + .unwrap(); + assert!(dist_command.is_none()); + let _ = command.execute(&service, &creator).wait(); + assert_eq!(Cacheable::Yes, cacheable); + assert_eq!(0, creator.lock().unwrap().children.len()); + } + + #[test] + #[cfg(feature = "dist-client")] + fn test_is_configure_probe() { + use std::path::Path; + // autoconf: conftest. in any directory. + assert!(is_configure_probe( + Path::new("conftest.c"), + Path::new("/build/foo/1.0/foo-1.0") + )); + assert!(is_configure_probe( + Path::new("conftest.cpp"), + Path::new("/build/foo/1.0/foo-1.0") + )); + // CMake try_compile: generated source inside a scratch directory. + assert!(is_configure_probe( + Path::new("CheckSymbolExists.c"), + Path::new("/build/foo/1.0/build/CMakeFiles/CMakeScratch/TryCompile-aB3xZ9") + )); + assert!(is_configure_probe( + Path::new("src.c"), + Path::new("/build/foo/1.0/build/CMakeTmp") + )); + // A real translation unit in a normal build directory must distribute. + assert!(!is_configure_probe( + Path::new("list.c"), + Path::new("/build/foo/1.0/foo-1.0/src") + )); + } + #[test] fn test_compile_double_dash_input() { let args = stringvec!["-c", "-o", "foo.o", "--", "foo.c"]; From dc5c9a382d31363929f6eec20c8b25dcb8f03f99 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Wed, 1 Jul 2026 12:15:51 -0600 Subject: [PATCH 15/43] dist/pkg: bundle the sysroot usr/lib for split-sysroot toolchains A relocated toolchain interpreter (the OE buildtools SDK loader) already triggers bundling of its own lib dir so libc/libm resolve inside the build sandbox. But split-sysroot toolchains keep the compiler's dependency libraries -- libmpc/libmpfr/libgmp for cc1, libbfd/libsframe/libopcodes for as -- in /usr/lib, not beside the loader. ldd runs against the host loader, so it resolves those deps to host /usr/lib paths (or reports them "not found"), and they never land where the sandbox loader searches. cc1/as then fail with "error while loading shared libraries" and the distributed compile returns exit 1, forcing a local fallback -- every native/cross/crosssdk compile runs on the primary node while the secondaries sit idle. Bundle the shared libraries in /usr/lib (shallow, filtered to *.so*) alongside the interpreter lib dir so every NEEDED library resolves in-sandbox. Verified end to end: the OE buildtools gcc now distributes to the second node (dist_compiles, zero errors) where it previously fell back to local. Signed-off-by: Javier Tia --- src/dist/pkg.rs | 77 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/src/dist/pkg.rs b/src/dist/pkg.rs index e7cab535c..d8dce0d03 100644 --- a/src/dist/pkg.rs +++ b/src/dist/pkg.rs @@ -111,6 +111,19 @@ mod toolchain_imp { libdir.display() ) })?; + // Split-sysroot toolchains (e.g. the OE buildtools SDK) keep the + // loader plus libc/libm in /lib but the compiler's own + // dependency libraries (libmpc, libbfd, libsframe, ...) in + // /usr/lib. The relocated loader searches both, but ldd + // (run against the host) resolves those deps to host paths or + // reports them "not found", so they never land where the sandbox + // loader looks. Bundle the sysroot usr/lib shared libraries so + // every NEEDED lib resolves inside the build sandbox. + if let Some(usr_libdir) = sysroot_usr_libdir(&libdir) { + self.add_shared_libraries(&usr_libdir).with_context(|| { + format!("Failed to bundle sysroot usr/lib {}", usr_libdir.display()) + })?; + } } let mut remaining = vec![executable]; while let Some(obj_path) = remaining.pop() { @@ -192,6 +205,43 @@ mod toolchain_imp { Ok(()) } + /// Add the shared libraries directly in `dir_path` to the package. + /// + /// Unlike `add_dir_contents` this is shallow and filtered to files + /// whose name looks like an ELF shared object (`*.so`, `*.so.N...`), so + /// bundling a large sysroot `usr/lib` pulls in only the runtime + /// libraries the relocated loader resolves -- not headers, static + /// archives, or subdirectories. Missing directories are a no-op. + pub fn add_shared_libraries(&mut self, dir_path: &Path) -> Result<()> { + if !dir_path.is_dir() { + return Ok(()); + } + for entry in fs::read_dir(dir_path)? { + let entry = entry?; + let name = entry.file_name(); + let name = name.to_string_lossy(); + if !(name.ends_with(".so") || name.contains(".so.")) { + continue; + } + let file_type = entry.file_type()?; + if file_type.is_symlink() { + // A symlink pointing at a regular file; add_file resolves it + // and tarify_path records the link. + if !fs::metadata(entry.path()) + .map(|m| m.is_file()) + .unwrap_or(false) + { + continue; + } + } else if !file_type.is_file() { + continue; + } + trace!("shared lib add_file {}", entry.path().display()); + self.add_file(entry.path())?; + } + Ok(()) + } + pub fn into_compressed_tar(self, writer: W) -> Result<()> { use gzp::{ deflate::Gzip, @@ -408,6 +458,33 @@ mod toolchain_imp { interp.parent().map(Path::to_path_buf) } + /// Given a relocated interpreter's lib dir (`/lib`), return the + /// sibling `/usr/lib` when it exists. + /// + /// Split-sysroot toolchains (the OE buildtools SDK) keep the compiler's + /// dependency libraries there rather than beside the loader; uninative + /// sysroots have no `usr/lib`, so this returns `None` and leaves that path + /// untouched. + fn sysroot_usr_libdir(libdir: &Path) -> Option { + let usr_lib = libdir.parent()?.join("usr").join("lib"); + usr_lib.is_dir().then_some(usr_lib) + } + + #[test] + fn test_sysroot_usr_libdir() { + let tmp = tempfile::tempdir().unwrap(); + let libdir = tmp.path().join("sysroot").join("lib"); + std::fs::create_dir_all(&libdir).unwrap(); + + // A uninative sysroot has no usr/lib sibling: leave that path untouched. + assert_eq!(sysroot_usr_libdir(&libdir), None); + + // A split sysroot (OE buildtools SDK) has usr/lib: bundle it. + let usr_lib = tmp.path().join("sysroot").join("usr").join("lib"); + std::fs::create_dir_all(&usr_lib).unwrap(); + assert_eq!(sysroot_usr_libdir(&libdir), Some(usr_lib)); + } + #[test] fn test_ldd_parse() { let ubuntu_ls_output = "\tlinux-vdso.so.1 => (0x00007fffcfffe000) From aa2a4b75ecca5a2c164dbdc64aa50a5135fdcffb Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Wed, 1 Jul 2026 16:15:19 -0600 Subject: [PATCH 16/43] compiler/gcc: run /dev/null feature-probes locally OpenSSL probes assembler support by running the compiler on /dev/null: `gcc -Wa,--help -c -o null.o -x assembler /dev/null`, then greps the assembler's help output for --noexecstack. Under an sccache wrapper this parsed as a cacheable assembler compile, but -Wa,--help makes the assembler print its help and exit without producing an object. sccache then aborted trying to zip the missing output and swallowed the probe's stdout, so OpenSSL saw no --noexecstack and assembled every .s with an executable stack. The resulting libcrypto.so.3 was rejected by a hardened host kernel, breaking later recipes that load it (python3-native). Refuse to cache a compile whose input is /dev/null. Such an input is never a real translation unit, so a transparent local exec costs nothing and lets the probe's output reach the caller, matching ccache's long-standing refusal to cache /dev/null. Signed-off-by: Javier Tia --- src/compiler/gcc.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/compiler/gcc.rs b/src/compiler/gcc.rs index d91740178..af3d8ce35 100644 --- a/src/compiler/gcc.rs +++ b/src/compiler/gcc.rs @@ -654,6 +654,15 @@ where // We can't cache compilation without an input. None => cannot_cache!("no input file"), }; + // Compiler feature-probes compile /dev/null (e.g. OpenSSL's + // `-Wa,--help ... -x assembler /dev/null`, which asks the assembler to + // print its help). They never produce a real object, so caching aborts + // when the missing output can't be zipped and swallows the probe's stdout, + // making the caller mis-detect the feature. Refuse to cache so sccache runs + // the compiler locally and the probe's output reaches the caller. + if Path::new(&input) == Path::new("/dev/null") { + cannot_cache!("/dev/null input"); + } let language = match language { None => { let mut lang = Language::from_file_name(Path::new(&input)); @@ -1499,6 +1508,29 @@ mod test { assert!(preprocessor_args.is_empty()); } + #[test] + fn test_parse_arguments_dev_null_input_not_cached() { + // OpenSSL's assembler feature-probe compiles /dev/null: + // gcc -Wa,--help -c -o null.o -x assembler /dev/null + // -Wa,--help makes the assembler print its help and exit without an + // object, so caching aborts when the missing output can't be zipped and + // swallows the probe's stdout - the caller then mis-detects + // --noexecstack support and emits exec-stack objects. Run it locally. + let args = stringvec![ + "-Wa,--help", + "-c", + "-o", + "null.o", + "-x", + "assembler", + "/dev/null" + ]; + assert_eq!( + CompilerArguments::CannotCache("/dev/null input", None), + parse_arguments_(args, false) + ); + } + #[test] fn test_parse_arguments_depfile_for_preprocessed_c_clang() { let args = stringvec!["-c", "foo.i", "-o", "foo.o", "-MD", "-MF", "foo.d"]; From 3b0fcea603f498cb172d8e495c01726559e7a9fc Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 05:36:19 -0600 Subject: [PATCH 17/43] dist: instrument job scheduling and client dispatch for load diagnosis The 2-node cluster leaves both build servers idle for long stretches of a Yocto cold build, but the existing logs cannot distinguish the three candidate causes: job-supply starvation (bitbake rarely runs enough concurrent do_compile phases), the client-side preprocessing tax that skews load onto the colocated node, and per-job round-trip latency on short compiles. Choosing which rework to pursue needs per-job and per-server timing data, not guesses. Add info-level instrumentation on both halves of the dist path, routed to SCCACHE_ERROR_LOG so it stays off the compiler's stderr. The scheduler logs every candidate server's load alongside the allocation decision (and on the no-capacity path), plus a one-line in-progress snapshot per status poll, exposing any first-hashed-server tie bias and the gate-full frequency. The client times put_toolchain, alloc, submit, and run+fetch separately, tracks an in-flight compile counter via an RAII guard so the count stays correct across every future exit, and emits a per-job summary naming the server that ran it. One instrumented build then tells supply starvation (low in-flight while nodes idle) from round-trip latency (run+fetch dominating) from a preprocessing bottleneck. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 33 +++++++++++++++++++++ src/compiler/compiler.rs | 56 ++++++++++++++++++++++++++++++++---- 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 49849b5ce..0f6513f4f 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -443,6 +443,11 @@ impl SchedulerIncoming for Scheduler { // LOCKS let mut servers = self.servers.lock().unwrap(); + // R0 instrumentation: snapshot every candidate's load so the + // allocation decision logged below exposes the load distribution + // across nodes and any first-hashed-server tie bias. + let mut candidate_loads: Vec<(ServerId, f64, usize, usize)> = Vec::new(); + let res = { let mut best = None; let mut best_err = None; @@ -458,6 +463,12 @@ impl SchedulerIncoming for Scheduler { continue; } let load = load_weight(details.jobs_assigned.len(), details.num_cpus); + candidate_loads.push(( + server_id, + load, + details.jobs_assigned.len(), + details.num_cpus, + )); if let Some(last_error) = details.last_error { if load < MAX_PER_CORE_LOAD { @@ -517,6 +528,15 @@ impl SchedulerIncoming for Scheduler { "Job {} created and will be assigned to server {:?}", job_id, server_id ); + info!( + "dist-alloc: job {} -> {:?} (now {}/{} jobs, load {:.3}); candidates {:?}", + job_id, + server_id, + server_details.jobs_assigned.len(), + server_details.num_cpus, + load_weight(server_details.jobs_assigned.len(), server_details.num_cpus), + candidate_loads, + ); let auth = server_details .job_authorizer .generate_token(job_id) @@ -534,6 +554,7 @@ impl SchedulerIncoming for Scheduler { "Insufficient capacity across {} available servers", servers.len() ); + warn!("dist-alloc: {}; candidates {:?}", msg, candidate_loads); return Ok(AllocJobResult::Fail { msg }); } }; @@ -770,6 +791,18 @@ impl SchedulerIncoming for Scheduler { self.prune_servers(&mut servers, &mut jobs); + // R0 instrumentation: one-line status snapshot per poll so the + // in-progress cadence across nodes is recoverable from the log. + let status_snapshot: Vec<(String, usize, usize)> = servers + .iter() + .map(|(id, d)| (id.addr().to_string(), d.jobs_assigned.len(), d.num_cpus)) + .collect(); + info!( + "dist-status poll: {} in-progress jobs; servers {:?}", + jobs.len(), + status_snapshot + ); + Ok(SchedulerStatusResult { num_servers: servers.len(), num_cpus: servers.values().map(|v| v.num_cpus).sum(), diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index eb5ec5195..0f013d60d 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -850,6 +850,34 @@ where .map(move |o| (cacheable, DistType::NoDist, o)) } +/// R0 instrumentation: number of compiles currently attempting distribution. +/// Read when a dist job starts so the log shows client-side supply pressure +/// (a low value while nodes sit idle points at job-supply starvation). +#[cfg(feature = "dist-client")] +static DIST_INFLIGHT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +/// RAII counter for `DIST_INFLIGHT`: increments on `enter` and decrements on +/// drop, so the count stays correct across every exit of the dist future - +/// success, `?`, `bail!`, or a dropped future - without a manual decrement in +/// each error arm and the local-fallback path. +#[cfg(feature = "dist-client")] +struct DistInflightGuard; + +#[cfg(feature = "dist-client")] +impl DistInflightGuard { + fn enter() -> (Self, usize) { + let inflight = DIST_INFLIGHT.fetch_add(1, std::sync::atomic::Ordering::SeqCst) + 1; + (DistInflightGuard, inflight) + } +} + +#[cfg(feature = "dist-client")] +impl Drop for DistInflightGuard { + fn drop(&mut self) { + DIST_INFLIGHT.fetch_sub(1, std::sync::atomic::Ordering::SeqCst); + } +} + #[cfg(feature = "dist-client")] async fn dist_or_local_compile( service: &server::SccacheService, @@ -895,6 +923,9 @@ where let local_executable2 = compile_cmd.get_executable(); let do_dist_compile = async move { + let (_inflight_guard, in_flight) = DistInflightGuard::enter(); + let dist_started = Instant::now(); + info!("[{}]: dist-job start (in_flight {})", out_pretty, in_flight); let mut dist_compile_cmd = dist_compile_cmd.context("Could not create distributed compile command")?; debug!("[{}]: Creating distributed compile request", out_pretty); @@ -918,9 +949,11 @@ where "[{}]: Identifying dist toolchain for {:?}", out_pretty, local_executable ); + let t_put_toolchain = Instant::now(); let (dist_toolchain, maybe_dist_compile_executable) = dist_client .put_toolchain(local_executable, weak_toolchain_key, toolchain_packager) .await?; + let ms_put_toolchain = t_put_toolchain.elapsed().as_millis(); let mut tc_archive = None; if let Some((dist_compile_executable, archive_path)) = maybe_dist_compile_executable { dist_compile_cmd.executable = dist_compile_executable; @@ -928,7 +961,10 @@ where } debug!("[{}]: Requesting allocation", out_pretty); + let t_alloc = Instant::now(); let jares = dist_client.do_alloc_job(dist_toolchain.clone()).await?; + let ms_alloc = t_alloc.elapsed().as_millis(); + let mut ms_submit: u128 = 0; let job_alloc = match jares { dist::AllocJobResult::Success { job_alloc, @@ -939,11 +975,13 @@ where out_pretty, dist_toolchain.archive_id, job_alloc.job_id ); - match dist_client + let t_submit = Instant::now(); + let submit_res = dist_client .do_submit_toolchain(job_alloc.clone(), dist_toolchain) .await - .map_err(|e| e.context("Could not submit toolchain"))? - { + .map_err(|e| e.context("Could not submit toolchain"))?; + ms_submit = t_submit.elapsed().as_millis(); + match submit_res { dist::SubmitToolchainResult::Success => Ok(job_alloc), dist::SubmitToolchainResult::JobNotFound => { bail!("Job {} not found on server", job_alloc.job_id) @@ -965,6 +1003,7 @@ where let job_id = job_alloc.job_id; let server_id = job_alloc.server_id; debug!("[{}]: Running job", out_pretty); + let t_run = Instant::now(); let ((job_id, server_id), (jres, path_transformer)) = dist_client .do_run_job( job_alloc, @@ -1078,10 +1117,17 @@ where ); } + let ms_run_fetch = t_run.elapsed().as_millis(); info!( - "[{}]: Distributed compilation finished on {}", + "[{}]: dist-job done on {} in {}ms (put_tc {}ms, alloc {}ms, submit {}ms, run+fetch {}ms, in_flight {})", out_pretty, - server_id.addr() + server_id.addr(), + dist_started.elapsed().as_millis(), + ms_put_toolchain, + ms_alloc, + ms_submit, + ms_run_fetch, + in_flight, ); Ok((DistType::Ok(server_id), jc.output.into())) }; From 0a57546f67adc860f375ce50bd4c8d651bcf2417 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 08:41:52 -0600 Subject: [PATCH 18/43] dist/scheduler: soften the error demotion so an idle remote node is not stranded A server that fails a job assignment is dropped into the best_err bucket and only chosen when no error-free server has capacity. That demotion is absolute: `best.or(best_err)` takes any healthy server over an errored one regardless of load. On a two-node cluster the errored bucket is not symmetric - the build server co-located with the scheduler assigns over loopback and effectively never errors, while the network-remote node takes the occasional transient assignment error. So the remote node gets demoted, sits idle, and the local node absorbs the work. Instrumentation over one image build measured 1710 allocations that skipped an idle server for a busier healthy one, every single one on the remote node. Treat a recent error as a bounded load penalty instead of an absolute veto: a recently-errored server still wins when it is more than RECENT_ERROR_LOAD_PENALTY less loaded than the healthy alternative. A genuinely overloaded or persistently failing server is still avoided (its real load, or a fresh error each attempt, keeps it behind), but a transient blip no longer strands a node's idle capacity for the full remember-error window - and a mistaken pick costs only one wasted round-trip before the local fallback. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 112 ++++++++++++++++++++++++++++++++++- 1 file changed, 110 insertions(+), 2 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 0f6513f4f..9a01d4617 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -331,6 +331,16 @@ fn init_logging() { // Maximum number of jobs per core - only occurs for one core, usually less, see load_weight() const MAX_PER_CORE_LOAD: f64 = 2f64; const SERVER_REMEMBER_ERROR_TIMEOUT: Duration = Duration::from_secs(300); +// A server that just failed a job assignment is demoted so the scheduler +// prefers a healthy one. The demotion must not be absolute, though: a +// network-remote server takes transient assignment errors that a +// scheduler-colocated server never does, so a hard demotion strands the remote +// node's idle capacity while jobs pile onto a busy local one (measured: 1710 +// idle-server skips in one build, all on the remote node). Treat a recent error +// as this much extra load instead, so a recently-errored server still wins when +// it is more than this margin less loaded than the healthy alternative - a +// repeat failure only costs one wasted round-trip and a local fallback. +const RECENT_ERROR_LOAD_PENALTY: f64 = 0.125; const UNCLAIMED_PENDING_TIMEOUT: Duration = Duration::from_secs(300); const UNCLAIMED_READY_TIMEOUT: Duration = Duration::from_secs(60); @@ -452,6 +462,7 @@ impl SchedulerIncoming for Scheduler { let mut best = None; let mut best_err = None; let mut best_load: f64 = MAX_PER_CORE_LOAD; + let mut best_err_load: f64 = MAX_PER_CORE_LOAD; let now = Instant::now(); for (&server_id, details) in servers.iter_mut() { // A server silent past the heartbeat timeout is dead but @@ -490,6 +501,7 @@ impl SchedulerIncoming for Scheduler { now - last_error ); best_err = Some((server_id, details)); + best_err_load = load; } } _ => { @@ -499,6 +511,7 @@ impl SchedulerIncoming for Scheduler { now - last_error ); best_err = Some((server_id, details)); + best_err_load = load; } } } @@ -512,8 +525,25 @@ impl SchedulerIncoming for Scheduler { } } - // Assign the job to our best choice - if let Some((server_id, server_details)) = best.or(best_err) { + // Assign the job to our best choice. Prefer the least-loaded + // healthy server, but fall to a recently-errored one when it is + // more than RECENT_ERROR_LOAD_PENALTY less loaded - so a remote + // node briefly demoted by a transient error is not stranded idle + // while a busy healthy node absorbs the work. best_load / + // best_err_load stay MAX_PER_CORE_LOAD while their bucket is + // empty, so the comparison naturally keeps the populated one. + let choice = match (best, best_err) { + (Some(clean), Some(errored)) => { + if best_err_load + RECENT_ERROR_LOAD_PENALTY < best_load { + Some(errored) + } else { + Some(clean) + } + } + (Some(clean), None) => Some(clean), + (None, errored) => errored, + }; + if let Some((server_id, server_details)) = choice { let job_count = self.job_count.fetch_add(1, Ordering::SeqCst) as u64; let job_id = JobId(job_count); assert!(server_details.jobs_assigned.insert(job_id)); @@ -1047,4 +1077,82 @@ mod scheduler_tests { "a deregistered server must be gone immediately" ); } + + // Insert a live server carrying `num_jobs` assigned jobs and an optional + // recent error. Job ids start high so they never collide with the ids the + // scheduler mints from job_count (which starts at 0) during the alloc. + fn insert_busy_server( + scheduler: &Scheduler, + addr: &str, + num_jobs: usize, + last_error: Option, + ) { + let server_id = ServerId::new(addr.parse::().unwrap()); + let mut jobs_assigned = HashSet::new(); + for i in 0..num_jobs { + jobs_assigned.insert(JobId(1000 + i as u64)); + } + let mut servers = scheduler.servers.lock().unwrap(); + servers.insert( + server_id, + ServerDetails { + last_seen: Instant::now(), + last_error, + jobs_assigned, + jobs_unclaimed: HashMap::new(), + num_cpus: 32, + server_nonce: ServerNonce::new(), + job_authorizer: Box::new(NoopAuthorizer), + }, + ); + } + + // A recently-errored but idle server must still win over a healthy but + // heavily-loaded one. A network-remote node takes transient assignment + // errors a colocated node never does; stranding its idle capacity while a + // busy node absorbs the work was the dominant misroute (measured 1710x, all + // on the remote node). Here the 12/32-vs-0 load gap clears the error + // penalty, so the idle errored node wins. + #[test] + fn test_alloc_job_prefers_idle_errored_server_over_loaded_healthy_server() { + let scheduler = Scheduler::new(); + insert_busy_server(&scheduler, "10.42.0.1:10501", 12, None); + insert_busy_server(&scheduler, "10.42.0.2:10501", 0, Some(Instant::now())); + + let res = scheduler + .handle_alloc_job(&AssignOk, a_toolchain()) + .expect("handle_alloc_job should not error"); + + match res { + AllocJobResult::Success { job_alloc, .. } => assert_eq!( + job_alloc.server_id.addr().to_string(), + "10.42.0.2:10501", + "the idle recently-errored server must win over the loaded healthy one" + ), + _ => panic!("expected a successful allocation"), + } + } + + // The error demotion is not abandoned: when the healthy server is only + // marginally busier (within the penalty), the recently-errored server stays + // demoted, so a flaky node is not chosen for a negligible capacity gain. + #[test] + fn test_alloc_job_keeps_healthy_server_when_error_penalty_outweighs_gap() { + let scheduler = Scheduler::new(); + insert_busy_server(&scheduler, "10.42.0.1:10501", 2, None); + insert_busy_server(&scheduler, "10.42.0.2:10501", 0, Some(Instant::now())); + + let res = scheduler + .handle_alloc_job(&AssignOk, a_toolchain()) + .expect("handle_alloc_job should not error"); + + match res { + AllocJobResult::Success { job_alloc, .. } => assert_eq!( + job_alloc.server_id.addr().to_string(), + "10.42.0.1:10501", + "a marginally-busier healthy server must be kept over a flaky one" + ), + _ => panic!("expected a successful allocation"), + } + } } From 43c5cdbbf5eeca20eebaeebf81fbdb6369804a23 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 10:16:17 -0600 Subject: [PATCH 19/43] compiler/rust: keep a distributed compile when its dep-info is unmatched Rust compiles never distributed on the cluster - measured 0 of 659 while C/C++ distributed 51k. Every rust crate ran on the build server, came back successfully, and was then thrown away: RustOutputsRewriter expects the dep-info (.d) file among the returned outputs and, when none matches, bail!s "No outputs matched dep info file". That aborts the whole distributed compile, so the or_else path recompiles locally - stranding every rust compile back on the client while its remote result is discarded. The dep file only feeds cargo's rebuild tracking; a bitbake do_compile runs cargo from scratch and never consumes it across tasks. So a good remote object/rlib must not be discarded over it. Replace the bail with a warning that keeps the returned outputs and skips only the dep-info path rewrite. This is the same shape as the fork's existing dropped-output tolerance for glibc's .o.dt file, applied to the rust rewriter. Also drop the error!-level log of the full dep-file body on every rewrite to trace! - it fired on the success path and would flood the log now that the success path is actually reached. Signed-off-by: Javier Tia --- src/compiler/rust.rs | 45 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 42 insertions(+), 3 deletions(-) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 3382616a4..d093dfc03 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -2281,7 +2281,7 @@ impl OutputsRewriter for RustOutputsRewriter { local_path.display() ) })?; - error!( + trace!( "RE replacing {} with {} in {}", re_str, local_path_str, deps ); @@ -2298,13 +2298,52 @@ impl OutputsRewriter for RustOutputsRewriter { return Ok(()); } } - // We expected there to be dep info, but none of the outputs matched - bail!("No outputs matched dep info file {}", dep_info.display()); + // The compile succeeded on the build server, but none of the + // outputs it returned matched the expected dep-info (.d) path. Do + // NOT fail the whole distributed compile over the dep file: keep the + // object/rlib the server produced and skip the dep-info path + // rewrite, instead of discarding a good remote compile and forcing a + // local recompile - which stranded every distributed rust compile + // back on the client. The dep file only feeds rebuild tracking, and + // a bitbake do_compile runs cargo from scratch, so a missing or + // unremapped .d is harmless here. + warn!( + "distributed compile did not return the expected dep info file {}; \ + keeping the remote outputs and skipping the dep-info rewrite", + dep_info.display() + ); } Ok(()) } } +// A successful distributed compile whose returned outputs do not include the +// expected dep-info file must NOT be failed - the object/rlib is good, and +// discarding it forces a local recompile that strands every rust compile on the +// client (measured: 0 of 659 rust compiles distributed, all rebuilt on PC1). +// The rewriter keeps the remote outputs and returns Ok, only skipping the +// dep-info path rewrite. +#[test] +#[cfg(feature = "dist-client")] +fn test_rust_outputs_rewriter_tolerates_unmatched_dep_info() { + use crate::compiler::OutputsRewriter; + + let rewriter = Box::new(RustOutputsRewriter { + dep_info: Some(PathBuf::from("/build/deps/somecrate.d")), + }); + let pt = dist::PathTransformer::new(); + + // No output path matches the expected dep-info - the old code bailed here. + let res = rewriter.handle_outputs(&pt, &[PathBuf::from("/build/deps/somecrate.rlib")], &[]); + + assert!( + res.is_ok(), + "an otherwise-successful distributed compile must not be failed just \ + because its dep-info file was not among the returned outputs: {:?}", + res.err() + ); +} + #[test] #[cfg(all(feature = "dist-client", target_os = "windows"))] fn test_rust_outputs_rewriter() { From c39c2bf5fbc68795baf7efa978932a7886505205 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 10:30:21 -0600 Subject: [PATCH 20/43] dist: don't fail a distributed compile over an optional missing output Softening the rust dep-info rewriter (previous commit) was not enough to make rust distribute: a rust compile still fell back to local, now caught by the second guard. After a distributed compile the client verifies every declared output exists and, if one is missing, discards the result and recompiles locally. That guard ignored the `optional` flag, so a missing optional output forced a local recompile even though the output was declared droppable - stranding every rust compile on the client because the rust dep-info (.d) file, which the build server does not reliably return, was declared non-optional. Make the guard honor `optional`: only non-optional outputs must exist. The C dep file and glibc's dropped .o.dt stay non-optional, so their local-fallback (the reason that guard exists) is unchanged; a `-gsplit-dwarf` .dwo, already declared optional, no longer wrongly forces fallback. Then mark the rust .d optional, since it only feeds cargo rebuild tracking a from-scratch bitbake do_compile never consumes. With both, a distributed rust compile whose .d is absent keeps its object and rlib instead of recompiling locally. Signed-off-by: Javier Tia --- src/compiler/compiler.rs | 1 + src/compiler/rust.rs | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 0f013d60d..a71c7456d 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -940,6 +940,7 @@ where // and salvaged (see the missing-output check after the run completes). let expected_output_paths: Vec = compilation .outputs() + .filter(|output| !output.optional) .map(|output| cwd.join(output.path)) .collect(); let (inputs_packager, toolchain_packager, outputs_rewriter) = diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index d093dfc03..625a4fd2c 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -1622,7 +1622,13 @@ where dep_info.to_string_lossy().into_owned(), ArtifactDescriptor { path: p.clone(), - optional: false, + // The build server does not reliably return the rust + // dep-info (.d) file, and it only feeds cargo's rebuild + // tracking (a bitbake do_compile runs cargo from scratch and + // never consumes it). Mark it optional so a distributed + // compile that omits it is not discarded and recompiled + // locally - which stranded every rust compile on the client. + optional: true, }, ); Some(p) From 776fad219a1425f878c2e69039ea0d3d95c05358 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 10:53:42 -0600 Subject: [PATCH 21/43] dist/pkg: skip dangling symlinks when packaging a toolchain Cold toolchain packaging walked the relocated interpreter's sysroot and called fs::metadata on every symlink to decide whether it pointed at a regular file. An OE recipe-sysroot-native tree carries sysroot-relative symlinks such as usr/bin/sg whose target resolves outside the packaged subtree, so metadata failed with ENOENT and the `?` aborted the entire add_dir_contents walk. The whole toolchain package failed to build, the client could not upload it, and every affected compile fell back to a local build instead of distributing. Rust recipes hit this on every cold cache because their toolchain is packaged fresh. Skip a symlink whose metadata cannot be resolved instead of failing, matching the swallow-on-error handling add_shared_libraries already uses for the identical check. A dangling link can never point at a file the package needs, and the walk is documented as best-effort, so dropping it is safe and keeps the package building. Signed-off-by: Javier Tia --- src/dist/pkg.rs | 40 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 38 insertions(+), 2 deletions(-) diff --git a/src/dist/pkg.rs b/src/dist/pkg.rs index d8dce0d03..1a48b38a2 100644 --- a/src/dist/pkg.rs +++ b/src/dist/pkg.rs @@ -190,8 +190,15 @@ mod toolchain_imp { if file_type.is_dir() { continue; } else if file_type.is_symlink() { - let metadata = fs::metadata(entry.path())?; - if !metadata.file_type().is_file() { + // A dangling symlink (e.g. a sysroot-relative link that + // resolves outside the packaged tree, like an OE + // recipe-sysroot-native `usr/bin/sg`) makes metadata fail. + // Skip it rather than aborting the whole best-effort + // package, matching add_shared_libraries below. + if !fs::metadata(entry.path()) + .map(|m| m.is_file()) + .unwrap_or(false) + { continue; } } else if !file_type.is_file() { @@ -485,6 +492,35 @@ mod toolchain_imp { assert_eq!(sysroot_usr_libdir(&libdir), Some(usr_lib)); } + #[test] + fn test_add_dir_contents_skips_dangling_symlink() { + use std::os::unix::fs::symlink; + + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + + // A real file that must be packaged. + let real = root.join("real_file"); + std::fs::write(&real, b"contents").unwrap(); + + // A dangling symlink whose target does not exist -- the shape of an + // OE recipe-sysroot-native `usr/bin/sg` that points outside the + // packaged tree. Before the fix, fs::metadata on this aborted the + // whole toolchain package. + let dangling = root.join("dangling_link"); + symlink("does/not/exist", &dangling).unwrap(); + + let mut builder = ToolchainPackageBuilder::new(); + builder.add_dir_contents(root).unwrap(); + + let packaged: Vec = builder.file_set.values().cloned().collect(); + assert!(packaged.contains(&real), "real file must still be packaged"); + assert!( + !packaged.contains(&dangling), + "dangling symlink must be skipped, not packaged" + ); + } + #[test] fn test_ldd_parse() { let ubuntu_ls_output = "\tlinux-vdso.so.1 => (0x00007fffcfffe000) From 4e7bd403a8eae00785bb25061fcccfedceebf534 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 11:31:01 -0600 Subject: [PATCH 22/43] compiler/rust: ship the target spec so a remote rustc resolves --target A distributed rust compile against an OpenEmbedded custom target failed on every genuinely-remote build server with `error loading target specification: could not find specification for target "aarch64-avocado-linux-gnu"`. OE passes the target by name and points rustc at the spec JSON via RUST_TARGET_PATH; sccache-dist ships a target spec only when --target is a path, and never repoints RUST_TARGET_PATH, so the name-form target was invisible to the server and the remote rustc aborted. The client then fell back to a local recompile, keeping rust off the cluster (a colocated server on the client host sometimes succeeded only because the spec existed at its real host path). Resolve a bare --target NAME against the compile's own RUST_TARGET_PATH to the .json it refers to, add that JSON to the shipped inputs, and rewrite RUST_TARGET_PATH in the remote environment to the shipped location so the server resolves the name exactly as the client did. The --target argument keeps its name form on purpose: rustc records the target's identity from that string, and the prebuilt std in the recipe sysroot was built name-form, so a path-form rewrite would make std fail to match with E0461. Built-in targets have no matching file and are left untouched. Recording the resolved spec also folds its content into the hash, so these compiles miss once as a name-form target previously did not invalidate the cache when the spec changed. --- src/compiler/rust.rs | 146 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 140 insertions(+), 6 deletions(-) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 625a4fd2c..5be287dbe 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -534,9 +534,9 @@ where &self, arguments: &[OsString], cwd: &Path, - _env_vars: &[(OsString, OsString)], + env_vars: &[(OsString, OsString)], ) -> CompilerArguments + 'static>> { - match parse_arguments(arguments, cwd) { + match parse_arguments(arguments, cwd, env_vars) { CompilerArguments::Ok(args) => CompilerArguments::Ok(Box::new(RustHasher { executable: self.executable.clone(), // if rustup exists, this must already contain the true resolved compiler path host: self.host.clone(), @@ -983,6 +983,23 @@ impl FromArg for ArgTarget { )) } } +/// Resolve a bare `--target ` against `RUST_TARGET_PATH` to the custom +/// target-spec JSON it refers to, if one exists. +/// +/// rustc resolves a bare target name by searching each `RUST_TARGET_PATH` +/// directory for `.json` (built-in targets have no file and are left +/// alone). sccache-dist ships a target spec only when `--target` is a path, so +/// a name-form custom target is invisible to the build server and the remote +/// rustc aborts with "could not find specification for target". Resolving the +/// name to its JSON path here lets the existing path-form machinery hash and +/// ship the spec. `env_vars` is the compile's own environment, not the daemon's. +fn resolve_target_json_in_path(name: &str, env_vars: &[(OsString, OsString)]) -> Option { + let (_, rust_target_path) = env_vars.iter().find(|(k, _)| k == "RUST_TARGET_PATH")?; + std::env::split_paths(rust_target_path) + .map(|dir| dir.join(format!("{name}.json"))) + .find(|candidate| candidate.is_file()) +} + impl IntoArg for ArgTarget { fn into_arg_os_string(self) -> OsString { match self { @@ -1065,7 +1082,11 @@ counted_array!(static ARGS: [ArgInfo; _] = [ take_arg!("-o", PathBuf, CanBeSeparated, TooHardPath), ]); -fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments { +fn parse_arguments( + arguments: &[OsString], + cwd: &Path, + env_vars: &[(OsString, OsString)], +) -> CompilerArguments { let mut args = vec![]; let mut emit: Option> = None; @@ -1174,7 +1195,15 @@ fn parse_arguments(arguments: &[OsString], cwd: &Path) -> CompilerArguments match target { ArgTarget::Path(json_path) => target_json = Some(json_path.to_owned()), ArgTarget::Unsure(_) => cannot_cache!("target unsure"), - ArgTarget::Name(_) => (), + ArgTarget::Name(name) => { + // Keep the name form (rustc records it as the target's + // identity, matching the prebuilt std in the sysroot); we + // ship the spec and repoint RUST_TARGET_PATH for the remote + // instead. Resolving here lets the spec be hashed and shipped. + if let Some(json_path) = resolve_target_json_in_path(name, env_vars) { + target_json = Some(json_path); + } + } }, None => { match arg { @@ -1669,6 +1698,9 @@ where .into_iter() .chain(abs_externs) .chain(abs_staticlibs) + // Ship the custom target-spec JSON (if any) so a remote rustc can + // load it; without it a distributed compile fails to find the target. + .chain(self.parsed_args.target_json.iter().map(|p| cwd.join(p))) .collect(); Ok(HashResult { @@ -1815,6 +1847,18 @@ impl Compilation for RustCompilation { "CARGO" | "CARGO_MANIFEST_DIR" => { *v = path_transformer.as_dist(Path::new(v))?; } + "RUST_TARGET_PATH" => { + // Repoint the target-spec search path at the shipped + // copies so the remote rustc resolves a name-form + // --target the same way the client did. The spec JSON is + // shipped as a compile input above. The server is always + // Linux, so the entries join with ':'. + let mut dist_dirs = Vec::new(); + for dir in std::env::split_paths(v) { + dist_dirs.push(path_transformer.as_dist(&dir)?); + } + *v = dist_dirs.join(":"); + } _ => (), } } @@ -2719,7 +2763,7 @@ mod test { fn _parse_arguments(arguments: &[String]) -> CompilerArguments { let arguments = arguments.iter().map(OsString::from).collect::>(); - parse_arguments(&arguments, ".".as_ref()) + parse_arguments(&arguments, ".".as_ref(), &[]) } macro_rules! parses { @@ -3612,7 +3656,7 @@ proc_macro false F: Fn(&Path) -> Result<()>, { let oargs = args.iter().map(OsString::from).collect::>(); - let parsed_args = match parse_arguments(&oargs, f.tempdir.path()) { + let parsed_args = match parse_arguments(&oargs, f.tempdir.path(), env_vars) { CompilerArguments::Ok(parsed_args) => parsed_args, o => panic!("Got unexpected parse result: {:?}", o), }; @@ -4029,4 +4073,94 @@ proc_macro false ))); assert_eq!(h.target_json, Some(PathBuf::from("/path/to/target.json"))); } + + #[test] + fn test_parse_target_name_resolved_via_rust_target_path() { + // A bare `--target NAME` naming a custom target spec found on + // RUST_TARGET_PATH is resolved to that JSON path, so the spec is hashed + // and shipped and a remote rustc can load it (OE passes the target by + // name and relies on RUST_TARGET_PATH, which the build server lacks). + let tmp = tempfile::Builder::new() + .prefix("rust-target-path") + .tempdir() + .unwrap(); + let spec = tmp.path().join("aarch64-avocado-linux-gnu.json"); + std::fs::write(&spec, b"{}").unwrap(); + let env_vars = vec![( + OsString::from("RUST_TARGET_PATH"), + OsString::from(tmp.path()), + )]; + + let args = [ + "--crate-name", + "foo", + "--crate-type", + "lib", + "./src/lib.rs", + "--emit=dep-info,link", + "--out-dir", + "/out", + "--target", + "aarch64-avocado-linux-gnu", + ] + .iter() + .map(OsString::from) + .collect::>(); + + let h = match parse_arguments(&args, ".".as_ref(), &env_vars) { + CompilerArguments::Ok(a) => a, + o => panic!("Got unexpected parse result: {:?}", o), + }; + + // The spec is recorded for hashing + shipping, but --target keeps its + // name form so rustc's target identity still matches the prebuilt std. + assert_eq!(h.target_json, Some(spec)); + assert!(h.arguments.contains(&Argument::WithValue( + "--target", + ArgData::Target(ArgTarget::Name("aarch64-avocado-linux-gnu".to_owned())), + ArgDisposition::Separated + ))); + } + + #[test] + fn test_parse_target_name_unresolved_stays_name() { + // RUST_TARGET_PATH set but no matching .json: the target stays a + // bare name and target_json is None. Guards builtins from being rewritten. + let tmp = tempfile::Builder::new() + .prefix("rust-target-path") + .tempdir() + .unwrap(); + let env_vars = vec![( + OsString::from("RUST_TARGET_PATH"), + OsString::from(tmp.path()), + )]; + + let args = [ + "--crate-name", + "foo", + "--crate-type", + "lib", + "./src/lib.rs", + "--emit=dep-info,link", + "--out-dir", + "/out", + "--target", + "x86_64-unknown-linux-gnu", + ] + .iter() + .map(OsString::from) + .collect::>(); + + let h = match parse_arguments(&args, ".".as_ref(), &env_vars) { + CompilerArguments::Ok(a) => a, + o => panic!("Got unexpected parse result: {:?}", o), + }; + + assert!(h.target_json.is_none()); + assert!(h.arguments.contains(&Argument::WithValue( + "--target", + ArgData::Target(ArgTarget::Name("x86_64-unknown-linux-gnu".to_owned())), + ArgDisposition::Separated + ))); + } } From ec49924b134f4a4a79fc0d91ef007ccd0a5ffd52 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 12:02:21 -0600 Subject: [PATCH 23/43] compiler: log a failed distributed compile's remote output at debug When a distributed compile exits non-zero the client logs only the exit code and falls back to a local recompile, discarding the remote compiler's stdout and stderr. That output is the only place the actual failure is visible - a missing target std, an input the packager did not ship, a sandbox path that does not resolve - so diagnosing a dist regression meant patching in a temporary dump and rebuilding the binary each time. Emit the remote stdout/stderr at debug on the non-zero path. SCCACHE_LOG=debug now surfaces the real remote error with no rebuild, while a normal SCCACHE_LOG=info run stays quiet (the one-line fallback reason is still logged at warn by the caller). Signed-off-by: Javier Tia --- src/compiler/compiler.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index a71c7456d..016c296c0 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -1087,6 +1087,20 @@ where ); if jc.output.code != 0 { + // The one-line reason above is logged by the caller at warn; the + // remote compiler's own stdout/stderr is the only place the actual + // failure (a missing target std, an unshipped input) is visible, and + // the local fallback discards it. Emit it at debug so SCCACHE_LOG=debug + // surfaces the remote error without a rebuild, while a normal + // SCCACHE_LOG=info run stays quiet. + debug!( + "[{}]: distributed compile on {} exited {}; remote stderr:\n{}\nremote stdout:\n{}", + out_pretty, + server_id.addr(), + jc.output.code, + String::from_utf8_lossy(&jc.output.stderr), + String::from_utf8_lossy(&jc.output.stdout), + ); // A non-zero remote result is frequently a distribution artifact // rather than a genuine compiler error: e.g. an object that // .incbin's a binary the inputs packager did not ship (the kernel's From 251f8516be4660982190b585c8fe7f9047dd764e Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 12:26:19 -0600 Subject: [PATCH 24/43] compiler/rust: ship the target sysroot std to distributed compiles A distributed cross-compile against an OpenEmbedded custom target failed on the remote with `error[E0463]: can't find crate for core`/`std`. The target's prebuilt std/core rlibs live in /usr/lib/rustlib//lib and are referenced via `-L`; that path is transformed and emitted to the server, but the rlibs themselves are dropped. write_inputs filters each crate-link dir to the crate names cargo discovered as rlib deps, and the implicit sysroot crates (core, std, alloc, compiler_builtins, proc_macro) are injected by rustc and never appear as externs, so the filter removes them and the remote rustc has no std. Exempt a target sysroot lib dir from that filter and ship all its rlibs. The dir is recognized by its `rustlib//lib` tail, so an ordinary `-L dependency=target//deps` search path is unaffected and still filtered to the discovered dep set. The rlibs enter the per-compile input set already scoped to this exact target, so no toolchain cache key changes are needed - unlike bundling them into the shared toolchain, whose key (weak_toolchain_key) is not target-specific and would alias std across targets built by the same host rustc. Signed-off-by: Javier Tia --- src/compiler/rust.rs | 41 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 2 deletions(-) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 5be287dbe..d1f0e16df 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -2059,6 +2059,24 @@ fn test_maybe_add_cargo_toml() { assert!(maybe_add_cargo_toml(&wgpu_lib, true).is_none()); } +/// Is `dir` a rustc target sysroot lib dir (`/lib/rustlib//lib`)? +/// +/// Such a dir holds the implicit sysroot crates (core, std, alloc, ...) that +/// rustc injects but that never appear as `--extern` deps, so the crate-dep +/// filter in `write_inputs` drops them. A cross-compile references the target's +/// prebuilt std here via `-L`; the remote rustc needs those rlibs, so libs from +/// this dir shape are shipped unconditionally. Anchored on the `rustlib/*/lib` +/// tail so an ordinary `-L dependency=target/*/deps` dir still gets filtered. +#[cfg(feature = "dist-client")] +fn is_rustlib_target_libdir(dir: &Path) -> bool { + dir.file_name().is_some_and(|n| n == "lib") + && dir + .parent() + .and_then(Path::parent) + .and_then(Path::file_name) + .is_some_and(|n| n == "rustlib") +} + #[cfg(feature = "dist-client")] impl pkg::InputsPackager for RustInputsPackager { #[allow(clippy::cognitive_complexity)] // TODO simplify this method. @@ -2140,6 +2158,9 @@ impl pkg::InputsPackager for RustInputsPackager { let mut tar_crate_libs = vec![]; for crate_link_path in crate_link_paths { let crate_link_path = pkg::simplify_path(&crate_link_path)?; + // Ship every rlib from a target sysroot lib dir; its implicit + // sysroot crates are absent from the cargo dep-name set below. + let ship_all = is_rustlib_target_libdir(&crate_link_path); let dir_entries = match fs::read_dir(crate_link_path) { Ok(iter) => iter, Err(e) if e.kind() == io::ErrorKind::NotFound => continue, @@ -2181,8 +2202,9 @@ impl pkg::InputsPackager for RustInputsPackager { _ => continue, }; if let Some((_, ref dep_crate_names)) = rlib_dep_reader_and_names { - // We have a list of crate names we care about, see if this lib is a candidate - if !dep_crate_names.contains(crate_name) { + // We have a list of crate names we care about, see if this lib is a candidate. + // A target rustlib dir is exempt: its sysroot crates never appear as deps. + if !ship_all && !dep_crate_names.contains(crate_name) { continue; } } @@ -4163,4 +4185,19 @@ proc_macro false ArgDisposition::Separated ))); } + + #[cfg(feature = "dist-client")] + #[test] + fn test_is_rustlib_target_libdir() { + // A target sysroot lib dir (holds the prebuilt std/core rlibs). + assert!(is_rustlib_target_libdir(Path::new( + "/work/recipe-sysroot/usr/lib/rustlib/aarch64-avocado-linux-gnu/lib" + ))); + // An ordinary cargo dependency search path must stay filtered. + assert!(!is_rustlib_target_libdir(Path::new( + "/work/git/target/aarch64-avocado-linux-gnu/release/deps" + ))); + // `lib` leaf without a `rustlib` grandparent is not a rustlib dir. + assert!(!is_rustlib_target_libdir(Path::new("/usr/lib"))); + } } From e1a415353503e1b25a2e8cf3b1f0bbee4ced4b82 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 13:44:57 -0600 Subject: [PATCH 25/43] compiler/rust: ship split-metadata rlibs whole instead of dropping them When packaging dist inputs, sccache trims a dependency rlib to just its `rust.metadata.bin` ar member, since a crate only inspects a dependency's metadata. The trim loop scanned for that member and, if it never found one, appended nothing at all - silently omitting the rlib from the inputs tar. An OpenEmbedded target sysroot builds its std/core with split metadata: the metadata lives in a sibling `.rmeta` and the `.rlib` carries only object members, no `rust.metadata.bin`. Every such rlib was therefore dropped, so a distributed cross-compile reached the build server with the target rustlib directory present but empty. no_std crates compiled anyway; every crate that needs `std` failed remotely with `error[E0463]: can't find crate for std` and fell back to a local recompile, keeping most Rust off the cluster. Extract the metadata scan into trimmed_rlib_metadata, returning None when the rlib has no embedded metadata member, and ship the whole rlib in that case. Normal rlibs still trim to metadata; split-metadata rlibs now travel intact. Signed-off-by: Javier Tia --- src/compiler/rust.rs | 108 +++++++++++++++++++++++++++++++++++-------- 1 file changed, 88 insertions(+), 20 deletions(-) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index d1f0e16df..45a4255ce 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -1981,6 +1981,81 @@ fn can_trim_this(input_path: &Path) -> bool { && !ar_path.exists() } +/// Return a metadata-only archive for a trimmable rlib: the single-member `ar` +/// holding its `rust.metadata.bin`, or `None` when the rlib has no embedded +/// metadata member. +/// +/// A rustc rlib normally carries its metadata as a `rust.metadata.bin` ar +/// member, so a dependency only inspected for metadata can ship trimmed. But a +/// split-metadata sysroot (e.g. an OpenEmbedded target `rustlib//lib`, +/// whose std/core rlibs keep metadata in a sibling `.rmeta` and carry only +/// object members) has no such member. Callers must ship the whole rlib in +/// that case -- trimming it to nothing drops the crate and leaves a remote +/// compile unable to find `std`. +#[cfg(feature = "dist-client")] +fn trimmed_rlib_metadata(input_path: &Path) -> Result>> { + let file = fs::File::open(input_path)?; + let mut archive = ar::Archive::new(file); + while let Some(entry_result) = archive.next_entry() { + let mut entry = entry_result?; + if entry.header().identifier() != b"rust.metadata.bin" { + continue; + } + let mut metadata_ar = vec![]; + { + let mut ar_builder = ar::Builder::new(&mut metadata_ar); + let header = entry.header().clone(); + ar_builder.append(&header, &mut entry)?; + } + return Ok(Some(metadata_ar)); + } + Ok(None) +} + +#[test] +#[cfg(feature = "dist-client")] +fn test_trimmed_rlib_metadata() { + let tmp = tempfile::Builder::new() + .prefix("sccache_trim") + .tempdir() + .unwrap(); + + // A split-metadata rlib (object members only, no rust.metadata.bin), as an + // OE target sysroot ships: must not trim to nothing -> None (ship whole). + let split = tmp.path().join("libcore-split.rlib"); + { + let mut b = ar::Builder::new(fs::File::create(&split).unwrap()); + b.append(&ar::Header::new(b"foo.o".to_vec(), 3), &b"abc"[..]) + .unwrap(); + } + assert!( + trimmed_rlib_metadata(&split).unwrap().is_none(), + "an rlib without rust.metadata.bin must ship whole, not trim to nothing" + ); + + // A normal rlib with an embedded rust.metadata.bin -> trim to that member. + let embedded = tmp.path().join("libfoo-embedded.rlib"); + { + let mut b = ar::Builder::new(fs::File::create(&embedded).unwrap()); + b.append( + &ar::Header::new(b"rust.metadata.bin".to_vec(), 4), + &b"META"[..], + ) + .unwrap(); + b.append(&ar::Header::new(b"foo.o".to_vec(), 3), &b"abc"[..]) + .unwrap(); + } + let trimmed = trimmed_rlib_metadata(&embedded).unwrap(); + assert!( + trimmed.is_some(), + "an rlib with metadata should trim to Some" + ); + assert!( + trimmed.unwrap().windows(4).any(|w| w == b"META"), + "the trimmed archive must contain the metadata member" + ); +} + #[test] #[cfg(feature = "dist-client")] fn test_can_trim_this() { @@ -2245,27 +2320,20 @@ impl pkg::InputsPackager for RustInputsPackager { for (input_path, dist_input_path) in all_tar_inputs.iter() { let mut file_header = pkg::make_tar_header(input_path, dist_input_path)?; - let file = fs::File::open(input_path)?; - if can_trim_rlibs && can_trim_this(input_path) { - let mut archive = ar::Archive::new(file); - - while let Some(entry_result) = archive.next_entry() { - let mut entry = entry_result?; - if entry.header().identifier() != b"rust.metadata.bin" { - continue; - } - let mut metadata_ar = vec![]; - { - let mut ar_builder = ar::Builder::new(&mut metadata_ar); - let header = entry.header().clone(); - ar_builder.append(&header, &mut entry)?; - } - file_header.set_size(metadata_ar.len() as u64); - file_header.set_cksum(); - builder.append(&file_header, metadata_ar.as_slice())?; - break; - } + let trimmed = if can_trim_rlibs && can_trim_this(input_path) { + trimmed_rlib_metadata(input_path)? + } else { + None + }; + if let Some(metadata_ar) = trimmed { + file_header.set_size(metadata_ar.len() as u64); + file_header.set_cksum(); + builder.append(&file_header, metadata_ar.as_slice())?; } else { + // Either not trimmable, or a split-metadata rlib with no + // embedded metadata member -- ship the whole file so the remote + // gets the real crate (e.g. an OE target sysroot's std/core). + let file = fs::File::open(input_path)?; file_header.set_cksum(); builder.append(&file_header, file)?; } From 651c0de5c9b39261e6904105b614b5fb055c20c4 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 14:16:14 -0600 Subject: [PATCH 26/43] compiler/rust: ship sysroot rlibs whose name has no hash suffix The dist input packager scans each crate link-path directory and parses a library's crate name by splitting the filename on its last `-` to drop the `-.rlib` suffix cargo emits. A file with no `-` fell into the skip branch and was dropped from the inputs entirely. An OpenEmbedded target sysroot names the std crate `libstd.rlib` with no hash suffix (every other crate there is `lib-.rlib`). It was therefore never shipped, so a remote cross-compile had core and alloc but no std - and every crate that pulls in the std prelude failed with a cascade of "cannot find type `Option`/`Result`/`Vec`" and fell back to a local recompile. Fall back to the filename with its extension stripped when there is no `-` to split on, so a hashless `libstd.rlib` resolves to crate name `std` and ships. With this the recipe's Rust crates distribute across both nodes (21/21, zero failed distributed compiles) where std-dependent crates previously all fell back local. Signed-off-by: Javier Tia --- src/compiler/rust.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 45a4255ce..27d571a57 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -2255,11 +2255,14 @@ impl pkg::InputsPackager for RustInputsPackager { let mut rev_name_split = name.rsplitn(2, '-'); let _extra_filename_and_ext = rev_name_split.next(); let libname = if let Some(libname) = rev_name_split.next() { + assert!(rev_name_split.next().is_none()); libname } else { - continue; + // No `-` suffix (e.g. an OE target sysroot's + // libstd.rlib): fall back to the name with its + // extension stripped so std still ships. + name.rsplit_once('.').map(|(stem, _)| stem).unwrap_or(name) }; - assert!(rev_name_split.next().is_none()); libname } None => continue, From 7c2ce300ea104ac1eea1aa0a94a45065b15580ab Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Thu, 2 Jul 2026 17:52:19 -0600 Subject: [PATCH 27/43] dist: time local preprocessing in the per-job load diagnostics The R0 dist instrumentation times put_toolchain, alloc, submit, and run+fetch per job, but not the client-side preprocessing that produces the shipped input. That leaves the W2 gap: the preprocessing tax is the leading theory for why the colocated node shows far more cc1 activity than the remote one, yet it is only inferred from cc1 process counts, never measured against the dist round-trip it precedes. Time the single cc1 -E invocation in generate_hash_key (the preprocess await in the C hasher) and thread the duration to the per-job line via a new Compilation::preprocess_duration accessor. The value is captured where the work happens rather than re-derived on the dist path, because preprocessing runs during hash-key generation, before the dist round-trip begins, so it cannot be timed inside do_dist_compile itself. A trait default returns None so only the C compiler, which does local preprocessing, carries a value; Rust and already-preprocessed inputs log 0ms. The number joins the existing dist-job done line as preprocess Nms and is not folded into the round-trip total. Signed-off-by: Javier Tia --- src/compiler/c.rs | 24 ++++++++++++++++++++++++ src/compiler/compiler.rs | 21 ++++++++++++++++++++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/compiler/c.rs b/src/compiler/c.rs index e38676bfe..db4e7844d 100644 --- a/src/compiler/c.rs +++ b/src/compiler/c.rs @@ -137,6 +137,13 @@ struct CCompilation { is_locally_preprocessed: bool, #[cfg(feature = "dist-client")] preprocessed_input: Vec, + /// R0 instrumentation: wall-time spent running the local preprocessor + /// (`cc1 -E`) for this compile during hash-key generation, threaded into + /// the per-job dist log so the client-side preprocessing tax is timed + /// rather than inferred. `None` when no local preprocessing ran (input was + /// already preprocessed, or a preprocessor-cache hit skipped the run). + #[cfg(feature = "dist-client")] + preprocess_duration: Option, executable: PathBuf, compiler: I, cwd: PathBuf, @@ -464,6 +471,8 @@ where None }; + #[cfg(feature = "dist-client")] + let mut preprocess_elapsed: Option = None; let (preprocessor_output, include_files) = if needs_preprocessing { if let Some(preprocessor_key) = &preprocessor_key { if cache_control == CacheControl::Default { @@ -519,6 +528,8 @@ where #[cfg(feature = "dist-client")] preprocessed_input: PREPROCESSING_SKIPPED_COMPILE_POISON .to_vec(), + #[cfg(feature = "dist-client")] + preprocess_duration: None, executable: self.executable.clone(), compiler: self.compiler.to_owned(), cwd: cwd.clone(), @@ -534,6 +545,8 @@ where } } + #[cfg(feature = "dist-client")] + let preprocess_start = std::time::Instant::now(); let result = self .compiler .preprocess( @@ -547,6 +560,10 @@ where use_preprocessor_cache_mode, ) .await; + #[cfg(feature = "dist-client")] + { + preprocess_elapsed = Some(preprocess_start.elapsed()); + } let out_pretty = self.parsed_args.output_pretty().into_owned(); let result = result.map_err(|e| { debug!("[{}]: preprocessor failed: {:?}", out_pretty, e); @@ -677,6 +694,8 @@ where is_locally_preprocessed: true, #[cfg(feature = "dist-client")] preprocessed_input: preprocessor_output, + #[cfg(feature = "dist-client")] + preprocess_duration: preprocess_elapsed, executable: self.executable.clone(), compiler: self.compiler.clone(), cwd, @@ -1248,6 +1267,11 @@ impl Compilation for CCompilation self.is_locally_preprocessed } + #[cfg(feature = "dist-client")] + fn preprocess_duration(&self) -> Option { + self.preprocess_duration + } + fn outputs<'a>(&'a self) -> Box + 'a> { Box::new( self.parsed_args diff --git a/src/compiler/compiler.rs b/src/compiler/compiler.rs index 016c296c0..b49fb36a2 100644 --- a/src/compiler/compiler.rs +++ b/src/compiler/compiler.rs @@ -926,6 +926,13 @@ where let (_inflight_guard, in_flight) = DistInflightGuard::enter(); let dist_started = Instant::now(); info!("[{}]: dist-job start (in_flight {})", out_pretty, in_flight); + // Local preprocessing (`cc1 -E`) already ran during hash-key generation, + // before this dist round-trip began; read its duration here so the tax + // that skews load onto the colocated node is timed in the per-job line. + let ms_preprocess = compilation + .preprocess_duration() + .map(|d| d.as_millis()) + .unwrap_or(0); let mut dist_compile_cmd = dist_compile_cmd.context("Could not create distributed compile command")?; debug!("[{}]: Creating distributed compile request", out_pretty); @@ -1134,10 +1141,11 @@ where let ms_run_fetch = t_run.elapsed().as_millis(); info!( - "[{}]: dist-job done on {} in {}ms (put_tc {}ms, alloc {}ms, submit {}ms, run+fetch {}ms, in_flight {})", + "[{}]: dist-job done on {} in {}ms (preprocess {}ms, put_tc {}ms, alloc {}ms, submit {}ms, run+fetch {}ms, in_flight {})", out_pretty, server_id.addr(), dist_started.elapsed().as_millis(), + ms_preprocess, ms_put_toolchain, ms_alloc, ms_submit, @@ -1212,6 +1220,17 @@ where true } + /// R0 instrumentation: wall-time the client spent locally preprocessing + /// this compile's source (`cc1 -E`) during hash-key generation, when that + /// step ran. Threaded into the per-job dist log so the preprocessing tax is + /// timed rather than inferred. `None` when no local preprocessing ran + /// (input already preprocessed, or a preprocessor-cache hit skipped it), or + /// for compilers whose hash step does no local preprocessing. + #[cfg(feature = "dist-client")] + fn preprocess_duration(&self) -> Option { + None + } + /// Returns an iterator over the results of this compilation. /// /// Each item is a descriptive (and unique) name of the output paired with From 194113506fe294b29dfe3d0548489d19b1ea50fd Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 08:05:46 -0600 Subject: [PATCH 28/43] dist/scheduler: make routing topology-aware and deterministic The load model weighs every server as job_count/core_count with the raw hardware core count, which is blind to the colocated node also spending cores preprocessing (cc1 -E) for the whole cluster, packaging toolchains, and running bitbake and the scheduler. It therefore balances jobs ~50/50 onto a node that is really the busier of the two, and its both-idle tie is decided by hash-map iteration order - the same node wins every time, so a compile burst always starts on the pinned local node - while the load==0 early break truncates the logged candidate list, hiding the imbalance from the routing metrics. Add an optional advertised num_cpus to the server config: a colocated node can advertise fewer cores than it has so load_weight de-weights it and its admission cutoff drops, reserving cores for the preprocessing tax the scheduler cannot see. Remove the early break so every server is scanned (complete candidate log, no hidden misroutes) and break ties deterministically toward the node with more free admission slots - with the advertised-cores override the remote node wins, so bursts start off the busy local node. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 36 +++++++++++++++++++++++++++--------- src/config.rs | 7 +++++++ src/dist/http.rs | 10 +++++++++- 3 files changed, 43 insertions(+), 10 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 9a01d4617..c7f004c1e 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -16,6 +16,7 @@ use sccache::dist::{ }; use sccache::util::BASE64_URL_SAFE_ENGINE; use sccache::util::daemonize; +use sccache::util::num_cpus; use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, HashMap, HashSet, btree_map}; use std::env; @@ -239,6 +240,7 @@ fn run(command: Command) -> Result { scheduler_url, scheduler_auth, toolchain_cache_size, + num_cpus: advertised_num_cpus, }) => { let builder: Box = match builder { #[cfg(not(target_os = "freebsd"))] @@ -298,11 +300,13 @@ fn run(command: Command) -> Result { let server = Server::new(builder, &cache_dir, toolchain_cache_size) .context("Failed to create sccache server instance")?; + let advertised_num_cpus = advertised_num_cpus.unwrap_or_else(num_cpus); let http_server = dist::http::Server::new( public_addr, bind_address, scheduler_url.to_url(), scheduler_auth, + advertised_num_cpus, server, ) .context("Failed to create sccache HTTP server instance")?; @@ -429,14 +433,17 @@ impl Default for Scheduler { } } -fn load_weight(job_count: usize, core_count: usize) -> f64 { +fn admission_ceiling(core_count: usize) -> usize { // Oversubscribe cores just a little to make up for network and I/O latency. This formula is // not based on hard data but an extrapolation to high core counts of the conventional wisdom // that slightly more jobs than cores achieve the shortest compile time. Which is originally // about local compiles and this is over the network, so be slightly less conservative. - let cores_plus_slack = core_count + 1 + core_count / 8; + core_count + 1 + core_count / 8 +} + +fn load_weight(job_count: usize, core_count: usize) -> f64 { // Note >=, not >, because the question is "can we add another job"? - if job_count >= cores_plus_slack { + if job_count >= admission_ceiling(core_count) { MAX_PER_CORE_LOAD + 1f64 // no new jobs for now } else { job_count as f64 / core_count as f64 @@ -462,6 +469,7 @@ impl SchedulerIncoming for Scheduler { let mut best = None; let mut best_err = None; let mut best_load: f64 = MAX_PER_CORE_LOAD; + let mut best_free: usize = 0; let mut best_err_load: f64 = MAX_PER_CORE_LOAD; let now = Instant::now(); for (&server_id, details) in servers.iter_mut() { @@ -515,12 +523,22 @@ impl SchedulerIncoming for Scheduler { } } } - } else if load < best_load { - best = Some((server_id, details)); - trace!("Selected {:?} as the server with the best load", server_id); - best_load = load; - if load == 0f64 { - break; + } else { + // Deterministic, remote-preferring tie-break, and no early + // break: scan every server so the logged candidate list is + // complete (a truncated list can never expose a misroute) and + // hash-map order never silently decides a both-idle tie. On + // equal load prefer the server with more free admission slots - + // with A1's advertised-cores override the colocated node + // advertises fewer, so the remote node wins the tie and a + // compile burst starts off the already-busy local node. + let free = admission_ceiling(details.num_cpus) + .saturating_sub(details.jobs_assigned.len()); + if load < best_load || (load == best_load && free > best_free) { + trace!("Selected {:?} as the server with the best load", server_id); + best = Some((server_id, details)); + best_load = load; + best_free = free; } } } diff --git a/src/config.rs b/src/config.rs index f6c3660ef..05f5ad40d 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1599,6 +1599,13 @@ pub mod server { pub scheduler_auth: SchedulerAuth, #[serde(default = "default_toolchain_cache_size")] pub toolchain_cache_size: u64, + // Advertise fewer cores than the hardware so the scheduler de-weights + // this node (main.rs load_weight). On the colocated node the real cores + // are also spent preprocessing for the whole cluster and packaging + // toolchains, which the scheduler cannot see; capping the advertised + // count reserves them. None = use the detected hardware count. + #[serde(default)] + pub num_cpus: Option, } pub fn from_path(conf_path: &Path) -> Result> { diff --git a/src/dist/http.rs b/src/dist/http.rs index 4a55c52be..a66092d90 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -914,6 +914,11 @@ mod server { jwt_key: Vec, // Randomly generated nonce to allow the scheduler to detect server restarts server_nonce: ServerNonce, + // Core count advertised to the scheduler's load model. Defaults to the + // detected hardware count; a config override lets the colocated node + // advertise fewer so load_weight reserves cores for its preprocessing + // and toolchain-packaging tax. + advertised_num_cpus: usize, handler: S, } @@ -923,6 +928,7 @@ mod server { bind_address: Option, scheduler_url: reqwest::Url, scheduler_auth: String, + advertised_num_cpus: usize, handler: S, ) -> Result { let (cert_digest, cert_pem, privkey_pem) = @@ -941,6 +947,7 @@ mod server { privkey_pem, jwt_key, server_nonce, + advertised_num_cpus, handler, }) } @@ -955,6 +962,7 @@ mod server { privkey_pem, jwt_key, server_nonce, + advertised_num_cpus, handler, } = self; // Graceful shutdown: on SIGTERM/SIGINT, deregister from the @@ -991,7 +999,7 @@ mod server { }); let heartbeat_req = HeartbeatServerHttpRequest { - num_cpus: num_cpus(), + num_cpus: advertised_num_cpus, jwt_key: jwt_key.clone(), server_nonce, cert_digest, From 248f405a16feca355b543a02abdae6e7ec600996 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 08:08:39 -0600 Subject: [PATCH 29/43] compiler/c: gauge live local preprocess concurrency The preprocessing wall - the client jobserver's cores saturated by local cc1 -E for the whole cluster while remote nodes idle - was only ever an inference from per-job timers. Nothing measured how many preprocesses run at once, so there was no way to confirm the jobserver token pool is the feed bottleneck rather than bitbake under-producing jobs. Wrap the preprocess call in an RAII counter mirroring DistInflightGuard and log "preprocess done in ms (concurrent )" so the concurrency is a time series. The guard drops the count as soon as preprocessing returns, before the dist round-trip, so it reflects preprocess pressure alone; gated on dist-client so a non-dist build is unchanged. Signed-off-by: Javier Tia --- src/compiler/c.rs | 39 ++++++++++++++++++++++++++++++++++++++- 1 file changed, 38 insertions(+), 1 deletion(-) diff --git a/src/compiler/c.rs b/src/compiler/c.rs index db4e7844d..625ae927b 100644 --- a/src/compiler/c.rs +++ b/src/compiler/c.rs @@ -47,6 +47,34 @@ use crate::errors::*; use super::CacheControl; use super::preprocessor_cache::PreprocessorCacheEntry; +/// Live count of local preprocess (`cc1 -E`) invocations, gauged so the PC1 +/// preprocessing wall - the jobserver's cores saturated by preprocessing for +/// the whole cluster while remote nodes idle - is a measured number rather than +/// an inference. +#[cfg(feature = "dist-client")] +static PREPROC_INFLIGHT: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); + +/// RAII counter for `PREPROC_INFLIGHT`: increments on `enter`, returning the +/// concurrency including this one, and decrements on drop so the count stays +/// correct across every exit of the preprocess future. +#[cfg(feature = "dist-client")] +struct PreprocInflightGuard; + +#[cfg(feature = "dist-client")] +impl PreprocInflightGuard { + fn enter() -> (Self, usize) { + let inflight = PREPROC_INFLIGHT.fetch_add(1, std::sync::atomic::Ordering::SeqCst) + 1; + (PreprocInflightGuard, inflight) + } +} + +#[cfg(feature = "dist-client")] +impl Drop for PreprocInflightGuard { + fn drop(&mut self) { + PREPROC_INFLIGHT.fetch_sub(1, std::sync::atomic::Ordering::SeqCst); + } +} + /// A generic implementation of the `Compiler` trait for C/C++ compilers. #[derive(Clone)] pub struct CCompiler @@ -547,6 +575,8 @@ where #[cfg(feature = "dist-client")] let preprocess_start = std::time::Instant::now(); + #[cfg(feature = "dist-client")] + let (preproc_guard, preproc_concurrency) = PreprocInflightGuard::enter(); let result = self .compiler .preprocess( @@ -562,7 +592,14 @@ where .await; #[cfg(feature = "dist-client")] { - preprocess_elapsed = Some(preprocess_start.elapsed()); + let elapsed = preprocess_start.elapsed(); + preprocess_elapsed = Some(elapsed); + drop(preproc_guard); + info!( + "preprocess done in {}ms (concurrent {})", + elapsed.as_millis(), + preproc_concurrency + ); } let out_pretty = self.parsed_args.output_pretty().into_owned(); let result = result.map_err(|e| { From 6e5b8ac480fccb4c7f0af2b0f6791b4a6a10ffd5 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 09:31:31 -0600 Subject: [PATCH 30/43] dist/scheduler: derive the colocated de-weight dynamically The advertised-cores override added earlier had to be a hardcoded absolute in each colocated node's server.conf (num_cpus = 20 for a 32-core PC1), which is wrong on any other cluster topology. A different node count or a differently-provisioned host would need the value hand-recomputed. Compute it instead. A build server on the scheduler's own host is the colocated orchestrator node - it also runs the client's preprocessing and bitbake - so it reserves a fraction of its cores; a server on a different host is a pure remote and advertises all of them. The fraction defaults to 0.35, overridable via colocated_reserve_fraction, and an explicit num_cpus still wins as an escape hatch. Everything derives from the node's own hardware, so the same binary and config are portable across clusters. The resolution lives in a pure advertised_cores() with unit tests; the harness gains the advertised-cores argument the server constructor now takes. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 74 +++++++++++++++++++++++++++++++++--- src/config.rs | 13 ++++++- tests/harness/mod.rs | 18 ++++++--- 3 files changed, 93 insertions(+), 12 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index c7f004c1e..8387e0646 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -241,6 +241,7 @@ fn run(command: Command) -> Result { scheduler_auth, toolchain_cache_size, num_cpus: advertised_num_cpus, + colocated_reserve_fraction, }) => { let builder: Box = match builder { #[cfg(not(target_os = "freebsd"))] @@ -300,7 +301,14 @@ fn run(command: Command) -> Result { let server = Server::new(builder, &cache_dir, toolchain_cache_size) .context("Failed to create sccache server instance")?; - let advertised_num_cpus = advertised_num_cpus.unwrap_or_else(num_cpus); + let colocated = scheduler_url.to_url().host_str().map(str::to_owned) + == Some(public_addr.ip().to_string()); + let advertised_num_cpus = advertised_cores( + advertised_num_cpus, + num_cpus(), + colocated, + colocated_reserve_fraction, + ); let http_server = dist::http::Server::new( public_addr, bind_address, @@ -450,6 +458,62 @@ fn load_weight(job_count: usize, core_count: usize) -> f64 { } } +/// Default fraction of its cores a colocated build server holds back from the +/// scheduler, reserving them for the local client's preprocessing and bitbake. +const DEFAULT_COLOCATED_RESERVE_FRACTION: f64 = 0.35; + +/// Resolve the core count a build server advertises to the scheduler. An +/// explicit `num_cpus` wins; otherwise a server colocated with the scheduler +/// reserves `reserve_fraction` of its hardware (default +/// [`DEFAULT_COLOCATED_RESERVE_FRACTION`]) so the load model routes +/// proportionally more to the pure-remote nodes, while a remote server +/// advertises all its cores. Computed from the node's own `hw`, so the same +/// binary and config are portable across cluster topologies. +fn advertised_cores( + explicit: Option, + hw: usize, + colocated: bool, + reserve_fraction: Option, +) -> usize { + if let Some(n) = explicit { + return n; + } + if colocated { + let fraction = reserve_fraction + .unwrap_or(DEFAULT_COLOCATED_RESERVE_FRACTION) + .clamp(0.0, 0.9); + std::cmp::max(1, (hw as f64 * (1.0 - fraction)).round() as usize) + } else { + hw + } +} + +#[cfg(test)] +mod advertised_cores_tests { + use super::advertised_cores; + + #[test] + fn reserves_the_default_fraction_when_colocated() { + // 32 cores, colocated, default 0.35 -> round(32 * 0.65) = 21. + assert_eq!(advertised_cores(None, 32, true, None), 21); + } + + #[test] + fn a_remote_server_advertises_all_cores() { + assert_eq!(advertised_cores(None, 32, false, None), 32); + } + + #[test] + fn an_explicit_count_overrides_the_fraction() { + assert_eq!(advertised_cores(Some(16), 32, true, None), 16); + } + + #[test] + fn a_custom_fraction_is_honoured() { + assert_eq!(advertised_cores(None, 32, true, Some(0.5)), 16); + } +} + impl SchedulerIncoming for Scheduler { fn handle_alloc_job( &self, @@ -803,7 +867,7 @@ impl SchedulerIncoming for Scheduler { let mut server_details = servers.get_mut(&server_id); if let Some(ref mut details) = server_details { details.last_seen = now; - }; + } match (job_detail.state, job_state) { (JobState::Pending, JobState::Ready) => entry.get_mut().state = job_state, @@ -811,14 +875,14 @@ impl SchedulerIncoming for Scheduler { if let Some(details) = server_details { details.jobs_unclaimed.remove(&job_id); } else { - warn!("Job state updated, but server is not known to scheduler") + warn!("Job state updated, but server is not known to scheduler"); } - entry.get_mut().state = job_state + entry.get_mut().state = job_state; } (JobState::Started, JobState::Complete) => { let (job_id, _) = entry.remove_entry(); if let Some(entry) = server_details { - assert!(entry.jobs_assigned.remove(&job_id)) + assert!(entry.jobs_assigned.remove(&job_id)); } else { bail!("Job was marked as finished, but server is not known to scheduler") } diff --git a/src/config.rs b/src/config.rs index 05f5ad40d..85932479b 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1606,6 +1606,15 @@ pub mod server { // count reserves them. None = use the detected hardware count. #[serde(default)] pub num_cpus: Option, + // When this build server shares a host with the scheduler (the colocated + // orchestrator node that also runs the client's preprocessing and + // bitbake), reserve this fraction of its cores instead of advertising + // them all - computed from the node's own hardware so the same config is + // portable across cluster topologies. Defaults to 0.35 when unset; + // ignored on a pure remote server. An explicit `num_cpus` above overrides + // this with an absolute count. + #[serde(default)] + pub colocated_reserve_fraction: Option, } pub fn from_path(conf_path: &Path) -> Result> { @@ -2643,8 +2652,10 @@ fn server_toml_parse() { token: "my server's token".to_owned() }, toolchain_cache_size: 10737418240, + num_cpus: None, + colocated_reserve_fraction: None, } - ) + ); } #[test] diff --git a/tests/harness/mod.rs b/tests/harness/mod.rs index 35d8c48bf..873676085 100644 --- a/tests/harness/mod.rs +++ b/tests/harness/mod.rs @@ -230,6 +230,8 @@ fn sccache_server_cfg( token: DIST_SERVER_TOKEN.to_owned(), }, toolchain_cache_size: TC_CACHE_SIZE, + num_cpus: None, + colocated_reserve_fraction: None, } } @@ -460,6 +462,7 @@ impl DistSystem { Some(SocketAddr::from(([0, 0, 0, 0], server_addr.port()))), self.scheduler_url().to_url(), token, + sccache::util::num_cpus(), handler, ) .unwrap(); @@ -473,8 +476,11 @@ impl DistSystem { env::set_var("SCCACHE_LOG", "sccache=trace"); } env_logger::try_init().unwrap(); - server.start().unwrap(); - unreachable!(); + // start() returns Result, so it can only resolve to an + // error; surface it as a panic so the arm diverges cleanly without + // an unreachable expression for clippy to flag. + let err = server.start().unwrap_err(); + panic!("dist build server exited: {err}"); } }; @@ -499,7 +505,7 @@ impl DistSystem { panic!("restart not yet implemented for pids") } } - self.wait_server_ready(handle) + self.wait_server_ready(handle); } pub fn count_toolchains_on_server(&mut self, handle: &ServerHandle) -> usize { match handle { @@ -643,7 +649,7 @@ impl Drop for DistSystem { nix::sys::wait::waitpid(pid, Some(WaitPidFlag::WNOHANG)).map(|ws| { if ws != WaitStatus::StillAlive { killagain = false; - exits.push(ws) + exits.push(ws); } }) ); @@ -698,7 +704,7 @@ impl Drop for DistSystem { ); } for exit in exits { - println!("EXIT: {:?}", exit) + println!("EXIT: {:?}", exit); } if did_err && !thread::panicking() { @@ -742,7 +748,7 @@ fn wait_for_http(url: HTTPUrl, interval: Duration, max_wait: Duration) { }, interval, max_wait, - ) + ); } fn wait_for Result<(), String>>(f: F, interval: Duration, max_wait: Duration) { From b1bff8ab307729df9340beb61f245dd96fc247fd Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 09:32:29 -0600 Subject: [PATCH 31/43] dist: satisfy clippy under the all-features gate The dist-client and dist-server code is feature-gated, and upstream CI runs clippy on default features, so this code was never linted - it carried a batch of latent warnings that only surface with --features all,dist-server -D warnings. Add the trailing semicolons flagged by semicolon_if_nothing_returned in the heartbeat loop and the sccache-dist bin, inline a let-and-return in the rlib name parser, and allow large_enum_variant on the sccache-dist Command enum - it is built once at startup and matched once, so the size difference is irrelevant and boxing would only add indirection. No behavior change; this makes the tree clippy-clean so the new pre-commit gate passes. Signed-off-by: Javier Tia --- src/bin/sccache-dist/build.rs | 24 ++++++++++++------------ src/bin/sccache-dist/cmdline/mod.rs | 4 ++++ src/bin/sccache-dist/cmdline/parse.rs | 4 ++-- src/compiler/rust.rs | 5 ++--- src/dist/http.rs | 4 ++-- 5 files changed, 22 insertions(+), 19 deletions(-) diff --git a/src/bin/sccache-dist/build.rs b/src/bin/sccache-dist/build.rs index bfd7a980e..5f6eb94bc 100644 --- a/src/bin/sccache-dist/build.rs +++ b/src/bin/sccache-dist/build.rs @@ -163,7 +163,7 @@ impl OverlayBuilder { fn cleanup(&self) -> Result<()> { if self.dir.exists() { - fs::remove_dir_all(&self.dir).context("Failed to clean up builder directory")? + fs::remove_dir_all(&self.dir).context("Failed to clean up builder directory")?; } Ok(()) } @@ -229,7 +229,7 @@ impl OverlayBuilder { // toolchains could be the opposite of the least recently // recently used, so we clear out half of the accumulated // toolchains to prevent repeated sort/delete cycles. - entries.sort_by(|a, b| (a.1).ctime.cmp(&(b.1).ctime)); + entries.sort_by_key(|a| (a.1).ctime); entries.truncate(entries.len() / 2); for (tc, _) in entries { warn!("Removing old un-compressed toolchain: {:?}", tc); @@ -306,7 +306,7 @@ impl OverlayBuilder { // This error is unfortunately not Send+Sync ) .mount() - .map_err(|e| anyhow!("Failed to mount overlay FS: {}", e.to_string()))?; + .map_err(|e| anyhow!("Failed to mount overlay FS: {}", e))?; trace!("copying in inputs"); // Note that we don't unpack directly into the upperdir since there overlayfs has some @@ -393,11 +393,11 @@ impl OverlayBuilder { Ok(file) => { let output = OutputData::try_from_reader(file) .context("Failed to read output file")?; - outputs.push((path, output)) + outputs.push((path, output)); } Err(e) => { if e.kind() == io::ErrorKind::NotFound { - debug!("Missing output path {:?}", path) + debug!("Missing output path {:?}", path); } else { return Err( Error::from(e).context("Failed to open output file") @@ -525,7 +525,7 @@ impl DockerBuilder { bail!("Malformed container listing - third field on row") } if image_name.starts_with("sccache-builder-") { - containers_to_rm.push(container_id) + containers_to_rm.push(container_id); } } if !containers_to_rm.is_empty() { @@ -555,7 +555,7 @@ impl DockerBuilder { bail!("Malformed image listing - third field on row") } if image_name.starts_with("sccache-builder-") { - images_to_rm.push(image_id) + images_to_rm.push(image_id); } } if !images_to_rm.is_empty() { @@ -563,7 +563,7 @@ impl DockerBuilder { .args(["rmi"]) .args(images_to_rm) .check_run() - .context("Failed to remove image")? + .context("Failed to remove image")?; } } @@ -647,7 +647,7 @@ impl DockerBuilder { .check_run() { // We do a final check anyway, so just continue - warn!("Failed to remove added path in a container: {}", e) + warn!("Failed to remove added path in a container: {}", e); } } @@ -683,7 +683,7 @@ impl DockerBuilder { // Good as new, add it back to the container list if let Some(entry) = self.container_lists.lock().unwrap().get_mut(tc) { debug!("Reclaimed container {}", cid); - entry.push(cid) + entry.push(cid); } else { warn!( "Was ready to reclaim container {} but toolchain went missing", @@ -835,9 +835,9 @@ impl DockerBuilder { if output.status.success() { let output = OutputData::try_from_reader(&*output.stdout) .expect("Failed to read compress output stdout"); - outputs.push((path, output)) + outputs.push((path, output)); } else { - debug!("Missing output path {:?}", path) + debug!("Missing output path {:?}", path); } } diff --git a/src/bin/sccache-dist/cmdline/mod.rs b/src/bin/sccache-dist/cmdline/mod.rs index fafe3902c..82ccdbb5e 100644 --- a/src/bin/sccache-dist/cmdline/mod.rs +++ b/src/bin/sccache-dist/cmdline/mod.rs @@ -19,6 +19,10 @@ mod parse; pub use parse::try_parse_from; +// The Scheduler/Server variants carry the full parsed daemon config; this enum +// is constructed once at startup and matched once, so the size difference is +// irrelevant and boxing the variants would only add indirection. +#[allow(clippy::large_enum_variant)] #[derive(Debug)] pub enum Command { Auth(AuthSubcommand), diff --git a/src/bin/sccache-dist/cmdline/parse.rs b/src/bin/sccache-dist/cmdline/parse.rs index 4ead7131a..68e053250 100644 --- a/src/bin/sccache-dist/cmdline/parse.rs +++ b/src/bin/sccache-dist/cmdline/parse.rs @@ -207,7 +207,7 @@ pub fn try_parse_from( matches .get_one::("secret-key") .expect("`secret-key` is required") - .to_string() + .clone() }; AuthSubcommand::JwtHS256ServerToken { @@ -282,7 +282,7 @@ mod tests { #[test] fn debug_assert() { - get_clap_command().debug_assert() + get_clap_command().debug_assert(); } #[test] diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 27d571a57..877ec07ca 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -2254,7 +2254,7 @@ impl pkg::InputsPackager for RustInputsPackager { Some(name) => { let mut rev_name_split = name.rsplitn(2, '-'); let _extra_filename_and_ext = rev_name_split.next(); - let libname = if let Some(libname) = rev_name_split.next() { + if let Some(libname) = rev_name_split.next() { assert!(rev_name_split.next().is_none()); libname } else { @@ -2262,8 +2262,7 @@ impl pkg::InputsPackager for RustInputsPackager { // libstd.rlib): fall back to the name with its // extension stripped so std still ships. name.rsplit_once('.').map(|(stem, _)| stem).unwrap_or(name) - }; - libname + } } None => continue, }; diff --git a/src/dist/http.rs b/src/dist/http.rs index a66092d90..015cc31df 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -1031,11 +1031,11 @@ mod server { if is_new { info!("Server connected to scheduler"); } - thread::sleep(HEARTBEAT_INTERVAL) + thread::sleep(HEARTBEAT_INTERVAL); } Err(e) => { error!(target: "sccache_heartbeat", "Failed to send heartbeat to server: {}", e); - thread::sleep(HEARTBEAT_ERROR_INTERVAL) + thread::sleep(HEARTBEAT_ERROR_INTERVAL); } } } From fee4f205e937b7b62969a38041d8f6e2158b96e0 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 09:33:58 -0600 Subject: [PATCH 32/43] ci: mirror the CI validation gate in pre-commit The pre-commit config only ran nightly fmt and clippy with no failure gate, so a break reached CI before anyone noticed - a changed function signature or struct literal in tests/ slipped through because plain cargo check and makepkg never compile the integration tests. Add the CI gate locally: clippy with the same -D warnings and allows CI sets, but --features all,dist-server so the feature-gated dist code is actually linted (CI's clippy runs default features and never covers it), and cargo check --all-targets so the test harness is type-checked at commit time. Move cargo test and cargo audit to pre-push so per-commit turnaround stays fast. fmt stays on nightly to match the tree's existing formatting. Signed-off-by: Javier Tia --- .pre-commit-config.yaml | 55 +++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f90466bed..993437503 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,17 +1,56 @@ repos: -- repo: local + - repo: local hooks: - - id: rust-linting - name: Rust linting - description: Run cargo fmt on files included in the commit. + # Format changed Rust files. Kept on nightly because the tree is + # nightly-formatted; stable fmt would reflow files needlessly. + - id: rust-fmt + name: cargo fmt + description: Format Rust sources included in the commit. entry: cargo +nightly fmt -- pass_filenames: true types: [file, rust] language: system - - id: rust-clippy - name: Rust clippy - description: Run cargo clippy on files included in the commit. - entry: cargo +nightly clippy --workspace --all-targets --all-features -- + + # Lint with the gate CI enforces (.github/workflows/ci.yml): -D warnings + # plus the three allows CI sets. --features all,dist-server so the + # feature-gated dist-client / dist-server code is actually linted - CI's + # clippy runs on default features and never covers it. + - id: rust-clippy + name: cargo clippy + description: Lint all targets with warnings denied. + entry: cargo clippy --locked --all-targets --features all,dist-server -- -D warnings -A unknown-lints -A clippy::type_complexity -A clippy::new-without-default pass_filenames: false types: [file, rust] language: system + + # Compile every target including the integration-test harness. This is the + # gate that catches a changed function signature or struct literal under + # tests/ that plain `cargo check` and `makepkg` never compile. + - id: rust-check + name: cargo check --all-targets + description: Type-check every target, including tests and benches. + entry: cargo check --all-targets --features all,dist-server + pass_filenames: false + types: [file, rust] + language: system + + # Run the CI test set on push (lib, bins, integration tests). On push, not + # per commit, so commit turnaround stays fast. + - id: rust-test + name: cargo test + description: Run the library, binary, and integration tests. + entry: cargo test --locked --features all,dist-server --lib --bins --tests + pass_filenames: false + types: [file, rust] + language: system + stages: [pre-push] + + # RustSec advisory scan on push (org security standard). + - id: cargo-audit + name: cargo audit + description: Scan dependencies for known advisories. + entry: cargo audit + pass_filenames: false + types: [file, rust] + language: system + stages: [pre-push] From 7cd5141928900f98471284912814b4900c3785c2 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 11:27:29 -0600 Subject: [PATCH 33/43] dist/scheduler: reap Started jobs whose Complete update was lost A distributed job that reached the Started state but never reported Complete leaked its jobs_assigned slot on the owning server. The Ready->Started transition removes the job from jobs_unclaimed, so the unclaimed reaper (which handles only Ready/Pending) can never see it again, and prune_servers reclaims jobs only from a server gone silent past the heartbeat timeout. On a live, still-heartbeating server these stuck-Started jobs accumulate until jobs_assigned reaches the admission ceiling, at which point load_weight returns MAX+1 and the scheduler stops routing to the node. Measured on the two-node cluster: the remote server pinned at 37/37 in-progress while idle (load 0.15, 0 cc1), its completion updates lost while clients backed up on the colocated node. Stamp each job with the time it entered its current state and, on every heartbeat, de-allocate any Started job the server has held past STARTED_COMPLETE_TIMEOUT (180s) - far longer than a real remote single-TU compile (~60s for a heavy LLVM object) yet short enough to reclaim a leaked slot briskly. Reaping only the scheduler's accounting never aborts the client's in-flight compile, which runs over a separate client-server channel, so an over-eager reap at worst mildly oversubscribes, which admission_ceiling already tolerates. This mirrors the prune_servers (dead servers) and unclaimed (stuck Ready/Pending) backstops already in the scheduler. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 174 ++++++++++++++++++++++++++++++++++- 1 file changed, 170 insertions(+), 4 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 8387e0646..9e8dcc10a 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -355,11 +355,29 @@ const SERVER_REMEMBER_ERROR_TIMEOUT: Duration = Duration::from_secs(300); const RECENT_ERROR_LOAD_PENALTY: f64 = 0.125; const UNCLAIMED_PENDING_TIMEOUT: Duration = Duration::from_secs(300); const UNCLAIMED_READY_TIMEOUT: Duration = Duration::from_secs(60); +// A job that entered Started but never reported Complete leaks its +// jobs_assigned slot: the Ready->Started transition drops it from +// jobs_unclaimed, so the unclaimed reaper can never see it again, and +// prune_servers only reclaims a DEAD server's jobs. On a live server, such +// jobs accumulate until jobs_assigned hits admission_ceiling and load_weight +// stops routing to the node - the measured PC2-idle-at-37/37 lockout, its +// completion updates lost while it sat pinned. Reap a Started job the owning +// server has not completed within this timeout, mirroring prune_servers (dead +// servers) and the unclaimed reaper (stuck Ready/Pending). The value is far +// longer than any real remote single-TU compile (a heavy LLVM object is ~60s) +// yet short enough to reclaim a leaked slot briskly. Reaping only the +// scheduler's accounting never aborts the client's in-flight compile - that +// runs over a separate client<->server channel - so an over-eager reap at +// worst mildly oversubscribes, which admission_ceiling already tolerates. +const STARTED_COMPLETE_TIMEOUT: Duration = Duration::from_secs(180); #[derive(Copy, Clone)] struct JobDetail { server_id: ServerId, state: JobState, + // When the job entered its current state; used to reap Started jobs whose + // Complete update was lost (see STARTED_COMPLETE_TIMEOUT). + since: Instant, } // To avoid deadlicking, make sure to do all locking at once (i.e. no further locking in a downward scope), @@ -699,8 +717,15 @@ impl SchedulerIncoming for Scheduler { job_id, state ); assert!( - jobs.insert(job_id, JobDetail { server_id, state }) - .is_none() + jobs.insert( + job_id, + JobDetail { + server_id, + state, + since: Instant::now(), + } + ) + .is_none() ); } let job_alloc = JobAlloc { @@ -792,6 +817,32 @@ impl SchedulerIncoming for Scheduler { } } + // Backstop for jobs stuck in Started (see STARTED_COMPLETE_TIMEOUT): + // the unclaimed sweep above cannot see them because Ready->Started + // drops them from jobs_unclaimed. Reap any this server has held in + // Started past the timeout so a lost Complete update cannot leak the + // slot forever and eventually lock the node out at its ceiling. + let mut stale_started = Vec::new(); + for &job_id in details.jobs_assigned.iter() { + if let Some(detail) = jobs.get(&job_id) { + if detail.state == JobState::Started + && now.duration_since(detail.since) > STARTED_COMPLETE_TIMEOUT + { + stale_started.push(job_id); + } + } + } + if !stale_started.is_empty() { + warn!( + "The following Started jobs had no Complete within {:?} and will be de-allocated: {:?}", + STARTED_COMPLETE_TIMEOUT, stale_started + ); + for job_id in stale_started { + details.jobs_assigned.remove(&job_id); + jobs.remove(&job_id); + } + } + return Ok(HeartbeatServerResult { is_new: false }); } Some(ref mut details) if details.server_nonce != server_nonce => { @@ -870,14 +921,20 @@ impl SchedulerIncoming for Scheduler { } match (job_detail.state, job_state) { - (JobState::Pending, JobState::Ready) => entry.get_mut().state = job_state, + (JobState::Pending, JobState::Ready) => { + let detail = entry.get_mut(); + detail.state = job_state; + detail.since = now; + } (JobState::Ready, JobState::Started) => { if let Some(details) = server_details { details.jobs_unclaimed.remove(&job_id); } else { warn!("Job state updated, but server is not known to scheduler"); } - entry.get_mut().state = job_state; + let detail = entry.get_mut(); + detail.state = job_state; + detail.since = now; } (JobState::Started, JobState::Complete) => { let (job_id, _) = entry.remove_entry(); @@ -1237,4 +1294,113 @@ mod scheduler_tests { _ => panic!("expected a successful allocation"), } } + + // Register `job_id` on `server_id` as an in-flight Started job whose state + // was entered `age` ago, exactly as it sits after a Ready->Started update: + // present in jobs_assigned and the jobs map, absent from jobs_unclaimed. + fn insert_started_job( + scheduler: &Scheduler, + server_id: ServerId, + job_id: JobId, + age: Duration, + ) { + let mut servers = scheduler.servers.lock().unwrap(); + let mut jobs_assigned = HashSet::new(); + jobs_assigned.insert(job_id); + servers.insert( + server_id, + ServerDetails { + last_seen: Instant::now(), + last_error: None, + jobs_assigned, + jobs_unclaimed: HashMap::new(), + num_cpus: 32, + server_nonce: ServerNonce::new(), + job_authorizer: Box::new(NoopAuthorizer), + }, + ); + let mut jobs = scheduler.jobs.lock().unwrap(); + jobs.insert( + job_id, + JobDetail { + server_id, + state: JobState::Started, + since: Instant::now() - age, + }, + ); + } + + // A job stuck in Started (its Complete update lost) must be reaped on the + // owning server's next heartbeat, or its jobs_assigned slot leaks until the + // node hits admission_ceiling and load_weight locks it out - the measured + // PC2-idle-at-37/37 failure. prune_servers reclaims only dead servers and + // the unclaimed reaper sees only Ready/Pending, so without a Started + // backstop the slot never frees on a live server. Falsifier: dropping the + // Started sweep leaves the job assigned after the heartbeat. + #[test] + fn test_heartbeat_reaps_started_job_past_completion_timeout() { + let scheduler = Scheduler::new(); + let server_id = ServerId::new("10.42.0.2:10501".parse::().unwrap()); + let job_id = JobId(1000); + insert_started_job( + &scheduler, + server_id, + job_id, + STARTED_COMPLETE_TIMEOUT + Duration::from_secs(10), + ); + let nonce = scheduler + .servers + .lock() + .unwrap() + .get(&server_id) + .unwrap() + .server_nonce + .clone(); + + scheduler + .handle_heartbeat_server(server_id, nonce, 32, Box::new(NoopAuthorizer)) + .expect("handle_heartbeat_server should not error"); + + let servers = scheduler.servers.lock().unwrap(); + let details = servers.get(&server_id).expect("server still registered"); + assert!( + !details.jobs_assigned.contains(&job_id), + "a Started job past the completion timeout must be de-allocated from jobs_assigned" + ); + let jobs = scheduler.jobs.lock().unwrap(); + assert!( + !jobs.contains_key(&job_id), + "the stale Started job must be removed from the jobs map" + ); + } + + // Guard against over-reaping: a Started job within the timeout is a compile + // in flight, so it must stay assigned and keep counting toward the server's + // load. + #[test] + fn test_heartbeat_keeps_started_job_within_completion_timeout() { + let scheduler = Scheduler::new(); + let server_id = ServerId::new("10.42.0.2:10501".parse::().unwrap()); + let job_id = JobId(1000); + insert_started_job(&scheduler, server_id, job_id, Duration::from_secs(0)); + let nonce = scheduler + .servers + .lock() + .unwrap() + .get(&server_id) + .unwrap() + .server_nonce + .clone(); + + scheduler + .handle_heartbeat_server(server_id, nonce, 32, Box::new(NoopAuthorizer)) + .expect("handle_heartbeat_server should not error"); + + let servers = scheduler.servers.lock().unwrap(); + let details = servers.get(&server_id).expect("server still registered"); + assert!( + details.jobs_assigned.contains(&job_id), + "a Started job within the completion timeout must stay assigned" + ); + } } From 387c73a88bf85b02fad07c9d42fe8599da62ac5f Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 13:04:53 -0600 Subject: [PATCH 34/43] dist/scheduler: add a between-nodes job-lifecycle leak detector Diagnosing scheduler slot leaks meant a rebuild-with-debug-logging cycle each time, and the per-compile debug logging that surfaced the remote errors itself dragged the client daemon under load. There was no cheap, always-on signal to tell a feed/unclaimed leak (jobs allocated to a node but never claimed) apart from a lost-completion leak (jobs that reached Started but never reported Complete), nor to confirm the accounting is balanced at all. Add five cumulative atomic counters - started, completed, reaped_unclaimed, reaped_started, pruned - incremented at their lifecycle transitions, and emit one dist-accounting summary line per heartbeat (roughly every 30s per server, not per compile, so it adds no hot-path cost). A rising allocated-minus-started-minus-reaped_unclaimed isolates a feed/unclaimed leak; a rising started-minus-completed-minus-reaped_started isolates a lost-completion leak. The counters use Relaxed ordering since they are diagnostic, not synchronization. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 51 ++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 9e8dcc10a..108f7ce7f 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -389,6 +389,20 @@ pub struct Scheduler { jobs: Mutex>, servers: Mutex>, + + // Cumulative job-lifecycle counters for the between-nodes leak detector + // logged once per heartbeat (see the dist-accounting line in + // handle_heartbeat_server). Relaxed: diagnostic counters, not + // synchronization. Two leak modes are separable from the balance: + // allocated - started - reaped_unclaimed - (unclaimed jobs still live) + // drifting up => a feed/unclaimed leak (jobs allocated, never claimed). + // started - completed - reaped_started - (started jobs still live) + // drifting up => a lost-completion leak (the Leak #1 class). + jobs_started: AtomicUsize, + jobs_completed: AtomicUsize, + jobs_reaped_unclaimed: AtomicUsize, + jobs_reaped_started: AtomicUsize, + jobs_pruned: AtomicUsize, } struct ServerDetails { @@ -409,6 +423,11 @@ impl Scheduler { job_count: AtomicUsize::new(0), jobs: Mutex::new(BTreeMap::new()), servers: Mutex::new(HashMap::new()), + jobs_started: AtomicUsize::new(0), + jobs_completed: AtomicUsize::new(0), + jobs_reaped_unclaimed: AtomicUsize::new(0), + jobs_reaped_started: AtomicUsize::new(0), + jobs_pruned: AtomicUsize::new(0), } } @@ -435,6 +454,8 @@ impl Scheduler { let server_details = servers .remove(&server_id) .expect("server went missing from map"); + self.jobs_pruned + .fetch_add(server_details.jobs_assigned.len(), Ordering::Relaxed); for job_id in server_details.jobs_assigned { warn!( "Non-terminated job {} was cleaned up in server pruning", @@ -791,6 +812,8 @@ impl SchedulerIncoming for Scheduler { "The following stale jobs will be de-allocated: {:?}", stale_jobs ); + self.jobs_reaped_unclaimed + .fetch_add(stale_jobs.len(), Ordering::Relaxed); for job_id in stale_jobs { if !details.jobs_assigned.remove(&job_id) { @@ -837,12 +860,31 @@ impl SchedulerIncoming for Scheduler { "The following Started jobs had no Complete within {:?} and will be de-allocated: {:?}", STARTED_COMPLETE_TIMEOUT, stale_started ); + self.jobs_reaped_started + .fetch_add(stale_started.len(), Ordering::Relaxed); for job_id in stale_started { details.jobs_assigned.remove(&job_id); jobs.remove(&job_id); } } + // Between-nodes leak detector: one cumulative accounting line + // per heartbeat (cheap - ~per-30s per server, NOT per compile). + // A rising (allocated - started - reaped_unclaimed - unclaimed + // still live) points to a feed/unclaimed leak; a rising (started + // - completed - reaped_started - started still live) points to a + // lost-completion leak. `live` = current jobs-map size. + info!( + "dist-accounting: allocated={} started={} completed={} reaped_unclaimed={} reaped_started={} pruned={} live={}", + self.job_count.load(Ordering::Relaxed), + self.jobs_started.load(Ordering::Relaxed), + self.jobs_completed.load(Ordering::Relaxed), + self.jobs_reaped_unclaimed.load(Ordering::Relaxed), + self.jobs_reaped_started.load(Ordering::Relaxed), + self.jobs_pruned.load(Ordering::Relaxed), + jobs.len(), + ); + return Ok(HeartbeatServerResult { is_new: false }); } Some(ref mut details) if details.server_nonce != server_nonce => { @@ -887,6 +929,8 @@ impl SchedulerIncoming for Scheduler { ); // Drop any jobs the departing server still held, as prune_servers // does, so the scheduler does not track work no node will finish. + self.jobs_pruned + .fetch_add(details.jobs_assigned.len(), Ordering::Relaxed); for job_id in details.jobs_assigned { jobs.remove(&job_id); } @@ -935,6 +979,7 @@ impl SchedulerIncoming for Scheduler { let detail = entry.get_mut(); detail.state = job_state; detail.since = now; + self.jobs_started.fetch_add(1, Ordering::Relaxed); } (JobState::Started, JobState::Complete) => { let (job_id, _) = entry.remove_entry(); @@ -943,6 +988,7 @@ impl SchedulerIncoming for Scheduler { } else { bail!("Job was marked as finished, but server is not known to scheduler") } + self.jobs_completed.fetch_add(1, Ordering::Relaxed); } (from, to) => bail!("Invalid job state transition from {} to {}", from, to), } @@ -1372,6 +1418,11 @@ mod scheduler_tests { !jobs.contains_key(&job_id), "the stale Started job must be removed from the jobs map" ); + assert_eq!( + scheduler.jobs_reaped_started.load(Ordering::Relaxed), + 1, + "reaping a stale Started job must increment the leak-detector counter" + ); } // Guard against over-reaping: a Started job within the timeout is a compile From 3ef8ec0f09a0cb0f6c45d9ba18e89d717f2a4dd6 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 13:40:51 -0600 Subject: [PATCH 35/43] dist/scheduler: make late job-state updates idempotent The 180s Started-job reaper can discard a successful compile: it reclaims the slot at the timeout, then the build server's run_job propagates a Complete update for that job, handle_update_job_state hits bail!("Unknown job"), run_job returns 500, and the client throws away the finished object files and recompiles locally. Any translation unit that legitimately compiles for longer than the timeout on a loaded remote node therefore fails at the finish line and is strictly worse off than not distributing. Treat a Started or Complete update for a job the scheduler no longer tracks as a logged no-op success rather than an error, and stop failing the build server's run_job when its terminal Complete update is rejected (log and return the results anyway). This makes the reaper safe for a compile of any duration: a reaped-then-completed job still delivers its objects to the client. A non-terminal update for an unknown job still warns. Covered by test_late_update_for_reaped_job_is_ok. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 52 +++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 4 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 108f7ce7f..e2ce3ee0d 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -994,7 +994,23 @@ impl SchedulerIncoming for Scheduler { } info!("Job {} updated state to {:?}", job_id, job_state); } else { - bail!("Unknown job") + // Idempotent late update: a Started/Complete update for a job the + // scheduler no longer tracks (already reaped, or reclaimed by a + // liveness sweep) is a logged no-op success, never an error. Erroring + // here fails the server's run_job (which propagates the terminal + // update), so the client would discard finished objects and recompile + // locally - the reap-then-complete hazard. + match job_state { + JobState::Started | JobState::Complete => { + info!( + "Late {:?} update for untracked job {} (already reaped); no-op success", + job_state, job_id + ); + } + other => { + warn!("State update to {:?} for unknown job {}", other, job_id); + } + } } Ok(UpdateJobStateResult::Success) } @@ -1131,9 +1147,16 @@ impl ServerIncoming for Server { } } }; - requester - .do_update_job_state(job_id, JobState::Complete) - .context("Updating job state failed")?; + if let Err(e) = requester.do_update_job_state(job_id, JobState::Complete) { + // Do not fail the compile because the scheduler rejected the terminal + // update (e.g. the job was reaped mid-compile): the objects are built + // and must reach the client rather than being discarded for a local + // recompile. + warn!( + "Completing job {} with the scheduler failed ({:#}); returning results anyway", + job_id, e + ); + } res } } @@ -1454,4 +1477,25 @@ mod scheduler_tests { "a Started job within the completion timeout must stay assigned" ); } + + // A late Complete (or Started) update for a job the scheduler no longer + // tracks must be a no-op success, not an error. If it errors, the build + // server's run_job (which propagates the terminal update) fails, and the + // client discards the finished objects and recompiles locally - the + // reap-then-complete hazard that a fixed-timeout reaper can trigger on a + // slow-but-successful remote compile. + #[test] + fn test_late_update_for_reaped_job_is_ok() { + let scheduler = Scheduler::new(); + let server_id = ServerId::new("10.42.0.2:10501".parse::().unwrap()); + + // No job was ever inserted, so job 1000 is "unknown" to the scheduler, + // exactly as it would be after a reap. + let res = scheduler.handle_update_job_state(JobId(1000), server_id, JobState::Complete); + + assert!( + res.is_ok(), + "a Complete update for an untracked (reaped) job must be a logged no-op success, not an error" + ); + } } From 532a7a9fa8c017aa14f1773ae1f71eae75f33cd0 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 13:46:54 -0600 Subject: [PATCH 36/43] dist/scheduler: measure the unclaimed reservation timeout from Ready The unclaimed-reservation reaper timed a job out from its allocation instant, so the toolchain-upload window (the Pending phase) ate into the claim budget: a job whose toolchain was still uploading could be reaped as a no-show before it ever became claimable, even though nothing was wrong. Reset the jobs_unclaimed timestamp when the job transitions Pending to Ready, so the timeout measures only the time a claimable job sits unclaimed. A job still uploading its toolchain is no longer counted against the reservation TTL. Covered by test_pending_to_ready_resets_unclaimed_clock. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 65 ++++++++++++++++++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index e2ce3ee0d..237570b0c 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -966,6 +966,15 @@ impl SchedulerIncoming for Scheduler { match (job_detail.state, job_state) { (JobState::Pending, JobState::Ready) => { + // Reset the unclaimed-reservation clock to Ready time rather + // than allocation time, so time spent uploading a toolchain + // (Pending) does not consume the claim budget and a job that + // is still uploading is not reaped as a no-show. + if let Some(details) = server_details { + if let Some(t) = details.jobs_unclaimed.get_mut(&job_id) { + *t = now; + } + } let detail = entry.get_mut(); detail.state = job_state; detail.since = now; @@ -1498,4 +1507,60 @@ mod scheduler_tests { "a Complete update for an untracked (reaped) job must be a logged no-op success, not an error" ); } + + // The unclaimed-reservation clock must be reset when a job becomes claimable + // (Pending->Ready), so time spent uploading a toolchain does not eat the + // claim budget and cause a no-show reap of a job that is still arriving. + #[test] + fn test_pending_to_ready_resets_unclaimed_clock() { + let scheduler = Scheduler::new(); + let server_id = ServerId::new("10.42.0.2:10501".parse::().unwrap()); + let job_id = JobId(1000); + let old = Instant::now() - Duration::from_secs(120); + { + let mut servers = scheduler.servers.lock().unwrap(); + let mut jobs_unclaimed = HashMap::new(); + jobs_unclaimed.insert(job_id, old); + let mut jobs_assigned = HashSet::new(); + jobs_assigned.insert(job_id); + servers.insert( + server_id, + ServerDetails { + last_seen: Instant::now(), + last_error: None, + jobs_assigned, + jobs_unclaimed, + num_cpus: 32, + server_nonce: ServerNonce::new(), + job_authorizer: Box::new(NoopAuthorizer), + }, + ); + let mut jobs = scheduler.jobs.lock().unwrap(); + jobs.insert( + job_id, + JobDetail { + server_id, + state: JobState::Pending, + since: old, + }, + ); + } + + scheduler + .handle_update_job_state(job_id, server_id, JobState::Ready) + .expect("Pending->Ready update should succeed"); + + let servers = scheduler.servers.lock().unwrap(); + let t = servers + .get(&server_id) + .unwrap() + .jobs_unclaimed + .get(&job_id) + .copied() + .expect("job must still be unclaimed after reaching Ready"); + assert!( + t.elapsed() < Duration::from_secs(60), + "the unclaimed clock must be reset to Ready time, not left at the ~120s-old allocation time" + ); + } } From 3e926bb6a76587d22816e419cf39c7beddc3b09b Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 13:50:27 -0600 Subject: [PATCH 37/43] dist: make scheduler restart safe for job accounting A scheduler restart reminted job ids from zero while build servers kept running jobs assigned before the restart. The reused id collided with the survivor's stale job_toolchains entry, and handle_assign_job asserted the insert returned no prior value, so the rouille worker panicked and the server 500'd. The per-job toolchain map was also only ever pruned in handle_run_job, so an assign whose run_job never arrived leaked its entry forever. Seed the scheduler's job counter from wall-clock millis so a restart does not restart ids at zero; change the assign-time assert to a warn-and- overwrite so a residual collision degrades gracefully; and timestamp each job_toolchains entry, sweeping entries older than ABANDONED_TOOLCHAIN_TIMEOUT on each assign so abandoned entries cannot accumulate. Covered by test_job_count_seeded_nonzero. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 68 ++++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 11 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 237570b0c..d4c83167b 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -370,6 +370,11 @@ const UNCLAIMED_READY_TIMEOUT: Duration = Duration::from_secs(60); // runs over a separate client<->server channel - so an over-eager reap at // worst mildly oversubscribes, which admission_ceiling already tolerates. const STARTED_COMPLETE_TIMEOUT: Duration = Duration::from_secs(180); +// A build server drops its per-job toolchain entry in handle_run_job. An +// assign whose run_job never arrives (client crash, scheduler restart) would +// otherwise leak the entry forever, so sweep entries older than this on each +// assign. Far longer than a legitimate assign->run_job gap. +const ABANDONED_TOOLCHAIN_TIMEOUT: Duration = Duration::from_secs(600); #[derive(Copy, Clone)] struct JobDetail { @@ -420,7 +425,15 @@ struct ServerDetails { impl Scheduler { pub fn new() -> Self { Scheduler { - job_count: AtomicUsize::new(0), + // Seed from wall-clock millis so a scheduler restart does not remint + // job ids from 0 and collide with an id a surviving build server + // still holds (which would trip the assign-time overwrite path). + job_count: AtomicUsize::new( + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as usize) + .unwrap_or(0), + ), jobs: Mutex::new(BTreeMap::new()), servers: Mutex::new(HashMap::new()), jobs_started: AtomicUsize::new(0), @@ -1062,7 +1075,7 @@ impl SchedulerIncoming for Scheduler { pub struct Server { builder: Box, cache: Mutex, - job_toolchains: Mutex>, + job_toolchains: Mutex>, } impl Server { @@ -1084,13 +1097,23 @@ impl Server { impl ServerIncoming for Server { fn handle_assign_job(&self, job_id: JobId, tc: Toolchain) -> Result { let need_toolchain = !self.cache.lock().unwrap().contains_toolchain(&tc); - assert!( - self.job_toolchains - .lock() - .unwrap() - .insert(job_id, tc) - .is_none() - ); + { + let now = Instant::now(); + let mut toolchains = self.job_toolchains.lock().unwrap(); + // GC abandoned entries (assigns whose run_job never arrived) so the + // map cannot grow without bound. + toolchains + .retain(|_, (_, since)| now.duration_since(*since) < ABANDONED_TOOLCHAIN_TIMEOUT); + // Overwrite rather than assert: after a scheduler restart the job + // counter reseeds and a surviving server can still hold a stale entry + // for a reused id. Warn and overwrite instead of panicking the worker. + if toolchains.insert(job_id, (tc, now)).is_some() { + warn!( + "Overwrote a stale toolchain entry for reused job id {}", + job_id + ); + } + } let state = if need_toolchain { JobState::Pending } else { @@ -1113,7 +1136,13 @@ impl ServerIncoming for Server { .context("Updating job state failed")?; // TODO: need to lock the toolchain until the container has started // TODO: can start prepping container - let tc = match self.job_toolchains.lock().unwrap().get(&job_id).cloned() { + let tc = match self + .job_toolchains + .lock() + .unwrap() + .get(&job_id) + .map(|(tc, _)| tc.clone()) + { Some(tc) => tc, None => return Ok(SubmitToolchainResult::JobNotFound), }; @@ -1140,7 +1169,12 @@ impl ServerIncoming for Server { requester .do_update_job_state(job_id, JobState::Started) .context("Updating job state failed")?; - let tc = self.job_toolchains.lock().unwrap().remove(&job_id); + let tc = self + .job_toolchains + .lock() + .unwrap() + .remove(&job_id) + .map(|(tc, _)| tc); let res = match tc { None => Ok(RunJobResult::JobNotFound), Some(tc) => { @@ -1563,4 +1597,16 @@ mod scheduler_tests { "the unclaimed clock must be reset to Ready time, not left at the ~120s-old allocation time" ); } + + // A scheduler restart must not remint job ids from 0: a new job N would then + // collide with an id a surviving build server still holds, tripping the + // assign-time overwrite (previously an assert-panic). + #[test] + fn test_job_count_seeded_nonzero() { + let scheduler = Scheduler::new(); + assert!( + scheduler.job_count.load(Ordering::Relaxed) > 0, + "job_count must be seeded from a non-zero base so a restart does not reuse ids starting at 0" + ); + } } From 771552de970c03f8350b50f55ec641cb8d22d960 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 14:13:52 -0600 Subject: [PATCH 38/43] dist: reap Started jobs by liveness lease, not a fixed timeout The scheduler reaped a Started job purely on a fixed 180s timeout, so a genuinely-slow remote compile that ran longer than the timeout had its slot reclaimed while it was still running - the very case the reaper was meant to leave alone. There was no signal distinguishing a live long compile from a job whose completion was lost. Have each build server report the ids it is actively running in every heartbeat (a running-jobs set maintained by an RAII guard around handle_run_job, snapshotted into the heartbeat request each beat), and reap a Started job only once it is absent from two consecutive heartbeats of the owning server. A single dropped or racing heartbeat cannot reap a live job, and a compile the server keeps naming survives at any duration. The fixed timeout stays only as a backstop for a server that stops heartbeating its running set, and it too now skips any job named in the latest beat. This adds an active_jobs field to the heartbeat request, changing the bincode wire format: scheduler and all build servers must run the matching binary. Covered by test_liveness_lease_reap. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 187 ++++++++++++++++++++++++++++++++++- src/dist/http.rs | 38 +++++-- src/dist/mod.rs | 5 + tests/dist.rs | 3 + 4 files changed, 221 insertions(+), 12 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index d4c83167b..7fa71853f 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -415,6 +415,11 @@ struct ServerDetails { // Jobs assigned that haven't seen a state change. Can only be pending // or ready. jobs_unclaimed: HashMap, + // Job ids this server reported running in its previous heartbeat. A Started + // job is reaped only when it is absent from both this set and the incoming + // heartbeat's active_jobs (two consecutive misses), so a single dropped or + // racing heartbeat never reaps a live compile. + last_active_jobs: HashSet, last_seen: Instant, last_error: Option, num_cpus: usize, @@ -779,6 +784,7 @@ impl SchedulerIncoming for Scheduler { server_nonce: ServerNonce, num_cpus: usize, job_authorizer: Box, + active_jobs: Vec, ) -> Result { if num_cpus == 0 { bail!("Invalid number of CPUs (0) specified in heartbeat") @@ -853,15 +859,55 @@ impl SchedulerIncoming for Scheduler { } } + // Primary Started-reap mechanism: the liveness lease. Every + // heartbeat carries the ids the server is actively running. + // Reap a Started job only once it is absent from BOTH the + // incoming active_jobs AND last_active_jobs (the prior beat) - + // two consecutive misses - so a genuinely-slow live compile, + // which the server reports every beat, is never reaped, and one + // dropped or racing heartbeat cannot reap a live job. The + // fixed-timeout sweep below stays only as a backstop for a + // server that stops heartbeating its running set entirely. + let active_set: HashSet = active_jobs.into_iter().collect(); + let mut liveness_reap = Vec::new(); + for &job_id in details.jobs_assigned.iter() { + if let Some(detail) = jobs.get(&job_id) { + if detail.state == JobState::Started + && !active_set.contains(&job_id) + && !details.last_active_jobs.contains(&job_id) + { + liveness_reap.push(job_id); + } + } + } + if !liveness_reap.is_empty() { + warn!( + "The following Started jobs were absent from two consecutive heartbeats and will be de-allocated: {:?}", + liveness_reap + ); + self.jobs_reaped_started + .fetch_add(liveness_reap.len(), Ordering::Relaxed); + for job_id in liveness_reap { + details.jobs_assigned.remove(&job_id); + jobs.remove(&job_id); + } + } + details.last_active_jobs = active_set; + // Backstop for jobs stuck in Started (see STARTED_COMPLETE_TIMEOUT): // the unclaimed sweep above cannot see them because Ready->Started // drops them from jobs_unclaimed. Reap any this server has held in // Started past the timeout so a lost Complete update cannot leak the // slot forever and eventually lock the node out at its ceiling. + // Never reap a job the server just reported active (now stored in + // last_active_jobs): a genuinely-slow live compile must survive at + // any duration, so the backstop only fires once the server has + // stopped naming the job in its heartbeats. let mut stale_started = Vec::new(); for &job_id in details.jobs_assigned.iter() { if let Some(detail) = jobs.get(&job_id) { if detail.state == JobState::Started + && !details.last_active_jobs.contains(&job_id) && now.duration_since(detail.since) > STARTED_COMPLETE_TIMEOUT { stale_started.push(job_id); @@ -921,6 +967,7 @@ impl SchedulerIncoming for Scheduler { last_error: None, jobs_assigned: HashSet::new(), jobs_unclaimed: HashMap::new(), + last_active_jobs: HashSet::new(), num_cpus, server_nonce, job_authorizer, @@ -1076,6 +1123,9 @@ pub struct Server { builder: Box, cache: Mutex, job_toolchains: Mutex>, + // Jobs currently executing a build. Reported in every heartbeat so the + // scheduler's liveness lease never reaps a compile still in flight here. + running_jobs: Mutex>, } impl Server { @@ -1090,10 +1140,25 @@ impl Server { builder, cache: Mutex::new(cache), job_toolchains: Mutex::new(HashMap::new()), + running_jobs: Mutex::new(HashSet::new()), }) } } +// Removes a job id from the running set no matter how handle_run_job returns +// (normal completion, build error, or panic), so a failed build can never +// leave a phantom entry that makes the scheduler believe the job is still live. +struct RunningJobGuard<'a> { + running_jobs: &'a Mutex>, + job_id: JobId, +} + +impl Drop for RunningJobGuard<'_> { + fn drop(&mut self) { + self.running_jobs.lock().unwrap().remove(&self.job_id); + } +} + impl ServerIncoming for Server { fn handle_assign_job(&self, job_id: JobId, tc: Toolchain) -> Result { let need_toolchain = !self.cache.lock().unwrap().contains_toolchain(&tc); @@ -1169,6 +1234,12 @@ impl ServerIncoming for Server { requester .do_update_job_state(job_id, JobState::Started) .context("Updating job state failed")?; + self.running_jobs.lock().unwrap().insert(job_id); + // De-register on every exit path (success, build error, panic). + let _running_guard = RunningJobGuard { + running_jobs: &self.running_jobs, + job_id, + }; let tc = self .job_toolchains .lock() @@ -1202,6 +1273,9 @@ impl ServerIncoming for Server { } res } + fn active_jobs(&self) -> Vec { + self.running_jobs.lock().unwrap().iter().copied().collect() + } } #[cfg(test)] @@ -1258,6 +1332,7 @@ mod scheduler_tests { last_error: None, jobs_assigned: HashSet::new(), jobs_unclaimed: HashMap::new(), + last_active_jobs: HashSet::new(), num_cpus: 32, server_nonce: ServerNonce::new(), job_authorizer: Box::new(NoopAuthorizer), @@ -1351,6 +1426,7 @@ mod scheduler_tests { last_error, jobs_assigned, jobs_unclaimed: HashMap::new(), + last_active_jobs: HashSet::new(), num_cpus: 32, server_nonce: ServerNonce::new(), job_authorizer: Box::new(NoopAuthorizer), @@ -1426,6 +1502,7 @@ mod scheduler_tests { last_error: None, jobs_assigned, jobs_unclaimed: HashMap::new(), + last_active_jobs: HashSet::new(), num_cpus: 32, server_nonce: ServerNonce::new(), job_authorizer: Box::new(NoopAuthorizer), @@ -1460,6 +1537,18 @@ mod scheduler_tests { job_id, STARTED_COMPLETE_TIMEOUT + Duration::from_secs(10), ); + // Seed the prior-beat active set so the liveness lease grants this job + // its grace beat: with the job absent from active_jobs but present in + // last_active_jobs, the liveness path skips it, so the reap here can + // only come from the age-based backstop this test exercises. + scheduler + .servers + .lock() + .unwrap() + .get_mut(&server_id) + .unwrap() + .last_active_jobs + .insert(job_id); let nonce = scheduler .servers .lock() @@ -1470,7 +1559,7 @@ mod scheduler_tests { .clone(); scheduler - .handle_heartbeat_server(server_id, nonce, 32, Box::new(NoopAuthorizer)) + .handle_heartbeat_server(server_id, nonce, 32, Box::new(NoopAuthorizer), Vec::new()) .expect("handle_heartbeat_server should not error"); let servers = scheduler.servers.lock().unwrap(); @@ -1509,8 +1598,11 @@ mod scheduler_tests { .server_nonce .clone(); + // The server reports the job still running, so both the liveness path + // (present in active_jobs) and the age-based backstop (within timeout) + // must leave it assigned. scheduler - .handle_heartbeat_server(server_id, nonce, 32, Box::new(NoopAuthorizer)) + .handle_heartbeat_server(server_id, nonce, 32, Box::new(NoopAuthorizer), vec![job_id]) .expect("handle_heartbeat_server should not error"); let servers = scheduler.servers.lock().unwrap(); @@ -1521,6 +1613,96 @@ mod scheduler_tests { ); } + // The liveness lease is the primary Started-reap mechanism: a Started job is + // reaped only after it is absent from two consecutive heartbeats of the + // owning server. A job the server keeps reporting active is never reaped (no + // matter how long the compile runs), and a single missed heartbeat is not + // enough - guarding both the slow-live-compile false reap the old fixed + // timeout caused and an over-eager one-beat reap. Job age stays ~0 + // throughout, so the STARTED_COMPLETE_TIMEOUT backstop never fires and every + // reap decision here is the liveness path. + #[test] + fn test_liveness_lease_reap() { + let scheduler = Scheduler::new(); + let server_id = ServerId::new("10.42.0.2:10501".parse::().unwrap()); + let job_id = JobId(1000); + insert_started_job(&scheduler, server_id, job_id, Duration::from_secs(0)); + let nonce = scheduler + .servers + .lock() + .unwrap() + .get(&server_id) + .unwrap() + .server_nonce + .clone(); + + // Reported active across many heartbeats: never reaped. + for _ in 0..5 { + scheduler + .handle_heartbeat_server( + server_id, + nonce.clone(), + 32, + Box::new(NoopAuthorizer), + vec![job_id], + ) + .expect("handle_heartbeat_server should not error"); + let servers = scheduler.servers.lock().unwrap(); + assert!( + servers + .get(&server_id) + .unwrap() + .jobs_assigned + .contains(&job_id), + "a job reported active must never be reaped" + ); + } + + // First absent heartbeat: one miss alone must not reap (needs two). + scheduler + .handle_heartbeat_server( + server_id, + nonce.clone(), + 32, + Box::new(NoopAuthorizer), + Vec::new(), + ) + .expect("handle_heartbeat_server should not error"); + assert!( + scheduler + .servers + .lock() + .unwrap() + .get(&server_id) + .unwrap() + .jobs_assigned + .contains(&job_id), + "a single absent heartbeat must not reap - the lease needs two consecutive misses" + ); + + // Second consecutive absent heartbeat: now the job is reaped. + scheduler + .handle_heartbeat_server(server_id, nonce, 32, Box::new(NoopAuthorizer), Vec::new()) + .expect("handle_heartbeat_server should not error"); + { + let servers = scheduler.servers.lock().unwrap(); + let details = servers.get(&server_id).expect("server still registered"); + assert!( + !details.jobs_assigned.contains(&job_id), + "two consecutive absent heartbeats must reap the Started job" + ); + } + assert!( + !scheduler.jobs.lock().unwrap().contains_key(&job_id), + "the liveness-reaped job must be removed from the jobs map" + ); + assert_eq!( + scheduler.jobs_reaped_started.load(Ordering::Relaxed), + 1, + "the liveness reap must increment the Started-reap counter exactly once" + ); + } + // A late Complete (or Started) update for a job the scheduler no longer // tracks must be a no-op success, not an error. If it errors, the build // server's run_job (which propagates the terminal update) fails, and the @@ -1564,6 +1746,7 @@ mod scheduler_tests { last_error: None, jobs_assigned, jobs_unclaimed, + last_active_jobs: HashSet::new(), num_cpus: 32, server_nonce: ServerNonce::new(), job_authorizer: Box::new(NoopAuthorizer), diff --git a/src/dist/http.rs b/src/dist/http.rs index 015cc31df..f61b5cf9f 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -150,6 +150,7 @@ mod common { pub server_nonce: dist::ServerNonce, pub cert_digest: Vec, pub cert_pem: Vec, + pub active_jobs: Vec, } #[cfg(feature = "dist-server")] @@ -163,6 +164,7 @@ mod common { "cert_digest", &BASE64_URL_SAFE_ENGINE.encode(&self.cert_digest), ) + .field("active_jobs", &self.active_jobs) .finish() } } @@ -254,7 +256,7 @@ mod server { use std::net::SocketAddr; use std::result::Result as StdResult; use std::sync::atomic; - use std::sync::{LazyLock, Mutex}; + use std::sync::{Arc, LazyLock, Mutex}; use std::thread; use std::time::Duration; @@ -811,7 +813,7 @@ mod server { let heartbeat_server = try_or_400_log!(req_id, bincode_input(request)); trace!(target: "sccache_heartbeat", "Req {}: heartbeat_server: {:?}", req_id, heartbeat_server); - let HeartbeatServerHttpRequest { num_cpus, jwt_key, server_nonce, cert_digest, cert_pem } = heartbeat_server; + let HeartbeatServerHttpRequest { num_cpus, jwt_key, server_nonce, cert_digest, cert_pem, active_jobs } = heartbeat_server; try_or_500_log!(req_id, maybe_update_certs( &mut requester.client.lock().unwrap(), &mut server_certificates.lock().unwrap(), @@ -821,7 +823,8 @@ mod server { let res: HeartbeatServerResult = try_or_500_log!(req_id, handler.handle_heartbeat_server( server_id, server_nonce, num_cpus, - job_authorizer + job_authorizer, + active_jobs )); prepare_response(request, &res) }, @@ -965,6 +968,9 @@ mod server { advertised_num_cpus, handler, } = self; + // Shared so the heartbeat thread can snapshot the handler's + // running-jobs set each beat while the request router still owns it. + let handler = Arc::new(handler); // Graceful shutdown: on SIGTERM/SIGINT, deregister from the // scheduler before exiting so it drops us at once instead of // waiting out the 90s heartbeat timeout. The timeout stays as the @@ -998,13 +1004,12 @@ mod server { } }); - let heartbeat_req = HeartbeatServerHttpRequest { - num_cpus: advertised_num_cpus, - jwt_key: jwt_key.clone(), - server_nonce, - cert_digest, - cert_pem: cert_pem.clone(), - }; + // Values the heartbeat thread rebuilds its request from each beat. + // cert_digest and server_nonce are used only here, so move them in; + // jwt_key and cert_pem are still needed below, so clone for the thread. + let heartbeat_jwt_key = jwt_key.clone(); + let heartbeat_cert_pem = cert_pem.clone(); + let heartbeat_handler = Arc::clone(&handler); let job_authorizer = JWTJobAuthorizer::new(jwt_key); let heartbeat_url = urls::scheduler_heartbeat_server(&scheduler_url); let requester = ServerRequester { @@ -1018,6 +1023,19 @@ mod server { let client = new_reqwest_blocking_client(); loop { trace!(target: "sccache_heartbeat", "Performing heartbeat"); + // Rebuild the request each beat so active_jobs is a fresh + // snapshot of what this server is running. The scheduler + // reaps a Started job only after two consecutive heartbeats + // omit it, so a stale snapshot could either strand a done + // job or reap a live one. + let heartbeat_req = HeartbeatServerHttpRequest { + num_cpus: advertised_num_cpus, + jwt_key: heartbeat_jwt_key.clone(), + server_nonce: server_nonce.clone(), + cert_digest: cert_digest.clone(), + cert_pem: heartbeat_cert_pem.clone(), + active_jobs: heartbeat_handler.active_jobs(), + }; match bincode_req( client .post(heartbeat_url.clone()) diff --git a/src/dist/mod.rs b/src/dist/mod.rs index 6d4021759..59466394c 100644 --- a/src/dist/mod.rs +++ b/src/dist/mod.rs @@ -692,6 +692,7 @@ pub trait SchedulerIncoming: Send + Sync { server_nonce: ServerNonce, num_cpus: usize, job_authorizer: Box, + active_jobs: Vec, ) -> ExtResult; // From Server fn handle_deregister_server( @@ -729,6 +730,10 @@ pub trait ServerIncoming: Send + Sync { outputs: Vec, inputs_rdr: InputsReader<'_>, ) -> ExtResult; + // Snapshot of the job ids this server is currently running, reported in + // every heartbeat so the scheduler can reap only jobs a server no longer + // reports (liveness lease), never a genuinely-slow live compile. + fn active_jobs(&self) -> Vec; } #[cfg(feature = "dist-server")] diff --git a/tests/dist.rs b/tests/dist.rs index f3f52aca3..391b76cf3 100644 --- a/tests/dist.rs +++ b/tests/dist.rs @@ -237,6 +237,9 @@ impl ServerIncoming for FailingServer { .context("Updating job state failed")?; bail!("internal build failure") } + fn active_jobs(&self) -> Vec { + Vec::new() + } } #[test] From 317187252fb6ac568ea0f1f4433ab8be86a2ea1b Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 14:39:49 -0600 Subject: [PATCH 39/43] dist/scheduler: address Group 1 review findings A cold-read review of the Group 1 changes surfaced two must-fix bugs. The liveness lease could reap a freshly-Started job on the first heartbeat after it started: a job that transitions to Started between two beats is vacuously absent from the prior beat, so a heartbeat snapshot that races the Started-update-then-registry-insert window sees it missing from both beats and reaps it milliseconds into its compile. Add a LIVENESS_GRACE floor (45s, above the 30s heartbeat interval) on time-in-Started so only a job that has lived across a full beat period is reapable, and register the running job before sending the Started update so the window shrinks to pure network reordering. Seeding job_count from wall-clock millis (for restart-safe ids) also clobbered the dist-accounting allocated= counter, which reads the same field, breaking the allocated-started-reaped-live identity the leak detector depends on. Split the roles: job_count is a 0-based allocation counter again (logged as allocated=), and a new job_id_base carries the millis seed so minted ids stay unique across a restart. Also restrict the unknown-job update no-op to Started/Complete (a Pending/Ready update for an untracked job still errors), warn when the backstop protects a job past the timeout because the server still names it (wedged-build visibility), and document the toolchain-GC timeout coupling. Covered by test_liveness_lease_does_not_reap_within_grace and test_job_id_base_seeded_and_counter_zero. Signed-off-by: Javier Tia --- src/bin/sccache-dist/main.rs | 149 +++++++++++++++++++++++++++++------ 1 file changed, 124 insertions(+), 25 deletions(-) diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 7fa71853f..8fda85c8a 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -370,10 +370,19 @@ const UNCLAIMED_READY_TIMEOUT: Duration = Duration::from_secs(60); // runs over a separate client<->server channel - so an over-eager reap at // worst mildly oversubscribes, which admission_ceiling already tolerates. const STARTED_COMPLETE_TIMEOUT: Duration = Duration::from_secs(180); +// Minimum time a job must have been in Started before the liveness lease may +// reap it for two consecutive absent heartbeats. Must exceed the 30s +// HEARTBEAT_INTERVAL (src/dist/http.rs) so a reapable job has been Started +// across at least one full beat period; otherwise a job that transitions to +// Started between two beats looks vacuously "absent from the prior beat" and +// could be reaped milliseconds after it starts. +const LIVENESS_GRACE: Duration = Duration::from_secs(45); // A build server drops its per-job toolchain entry in handle_run_job. An // assign whose run_job never arrives (client crash, scheduler restart) would // otherwise leak the entry forever, so sweep entries older than this on each -// assign. Far longer than a legitimate assign->run_job gap. +// assign. Far longer than a legitimate assign->run_job gap. Must stay greater +// than UNCLAIMED_PENDING_TIMEOUT (300s) + UNCLAIMED_READY_TIMEOUT (60s) so the +// GC never evicts a still-in-flight job's toolchain entry. const ABANDONED_TOOLCHAIN_TIMEOUT: Duration = Duration::from_secs(600); #[derive(Copy, Clone)] @@ -388,8 +397,19 @@ struct JobDetail { // To avoid deadlicking, make sure to do all locking at once (i.e. no further locking in a downward scope), // in alphabetical order pub struct Scheduler { + // Pure 0-based allocation counter. Logged as allocated= in the + // dist-accounting line, where the operator validates the identity + // allocated - started - reaped_unclaimed - live == 0, so it must start at + // 0. Job id uniqueness across restarts comes from job_id_base, not here. job_count: AtomicUsize, + // Restart-safety base for minted job ids. Seeded from wall-clock millis so + // a scheduler restart does not remint ids from 0 and collide with an id a + // surviving build server still holds (which would trip the assign-time + // overwrite path). Kept separate from job_count so the millis seed does not + // pollute the 0-based allocated= counter. + job_id_base: usize, + // Currently running jobs, can never be Complete jobs: Mutex>, @@ -430,15 +450,14 @@ struct ServerDetails { impl Scheduler { pub fn new() -> Self { Scheduler { + job_count: AtomicUsize::new(0), // Seed from wall-clock millis so a scheduler restart does not remint // job ids from 0 and collide with an id a surviving build server // still holds (which would trip the assign-time overwrite path). - job_count: AtomicUsize::new( - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_millis() as usize) - .unwrap_or(0), - ), + job_id_base: std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_millis() as usize) + .unwrap_or(0), jobs: Mutex::new(BTreeMap::new()), servers: Mutex::new(HashMap::new()), jobs_started: AtomicUsize::new(0), @@ -683,8 +702,9 @@ impl SchedulerIncoming for Scheduler { (None, errored) => errored, }; if let Some((server_id, server_details)) = choice { - let job_count = self.job_count.fetch_add(1, Ordering::SeqCst) as u64; - let job_id = JobId(job_count); + let job_id = JobId( + (self.job_id_base + self.job_count.fetch_add(1, Ordering::SeqCst)) as u64, + ); assert!(server_details.jobs_assigned.insert(job_id)); assert!( server_details @@ -875,6 +895,7 @@ impl SchedulerIncoming for Scheduler { if detail.state == JobState::Started && !active_set.contains(&job_id) && !details.last_active_jobs.contains(&job_id) + && now.duration_since(detail.since) > LIVENESS_GRACE { liveness_reap.push(job_id); } @@ -907,10 +928,23 @@ impl SchedulerIncoming for Scheduler { for &job_id in details.jobs_assigned.iter() { if let Some(detail) = jobs.get(&job_id) { if detail.state == JobState::Started - && !details.last_active_jobs.contains(&job_id) && now.duration_since(detail.since) > STARTED_COMPLETE_TIMEOUT { - stale_started.push(job_id); + if details.last_active_jobs.contains(&job_id) { + // Still reported active past the backstop timeout: + // not reaped (a genuinely-slow live compile must + // survive) but likely a wedged build the server + // names forever. Surface it so it is visible in + // logs rather than silently accumulating. + warn!( + "Started job {} on server {} still reported active past {:?}; possible wedged build", + job_id, + server_id.addr(), + STARTED_COMPLETE_TIMEOUT + ); + } else { + stale_started.push(job_id); + } } } } @@ -1076,9 +1110,9 @@ impl SchedulerIncoming for Scheduler { job_state, job_id ); } - other => { - warn!("State update to {:?} for unknown job {}", other, job_id); - } + // A Pending/Ready update for an untracked job is a real anomaly, + // not a benign late terminal update, so keep erroring on it. + _ => bail!("Unknown job"), } } Ok(UpdateJobStateResult::Success) @@ -1231,15 +1265,19 @@ impl ServerIncoming for Server { outputs: Vec, inputs_rdr: InputsReader, ) -> Result { - requester - .do_update_job_state(job_id, JobState::Started) - .context("Updating job state failed")?; + // Register the job and arm the cleanup guard BEFORE telling the + // scheduler the job started, so the id is tracked before the scheduler + // acts on the Started update (shrinking the vacuous liveness-reap + // window). De-registers on every exit path (success, build error, + // panic), and the guard's Drop still cleans up if the update fails. self.running_jobs.lock().unwrap().insert(job_id); - // De-register on every exit path (success, build error, panic). let _running_guard = RunningJobGuard { running_jobs: &self.running_jobs, job_id, }; + requester + .do_update_job_state(job_id, JobState::Started) + .context("Updating job state failed")?; let tc = self .job_toolchains .lock() @@ -1618,15 +1656,16 @@ mod scheduler_tests { // owning server. A job the server keeps reporting active is never reaped (no // matter how long the compile runs), and a single missed heartbeat is not // enough - guarding both the slow-live-compile false reap the old fixed - // timeout caused and an over-eager one-beat reap. Job age stays ~0 - // throughout, so the STARTED_COMPLETE_TIMEOUT backstop never fires and every - // reap decision here is the liveness path. + // timeout caused and an over-eager one-beat reap. Job age is seeded past the + // LIVENESS_GRACE floor (45s) but below the STARTED_COMPLETE_TIMEOUT backstop + // (180s), so the backstop never fires and every reap decision here is the + // liveness path. #[test] fn test_liveness_lease_reap() { let scheduler = Scheduler::new(); let server_id = ServerId::new("10.42.0.2:10501".parse::().unwrap()); let job_id = JobId(1000); - insert_started_job(&scheduler, server_id, job_id, Duration::from_secs(0)); + insert_started_job(&scheduler, server_id, job_id, Duration::from_secs(60)); let nonce = scheduler .servers .lock() @@ -1703,6 +1742,61 @@ mod scheduler_tests { ); } + // The LIVENESS_GRACE floor protects a just-started job: a job that has been + // Started for less than the grace must NOT be reaped even after two absent + // heartbeats, because a job that transitioned to Started between two beats + // is vacuously "absent from the prior beat" and would otherwise be reaped + // milliseconds after it started. Falsifier: dropping the grace clause reaps + // this job on the second absent heartbeat. + #[test] + fn test_liveness_lease_does_not_reap_within_grace() { + let scheduler = Scheduler::new(); + let server_id = ServerId::new("10.42.0.2:10501".parse::().unwrap()); + let job_id = JobId(1000); + // Aged below LIVENESS_GRACE (45s): a fresh Started job. + insert_started_job(&scheduler, server_id, job_id, Duration::from_secs(10)); + let nonce = scheduler + .servers + .lock() + .unwrap() + .get(&server_id) + .unwrap() + .server_nonce + .clone(); + + // Two consecutive absent heartbeats: the two-miss condition is met, but + // the grace floor must still protect the just-started job. + for _ in 0..2 { + scheduler + .handle_heartbeat_server( + server_id, + nonce.clone(), + 32, + Box::new(NoopAuthorizer), + Vec::new(), + ) + .expect("handle_heartbeat_server should not error"); + } + + { + let servers = scheduler.servers.lock().unwrap(); + let details = servers.get(&server_id).expect("server still registered"); + assert!( + details.jobs_assigned.contains(&job_id), + "a job Started within LIVENESS_GRACE must not be reaped even after two absent heartbeats" + ); + } + assert!( + scheduler.jobs.lock().unwrap().contains_key(&job_id), + "a job Started within the grace must remain in the jobs map" + ); + assert_eq!( + scheduler.jobs_reaped_started.load(Ordering::Relaxed), + 0, + "no Started reap must occur within the grace floor" + ); + } + // A late Complete (or Started) update for a job the scheduler no longer // tracks must be a no-op success, not an error. If it errors, the build // server's run_job (which propagates the terminal update) fails, and the @@ -1785,11 +1879,16 @@ mod scheduler_tests { // collide with an id a surviving build server still holds, tripping the // assign-time overwrite (previously an assert-panic). #[test] - fn test_job_count_seeded_nonzero() { + fn test_job_id_base_seeded_and_counter_zero() { let scheduler = Scheduler::new(); assert!( - scheduler.job_count.load(Ordering::Relaxed) > 0, - "job_count must be seeded from a non-zero base so a restart does not reuse ids starting at 0" + scheduler.job_id_base > 0, + "job_id_base must be seeded from a non-zero base so a restart does not reuse ids starting at 0" + ); + assert_eq!( + scheduler.job_count.load(Ordering::Relaxed), + 0, + "job_count is the 0-based allocated= counter and must start at 0" ); } } From fd86b635cda5499acaba0140ef8b8268ff911acb Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 15:53:54 -0600 Subject: [PATCH 40/43] dist/http: reuse connections and offload large deserialize The single client daemon is the throughput chokepoint of a full image build. It opened a fresh TCP+TLS connection on every coordination call (pool_max_idle_per_host(0) plus a Connection: close header), decoded multi-megabyte object blobs inline on the same async workers that poll every other in-flight request, and rebuilt the reqwest client under a Mutex held across the entire rebuild whenever a server certificate refreshed. Under make -jN the daemon backed up while build cores sat idle, which is why sccache-dist lost to plain ccache on the full image even though it wins on expensive single-object recipes like llvm. Re-enable keep-alive on all four dist client builders (a small idle pool plus a 90s idle timeout) and drop the Connection: close workaround. tiny_http 0.12 drains the request body on drop of the request reader, so a partially-read body on a reused connection cannot desync the next request framed on it. Deserialize responses above a config-selectable threshold via spawn_blocking so a large blob no longer stalls the reactor from polling other compiles, and hot-swap the TLS client through ArcSwap so a certificate refresh publishes the new client without holding a lock across the rebuild. The run_job compression level is now config-selectable with the default unchanged, so a no-compression setting can be measured on a fast LAN where CPU is scarcer than bandwidth. Keep-alive is the one intentional default behavior change; it carries an A5 revert marker in bincode_req_fut pointing at the tiny_http #151 risk so a soak-triggered rollback has an exact anchor. Signed-off-by: Javier Tia --- Cargo.lock | 1 + Cargo.toml | 1 + src/config.rs | 13 ++++++ src/dist/http.rs | 108 ++++++++++++++++++++++++++++++++--------------- src/server.rs | 8 ++++ 5 files changed, 96 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9263cd2c5..3be7bc16d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2889,6 +2889,7 @@ version = "0.16.0" dependencies = [ "anyhow", "ar", + "arc-swap", "assert_cmd", "async-trait", "backon", diff --git a/Cargo.toml b/Cargo.toml index e35ea56ae..6649dac77 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,6 +41,7 @@ strip = true [dependencies] anyhow = { version = "1.0", features = ["backtrace"] } ar = "0.9" +arc-swap = "1.7.1" async-trait = "0.1" backon = { version = "1", default-features = false, features = [ "std-blocking-sleep", diff --git a/src/config.rs b/src/config.rs index 85932479b..5e8b5b4f5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -119,6 +119,12 @@ fn default_disk_cache_size() -> u64 { fn default_toolchain_cache_size() -> u64 { TEN_GIGS } +fn default_client_deserialize_offload_threshold() -> u64 { + 1024 * 1024 +} +fn default_run_job_compression_level() -> u32 { + 1 +} struct StringOrU64Visitor; @@ -763,6 +769,9 @@ pub struct DistConfig { #[serde(deserialize_with = "deserialize_size_from_str")] pub toolchain_cache_size: u64, pub rewrite_includes_only: bool, + #[serde(deserialize_with = "deserialize_size_from_str")] + pub client_deserialize_offload_threshold: u64, + pub run_job_compression_level: u32, } impl Default for DistConfig { @@ -774,6 +783,8 @@ impl Default for DistConfig { toolchains: Default::default(), toolchain_cache_size: default_toolchain_cache_size(), rewrite_includes_only: false, + client_deserialize_offload_threshold: default_client_deserialize_offload_threshold(), + run_job_compression_level: default_run_job_compression_level(), } } } @@ -2586,6 +2597,8 @@ key_prefix = "cosprefix" toolchains: vec![], toolchain_cache_size: 5368709120, rewrite_includes_only: false, + client_deserialize_offload_threshold: 1024 * 1024, + run_job_compression_level: 1, }, server_startup_timeout_ms: Some(10000), basedirs: vec![], diff --git a/src/dist/http.rs b/src/dist/http.rs index f61b5cf9f..f8e54a94e 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -66,12 +66,15 @@ mod common { } #[cfg(feature = "dist-client")] - pub async fn bincode_req_fut( + pub async fn bincode_req_fut( req: reqwest::RequestBuilder, + deserialize_offload_threshold: u64, ) -> Result { - // Work around tiny_http issue #151 by disabling HTTP pipeline with - // `Connection: close`. - let res = req.header(header::CONNECTION, "close").send().await?; + // A5: keep-alive is left enabled (no `Connection: close`), so a reused + // connection reaches tiny_http, which risks issue #151 (pipeline/body + // desync) surfacing as bincode decode errors under soak. Revert point: + // re-add `.header(header::CONNECTION, "close")` on the request below. + let res = req.send().await?; let status = res.status(); let bytes = res.bytes().await?; @@ -86,6 +89,11 @@ mod common { } else { anyhow::bail!(errmsg); } + } else if bytes.len() as u64 > deserialize_offload_threshold { + // Deserializing a multi-MB blob inline stalls this async worker from + // polling other in-flight requests; offload large payloads to a + // blocking thread so the reactor keeps making progress. + Ok(tokio::task::spawn_blocking(move || bincode::deserialize(&bytes)).await??) } else { Ok(bincode::deserialize(&bytes)?) } @@ -280,9 +288,9 @@ mod server { pub fn bincode_req( req: reqwest::blocking::RequestBuilder, ) -> Result { - // Work around tiny_http issue #151 by disabling HTTP pipeline with - // `Connection: close`. - let mut res = req.header(reqwest::header::CONNECTION, "close").send()?; + // Keep-alive is left enabled (no `Connection: close`) so the connection + // pool can reuse this socket for the next coordination call. + let mut res = req.send()?; let status = res.status(); let mut body = vec![]; res.copy_to(&mut body) @@ -753,9 +761,11 @@ mod server { } // Finish the client let new_client = client_builder - // Disable connection pool to avoid broken connection - // between runtime - .pool_max_idle_per_host(0) + // Keep a small idle pool so keep-alive connections are + // reused across coordination calls; expire them under the + // LAN idle limit to avoid reusing a half-closed socket. + .pool_max_idle_per_host(2) + .pool_idle_timeout(Duration::from_secs(90)) .build() .context("failed to create a HTTP client")?; // Use the updated certificates @@ -980,7 +990,10 @@ mod server { install_shutdown_handler(); thread::spawn(move || { let client = reqwest::blocking::Client::builder() - .pool_max_idle_per_host(0) + // Keep a small idle pool with a sub-LAN-limit idle timeout + // so keep-alive connections are reused, not rebuilt. + .pool_max_idle_per_host(2) + .pool_idle_timeout(Duration::from_secs(90)) .timeout(Duration::from_secs(5)) .build() .expect("deregister http client must build"); @@ -1155,6 +1168,7 @@ mod client { SchedulerStatusResult, SubmitToolchainResult, Toolchain, }; + use arc_swap::ArcSwap; use async_trait::async_trait; use byteorder::{BigEndian, WriteBytesExt}; use flate2::Compression; @@ -1177,18 +1191,31 @@ mod client { const REQUEST_TIMEOUT_SECS: u64 = 1200; const CONNECT_TIMEOUT_SECS: u64 = 5; + // Map a config level to a zlib setting: 0 disables compression (for + // measurement on a fast LAN), 1-9 select the zlib level. Level 1 reproduces + // the historical `Compression::fast()` default. + fn run_job_compression(level: u32) -> Compression { + match level { + 0 => Compression::none(), + l => Compression::new(l.min(9)), + } + } + pub struct Client { auth_token: String, scheduler_url: reqwest::Url, // cert_digest -> cert_pem server_certs: Arc, Vec>>>, - client: Arc>, + client: Arc>, pool: tokio::runtime::Handle, tc_cache: Arc, rewrite_includes_only: bool, + deserialize_offload_threshold: u64, + run_job_compression_level: u32, } impl Client { + #[allow(clippy::too_many_arguments)] pub fn new( pool: &tokio::runtime::Handle, scheduler_url: reqwest::Url, @@ -1197,15 +1224,19 @@ mod client { toolchain_configs: &[config::DistToolchainConfig], auth_token: String, rewrite_includes_only: bool, + deserialize_offload_threshold: u64, + run_job_compression_level: u32, ) -> Result { let timeout = Duration::new(REQUEST_TIMEOUT_SECS, 0); let connect_timeout = Duration::new(CONNECT_TIMEOUT_SECS, 0); let client = reqwest::ClientBuilder::new() .timeout(timeout) .connect_timeout(connect_timeout) - // Disable connection pool to avoid broken connection - // between runtime - .pool_max_idle_per_host(0) + // Keep a small idle pool so keep-alive connections are reused + // across coordination calls; expire them under the LAN idle + // limit to avoid reusing a half-closed socket. + .pool_max_idle_per_host(2) + .pool_idle_timeout(Duration::from_secs(90)) .build() .context("failed to create an async HTTP client")?; let client_toolchains = @@ -1215,15 +1246,17 @@ mod client { auth_token, scheduler_url, server_certs: Default::default(), - client: Arc::new(Mutex::new(client)), + client: Arc::new(ArcSwap::from_pointee(client)), pool: pool.clone(), tc_cache: Arc::new(client_toolchains), rewrite_includes_only, + deserialize_offload_threshold, + run_job_compression_level, }) } fn update_certs( - client: &mut reqwest::Client, + client: &ArcSwap, certs: &mut HashMap, Vec>, cert_digest: Vec, cert_pem: Vec, @@ -1243,12 +1276,15 @@ 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) + // Keep a small idle pool so keep-alive connections are reused; + // expire them under the LAN idle limit. + .pool_max_idle_per_host(2) + .pool_idle_timeout(Duration::from_secs(90)) .build() .context("failed to create an async HTTP client")?; - // Use the updated certificates - *client = new_client_async; + // Hot-swap the client without holding a lock across the rebuild, so + // in-flight requests keep serving off the old client until the swap. + client.store(Arc::new(new_client_async)); certs.insert(cert_digest, cert_pem); Ok(()) } @@ -1259,13 +1295,13 @@ mod client { async fn do_alloc_job(&self, tc: Toolchain) -> Result { let scheduler_url = self.scheduler_url.clone(); let url = urls::scheduler_alloc_job(&scheduler_url); - let mut req = self.client.lock().unwrap().post(url); + let mut req = self.client.load().post(url); req = req.bearer_auth(self.auth_token.clone()).bincode(&tc)?; let client = self.client.clone(); let server_certs = self.server_certs.clone(); - match bincode_req_fut(req).await? { + match bincode_req_fut(req, self.deserialize_offload_threshold).await? { AllocJobHttpResponse::Success { job_alloc, need_toolchain, @@ -1284,10 +1320,11 @@ mod client { server_id.addr() ); let url = urls::scheduler_server_certificate(&scheduler_url, server_id); - let req = client.lock().unwrap().get(url); - let res: ServerCertificateHttpResponse = bincode_req_fut(req) - .await - .context("GET to scheduler server_certificate failed")?; + let req = client.load().get(url); + let res: ServerCertificateHttpResponse = + bincode_req_fut(req, self.deserialize_offload_threshold) + .await + .context("GET to scheduler server_certificate failed")?; // TODO: Move to asynchronous reqwest client only. // This function internally builds a blocking reqwest client; @@ -1300,7 +1337,7 @@ mod client { .pool .spawn_blocking(move || { Self::update_certs( - &mut client.lock().unwrap(), + &client, &mut server_certs.lock().unwrap(), res.cert_digest, res.cert_pem, @@ -1319,8 +1356,8 @@ mod client { async fn do_get_status(&self) -> Result { let scheduler_url = self.scheduler_url.clone(); let url = urls::scheduler_status(&scheduler_url); - let req = self.client.lock().unwrap().get(url); - bincode_req_fut(req).await + let req = self.client.load().get(url); + bincode_req_fut(req, self.deserialize_offload_threshold).await } async fn do_submit_toolchain( @@ -1331,12 +1368,12 @@ mod client { match self.tc_cache.get_toolchain(&tc) { Ok(Some(toolchain_file)) => { let url = urls::server_submit_toolchain(job_alloc.server_id, job_alloc.job_id); - let req = self.client.lock().unwrap().post(url); + let req = self.client.load().post(url); let toolchain_file = tokio::fs::File::from_std(toolchain_file.into()); let toolchain_file_stream = tokio_util::io::ReaderStream::new(toolchain_file); let body = Body::wrap_stream(toolchain_file_stream); let req = req.bearer_auth(job_alloc.auth).body(body); - bincode_req_fut(req).await + bincode_req_fut(req, self.deserialize_offload_threshold).await } Ok(None) => Err(anyhow!("couldn't find toolchain locally")), Err(e) => Err(e), @@ -1351,6 +1388,7 @@ mod client { inputs_packager: Box, ) -> Result<(RunJobResult, PathTransformer)> { let url = urls::server_run_job(job_alloc.server_id, job_alloc.job_id); + let compression = run_job_compression(self.run_job_compression_level); let (body, path_transformer) = self .pool @@ -1366,7 +1404,7 @@ mod client { .expect("Infallible write of bincode body to vec failed"); let path_transformer; { - let mut compressor = ZlibWriteEncoder::new(&mut body, Compression::fast()); + let mut compressor = ZlibWriteEncoder::new(&mut body, compression); path_transformer = inputs_packager .write_inputs(&mut compressor) .context("Could not write inputs for compilation")?; @@ -1382,9 +1420,9 @@ mod client { Ok((body, path_transformer)) }) .await??; - let mut req = self.client.lock().unwrap().post(url); + let mut req = self.client.load().post(url); req = req.bearer_auth(job_alloc.auth.clone()).bytes(body); - bincode_req_fut(req) + bincode_req_fut(req, self.deserialize_offload_threshold) .map_ok(|res| (res, path_transformer)) .await } diff --git a/src/server.rs b/src/server.rs index 227b95318..0454c0249 100644 --- a/src/server.rs +++ b/src/server.rs @@ -167,6 +167,8 @@ pub struct DistClientConfig { toolchain_cache_size: u64, toolchains: Vec, rewrite_includes_only: bool, + deserialize_offload_threshold: u64, + run_job_compression_level: u32, } #[cfg(feature = "dist-client")] @@ -223,6 +225,8 @@ impl DistClientContainer { toolchain_cache_size: config.dist.toolchain_cache_size, toolchains: config.dist.toolchains.clone(), rewrite_includes_only: config.dist.rewrite_includes_only, + deserialize_offload_threshold: config.dist.client_deserialize_offload_threshold, + run_job_compression_level: config.dist.run_job_compression_level, }; let state = Self::create_state(config); let state = pool.block_on(state); @@ -388,6 +392,8 @@ impl DistClientContainer { &config.toolchains, auth_token, config.rewrite_includes_only, + config.deserialize_offload_threshold, + config.run_job_compression_level, ); let dist_client = try_or_retry_later!(dist_client.context("failure during dist client creation")); @@ -1066,6 +1072,8 @@ where toolchain_cache_size: 0, toolchains: vec![], rewrite_includes_only: false, + deserialize_offload_threshold: 1024 * 1024, + run_job_compression_level: 1, }), dist_client, ))), From 9c8d7961c5641e971221e04be0ace85754123053 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 16:19:15 -0600 Subject: [PATCH 41/43] dist/server: shrink toolchain locks so compiles stop convoying The build server serialized all concurrent compiles behind two global locks. prepare_overlay_dirs held the toolchain_dir_map mutex across the whole gzip+untar and across eviction's remove_dir_all, so unpacking one toolchain blocked every other compile that needed any toolchain, cached or not. handle_submit_toolchain held the cache mutex across the entire upload io::copy, so one toolchain upload blocked every cache read for its whole duration. Give each toolchain its own preparation lock. The global map lock now only looks up or inserts the per-toolchain Arc> and picks eviction victims; the untar runs under the per-toolchain lock, and each victim's remove_dir_all runs after the map lock is dropped. Two requests for the same uncached toolchain still serialize on the same entry lock (re-checked after acquiring it) so no directory is ever half-written, while requests for different toolchains prepare in parallel. Stream the submit-toolchain upload into a NamedTempFile with no lock held, then re-acquire the cache lock only to graft the finished file in via a new hash-verified TcCache::insert_at. The assign-time duplicate-insert assert becomes a warn, matching the restart-safe accounting convention. Signed-off-by: Javier Tia --- src/bin/sccache-dist/build.rs | 248 +++++++++++++++++++++++++--------- src/bin/sccache-dist/main.rs | 67 ++++++++- src/dist/cache.rs | 20 +++ 3 files changed, 264 insertions(+), 71 deletions(-) diff --git a/src/bin/sccache-dist/build.rs b/src/bin/sccache-dist/build.rs index 5f6eb94bc..d88388dca 100644 --- a/src/bin/sccache-dist/build.rs +++ b/src/bin/sccache-dist/build.rs @@ -26,7 +26,7 @@ use std::io; use std::iter; use std::path::{self, Path, PathBuf}; use std::process::{ChildStdin, Command, Output, Stdio}; -use std::sync::Mutex; +use std::sync::{Arc, Mutex}; use std::time::Instant; use version_compare::Version; @@ -104,6 +104,28 @@ pub struct OverlayBuilder { bubblewrap: PathBuf, dir: PathBuf, toolchain_dir_map: Mutex>, + // Per-toolchain preparation locks. The gzip+untar of an uncached toolchain + // runs under the matching entry lock here, NOT under toolchain_dir_map, so + // two requests for different toolchains prepare in parallel while two for + // the same uncached toolchain serialize (no half-written dir, no double + // unpack). + toolchain_prepare_locks: Mutex>>>, +} + +// Return the preparation lock for `tc`, creating it on first use. Held only +// long enough to look up or insert the entry, so distinct toolchains never +// contend here. Same key yields the same Arc; distinct keys yield distinct +// Arcs. +fn get_prepare_lock( + locks: &Mutex>>>, + tc: &Toolchain, +) -> Arc> { + locks + .lock() + .unwrap() + .entry(tc.clone()) + .or_insert_with(|| Arc::new(Mutex::new(()))) + .clone() } impl OverlayBuilder { @@ -151,6 +173,7 @@ impl OverlayBuilder { bubblewrap, dir, toolchain_dir_map: Mutex::new(HashMap::new()), + toolchain_prepare_locks: Mutex::new(HashMap::new()), }; ret.cleanup()?; fs::create_dir(&ret.dir).context("Failed to create base directory for builder")?; @@ -173,75 +196,123 @@ impl OverlayBuilder { tc: &Toolchain, tccache: &Mutex, ) -> Result { - let DeflatedToolchain { - path: toolchain_dir, - build_count: id, - ctime: _, - } = { - let mut toolchain_dir_map = self.toolchain_dir_map.lock().unwrap(); - // Create the toolchain dir (if necessary) while we have an exclusive lock - let toolchain_dir = self.dir.join("toolchains").join(&tc.archive_id); - if toolchain_dir_map.contains_key(tc) && toolchain_dir.exists() { - // TODO: use if let when sccache can use NLL - let entry = toolchain_dir_map - .get_mut(tc) - .expect("Key missing after checking"); - entry.build_count += 1; - entry.clone() - } else { - trace!("Creating toolchain directory for {}", tc.archive_id); - fs::create_dir(&toolchain_dir)?; - - let mut tccache = tccache.lock().unwrap(); - let toolchain_rdr = match tccache.get(tc) { - Ok(rdr) => rdr, - Err(LruError::FileNotInCache) => { - bail!("expected toolchain {}, but not available", tc.archive_id) - } - Err(e) => { - return Err(Error::from(e).context("failed to get toolchain from cache")); - } - }; + let toolchain_dir = self.dir.join("toolchains").join(&tc.archive_id); - tar::Archive::new(GzDecoder::new(toolchain_rdr)) - .unpack(&toolchain_dir) - .or_else(|e| { - warn!("Failed to unpack toolchain: {:?}", e); - fs::remove_dir_all(&toolchain_dir) - .context("Failed to remove unpacked toolchain")?; - tccache - .remove(tc) - .context("Failed to remove corrupt toolchain")?; - Err(Error::from(e)) - })?; - - let entry = DeflatedToolchain { - path: toolchain_dir, - build_count: 1, - ctime: Instant::now(), - }; + // Fast path: an already-prepared toolchain just bumps its build + // counter. The global map lock is held only for this lookup. + if let Some(entry) = self.bump_prepared_toolchain(tc, &toolchain_dir) { + return self.make_overlay_spec(tc, entry); + } + + // Serialize preparation on the per-toolchain lock, NOT the global map + // lock, so requests for different toolchains prepare in parallel while + // requests for the same uncached toolchain wait here. + let prepare_lock = get_prepare_lock(&self.toolchain_prepare_locks, tc); + let _prepare_guard = prepare_lock.lock().unwrap(); - toolchain_dir_map.insert(tc.clone(), entry.clone()); - if toolchain_dir_map.len() > tccache.len() { - let dir_map = toolchain_dir_map.clone(); - let mut entries: Vec<_> = dir_map.iter().collect(); - // In the pathological case, creation time for unpacked - // toolchains could be the opposite of the least recently - // recently used, so we clear out half of the accumulated - // toolchains to prevent repeated sort/delete cycles. - entries.sort_by_key(|a| (a.1).ctime); - entries.truncate(entries.len() / 2); - for (tc, _) in entries { - warn!("Removing old un-compressed toolchain: {:?}", tc); - assert!(toolchain_dir_map.remove(tc).is_some()); - fs::remove_dir_all(self.dir.join("toolchains").join(&tc.archive_id)) - .context("Failed to remove old toolchain directory")?; + // Another request may have prepared this toolchain while we waited on + // the prepare lock; re-check before unpacking. + if let Some(entry) = self.bump_prepared_toolchain(tc, &toolchain_dir) { + return self.make_overlay_spec(tc, entry); + } + + trace!("Creating toolchain directory for {}", tc.archive_id); + fs::create_dir(&toolchain_dir)?; + + { + let mut tccache = tccache.lock().unwrap(); + let toolchain_rdr = match tccache.get(tc) { + Ok(rdr) => rdr, + Err(LruError::FileNotInCache) => { + bail!("expected toolchain {}, but not available", tc.archive_id) + } + Err(e) => { + return Err(Error::from(e).context("failed to get toolchain from cache")); + } + }; + + tar::Archive::new(GzDecoder::new(toolchain_rdr)) + .unpack(&toolchain_dir) + .or_else(|e| { + warn!("Failed to unpack toolchain: {:?}", e); + fs::remove_dir_all(&toolchain_dir) + .context("Failed to remove unpacked toolchain")?; + tccache + .remove(tc) + .context("Failed to remove corrupt toolchain")?; + Err(Error::from(e)) + })?; + } + + let entry = DeflatedToolchain { + path: toolchain_dir, + build_count: 1, + ctime: Instant::now(), + }; + + // Insert the new entry and pick eviction victims under the global map + // lock, but run the remove_dir_all for each victim AFTER dropping it so + // filesystem teardown never serializes concurrent preparations. + let mut evictions: Vec = Vec::new(); + { + let mut toolchain_dir_map = self.toolchain_dir_map.lock().unwrap(); + toolchain_dir_map.insert(tc.clone(), entry.clone()); + if toolchain_dir_map.len() > tccache.lock().unwrap().len() { + let dir_map = toolchain_dir_map.clone(); + let mut entries: Vec<_> = dir_map.iter().collect(); + // In the pathological case, creation time for unpacked + // toolchains could be the opposite of the least recently + // recently used, so we clear out half of the accumulated + // toolchains to prevent repeated sort/delete cycles. + entries.sort_by_key(|a| (a.1).ctime); + entries.truncate(entries.len() / 2); + for (victim, _) in entries { + if toolchain_dir_map.remove(victim).is_some() { + evictions.push(victim.clone()); + } else { + warn!( + "toolchain {:?} already gone from dir map during eviction", + victim + ); } } - entry } - }; + } + for victim in evictions { + warn!("Removing old un-compressed toolchain: {:?}", victim); + fs::remove_dir_all(self.dir.join("toolchains").join(&victim.archive_id)) + .context("Failed to remove old toolchain directory")?; + } + + self.make_overlay_spec(tc, entry) + } + + // Bump and return a clone of the prepared-toolchain entry when it is both + // recorded in the map and present on disk, else None. Holds the global map + // lock only for the lookup and counter bump. + fn bump_prepared_toolchain( + &self, + tc: &Toolchain, + toolchain_dir: &Path, + ) -> Option { + let mut toolchain_dir_map = self.toolchain_dir_map.lock().unwrap(); + if toolchain_dir_map.contains_key(tc) && toolchain_dir.exists() { + let entry = toolchain_dir_map + .get_mut(tc) + .expect("Key missing after checking"); + entry.build_count += 1; + Some(entry.clone()) + } else { + None + } + } + fn make_overlay_spec(&self, tc: &Toolchain, entry: DeflatedToolchain) -> Result { + let DeflatedToolchain { + path: toolchain_dir, + build_count: id, + ctime: _, + } = entry; trace!("Creating build directory for {}-{}", tc.archive_id, id); let build_dir = self .dir @@ -873,3 +944,52 @@ impl BuilderIncoming for DockerBuilder { Ok(res) } } + +#[cfg(test)] +mod tests { + use super::*; + + fn tc(id: &str) -> Toolchain { + Toolchain { + archive_id: id.to_owned(), + } + } + + // Two requests for the same uncached toolchain must serialize on one + // preparation lock (so the gzip+untar happens once, with no half-written + // dir), while requests for different toolchains get independent locks and + // never block each other. Constructing an OverlayBuilder requires root, so + // drive the lock-acquisition invariant that prepare_overlay_dirs relies on + // directly. + #[test] + fn test_prepare_lock_shared_per_key_and_distinct_across_keys() { + let locks = Mutex::new(HashMap::new()); + + let a1 = get_prepare_lock(&locks, &tc("aaaa")); + let a2 = get_prepare_lock(&locks, &tc("aaaa")); + let b = get_prepare_lock(&locks, &tc("bbbb")); + + assert!( + Arc::ptr_eq(&a1, &a2), + "same toolchain key must share one preparation lock" + ); + assert!( + !Arc::ptr_eq(&a1, &b), + "different toolchain keys must get distinct preparation locks" + ); + + // The shared lock actually serializes: while a1 is held, a second + // acquire of the same Arc blocks, but a different toolchain's lock is + // free to take. + let guard = a1.lock().unwrap(); + assert!( + a2.try_lock().is_err(), + "second acquire of the shared preparation lock must block" + ); + assert!( + b.try_lock().is_ok(), + "a different toolchain's preparation lock must not block" + ); + drop(guard); + } +} diff --git a/src/bin/sccache-dist/main.rs b/src/bin/sccache-dist/main.rs index 8fda85c8a..acdcba84e 100644 --- a/src/bin/sccache-dist/main.rs +++ b/src/bin/sccache-dist/main.rs @@ -1245,15 +1245,32 @@ impl ServerIncoming for Server { Some(tc) => tc, None => return Ok(SubmitToolchainResult::JobNotFound), }; - let mut cache = self.cache.lock().unwrap(); - // TODO: this returns before reading all the data, is that valid? - if cache.contains_toolchain(&tc) { - return Ok(SubmitToolchainResult::Success); + // Fast path: skip the transfer entirely if the toolchain is already + // cached. Grab the cache directory while we hold the lock so the temp + // file lands on the same filesystem and the final insert is a rename. + let tc_cache_dir = { + let cache = self.cache.lock().unwrap(); + // TODO: this returns before reading all the data, is that valid? + if cache.contains_toolchain(&tc) { + return Ok(SubmitToolchainResult::Success); + } + cache.path().to_path_buf() + }; + // Stream the upload to a temp file WITHOUT holding the cache lock, so a + // slow transfer for one toolchain no longer serializes every other + // submit behind the global cache mutex. + let mut tmpfile = match tempfile::NamedTempFile::new_in(&tc_cache_dir) { + Ok(tmpfile) => tmpfile, + Err(_) => return Ok(SubmitToolchainResult::CannotCache), + }; + if io::copy(&mut { tc_rdr }, tmpfile.as_file_mut()).is_err() { + return Ok(SubmitToolchainResult::CannotCache); } + let temp_path = tmpfile.into_temp_path(); + // Re-acquire the lock ONLY to graft the finished file into the cache. + let mut cache = self.cache.lock().unwrap(); Ok(cache - .insert_with(&tc, |mut file| { - io::copy(&mut { tc_rdr }, &mut file).map(|_| ()) - }) + .insert_at(&tc, &temp_path) .map(|_| SubmitToolchainResult::Success) .unwrap_or(SubmitToolchainResult::CannotCache)) } @@ -1891,4 +1908,40 @@ mod scheduler_tests { "job_count is the 0-based allocated= counter and must start at 0" ); } + + // The upload transfer in handle_submit_toolchain streams to a temp file + // with the cache lock released; only the final graft reacquires the lock. + // Drive that graft path directly: a materialized temp file inserts under + // its toolchain key, and a file whose contents do not hash to the key is + // refused rather than served as a corrupt entry. + #[test] + fn test_submit_toolchain_no_lock_across_transfer() { + use sccache::util::Digest; + use std::io::Write; + + let cache_dir = tempfile::TempDir::new().unwrap(); + let mut cache = TcCache::new(cache_dir.path(), u64::MAX).unwrap(); + + // Materialize the upload exactly as the handler does after io::copy. + let mut tmpfile = tempfile::NamedTempFile::new_in(cache.path()).unwrap(); + tmpfile.write_all(b"toolchain-bytes").unwrap(); + tmpfile.flush().unwrap(); + let archive_id = Digest::reader_sync(std::fs::File::open(tmpfile.path()).unwrap()).unwrap(); + let tc = Toolchain { archive_id }; + let temp_path = tmpfile.into_temp_path(); + + assert!(!cache.contains_toolchain(&tc)); + cache.insert_at(&tc, &temp_path).unwrap(); + assert!(cache.contains_toolchain(&tc)); + + // A temp file that does not hash to the requested key is refused. + let mut bad = tempfile::NamedTempFile::new_in(cache.path()).unwrap(); + bad.write_all(b"not-the-right-bytes").unwrap(); + bad.flush().unwrap(); + let wrong_tc = Toolchain { + archive_id: "deadbeefcafe".to_owned(), + }; + let bad_path = bad.into_temp_path(); + assert!(cache.insert_at(&wrong_tc, &bad_path).is_err()); + } } diff --git a/src/dist/cache.rs b/src/dist/cache.rs index 72e9c3d66..87057f8b9 100644 --- a/src/dist/cache.rs +++ b/src/dist/cache.rs @@ -489,6 +489,22 @@ impl TcCache { } } + /// Insert an already-materialized toolchain file into the cache by moving it + /// under `tc`'s key. The caller streams the upload to a temp file first, so + /// the transfer runs without holding the cache lock; this method takes the + /// lock only to graft the finished file in. + pub fn insert_at(&mut self, tc: &Toolchain, path: &Path) -> Result<()> { + self.inner + .insert_file(make_lru_key_path(&tc.archive_id), path)?; + let verified_archive_id = file_key(self.get(tc)?)?; + // TODO: remove created toolchain? + if verified_archive_id == tc.archive_id { + Ok(()) + } else { + Err(anyhow!("written file does not match expected hash key")) + } + } + pub fn get_file(&mut self, tc: &Toolchain) -> LruResult { self.inner.get_file(make_lru_key_path(&tc.archive_id)) } @@ -497,6 +513,10 @@ impl TcCache { self.inner.get(make_lru_key_path(&tc.archive_id)) } + pub fn path(&self) -> &Path { + self.inner.path() + } + pub fn len(&self) -> usize { self.inner.len() } From d11eeb3b50f9bf621adddbc8785d1a2f2a9632d3 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 17:09:59 -0600 Subject: [PATCH 42/43] dist: fix chunked-upload keep-alive desync and eviction race A review against the tiny_http-0.12.0 source found the G2 keep-alive change had one hole. do_submit_toolchain posts a chunked body (wrap_stream, no Content-Length), and tiny_http's chunked reader has no drop-drain, unlike the Content-Length EqualReader the change relied on. The server's submit handler has early returns that skip reading the body (cache fast-path, JobNotFound), so once Connection: close was dropped a partially-read chunked body could leave chunk bytes on a pooled connection and desync the next request framed on it. Two jobs racing a new toolchain hit this on every cold build. Re-add Connection: close on the do_submit_toolchain request only. Submits are rare and huge, so keep-alive there is worth ~nothing, and it makes the unread early-returns legal again without a server-side drain. Every other path sends a sized bincode body that tiny_http's EqualReader sweeps, so keep-alive stays on where it pays. Also address the review's promptly-fix items: delete eviction victims under their own prepare lock (try_lock-and-skip, which is deadlock-safe against a concurrent eviction where a held victim lock already means an active preparer that must not be deleted) and tolerate a stale leftover dir, so a victim being re-prepared cannot be deleted mid-untar and escalate into a lost archive; untar from an owned get_file handle so two different toolchains no longer serialize on the cache mutex; remove a hash-mismatched entry in insert_at instead of leaving it cached; and duplicate the A5 keep-alive revert marker into the blocking bincode_req path so a soak rollback re-closes both client paths. Signed-off-by: Javier Tia --- src/bin/sccache-dist/build.rs | 96 +++++++++++++++++++++++++++++++---- src/dist/cache.rs | 4 +- src/dist/http.rs | 19 +++++-- 3 files changed, 106 insertions(+), 13 deletions(-) diff --git a/src/bin/sccache-dist/build.rs b/src/bin/sccache-dist/build.rs index d88388dca..1a2ff1e15 100644 --- a/src/bin/sccache-dist/build.rs +++ b/src/bin/sccache-dist/build.rs @@ -217,27 +217,46 @@ impl OverlayBuilder { } trace!("Creating toolchain directory for {}", tc.archive_id); + // We hold this toolchain's prepare lock and it is absent from the map, so + // any directory on disk is a stale leftover (an interrupted preparation, + // or an eviction that pruned the map entry before deleting the dir). Clear + // it so create_dir does not fail with AlreadyExists. + if toolchain_dir.exists() { + fs::remove_dir_all(&toolchain_dir) + .context("Failed to remove stale toolchain directory")?; + } fs::create_dir(&toolchain_dir)?; { - let mut tccache = tccache.lock().unwrap(); - let toolchain_rdr = match tccache.get(tc) { - Ok(rdr) => rdr, - Err(LruError::FileNotInCache) => { - bail!("expected toolchain {}, but not available", tc.archive_id) - } - Err(e) => { - return Err(Error::from(e).context("failed to get toolchain from cache")); + // Take an owned handle to the cached archive under the lock, then + // release the tccache mutex before untarring. The archive reader used + // to borrow the cache guard, which serialized every toolchain's untar + // on this one mutex; an owned File lets different toolchains untar in + // parallel. + let toolchain_file = { + let mut tccache = tccache.lock().unwrap(); + match tccache.get_file(tc) { + Ok(file) => file, + Err(LruError::FileNotInCache) => { + bail!("expected toolchain {}, but not available", tc.archive_id) + } + Err(e) => { + return Err(Error::from(e).context("failed to get toolchain from cache")); + } } }; - tar::Archive::new(GzDecoder::new(toolchain_rdr)) + tar::Archive::new(GzDecoder::new(toolchain_file)) .unpack(&toolchain_dir) .or_else(|e| { warn!("Failed to unpack toolchain: {:?}", e); fs::remove_dir_all(&toolchain_dir) .context("Failed to remove unpacked toolchain")?; + // Removing the corrupt archive needs the cache lock again; + // the untar handle is dropped by now, so re-acquire briefly. tccache + .lock() + .unwrap() .remove(tc) .context("Failed to remove corrupt toolchain")?; Err(Error::from(e)) @@ -279,9 +298,41 @@ impl OverlayBuilder { } } for victim in evictions { + // Delete each victim under its own preparation lock so a concurrent + // request that legitimately re-prepares the same toolchain cannot + // create_dir+untar into the path we are deleting. Use try_lock, never + // a blocking acquire: we already hold this request's prepare lock, and + // blocking on a second prepare lock could deadlock against a racer that + // picked us as its own eviction victim. A held lock means a racer is + // actively preparing the victim, so skip its deletion. + let victim_lock = get_prepare_lock(&self.toolchain_prepare_locks, &victim); + let _victim_guard = match victim_lock.try_lock() { + Ok(guard) => guard, + Err(_) => { + trace!("Skipping eviction of {:?}: being prepared", victim); + continue; + } + }; + + // A racer may have finished re-preparing the victim while we waited on + // its lock. Re-verify it is still absent from the map before deleting, + // holding the map lock only for the check and never across the + // remove_dir_all below (keeps map and prepare locks disjoint). + { + let toolchain_dir_map = self.toolchain_dir_map.lock().unwrap(); + if toolchain_dir_map.contains_key(&victim) { + trace!("Skipping eviction of {:?}: re-prepared", victim); + continue; + } + } + warn!("Removing old un-compressed toolchain: {:?}", victim); fs::remove_dir_all(self.dir.join("toolchains").join(&victim.archive_id)) .context("Failed to remove old toolchain directory")?; + + // Prune the now-unused preparation lock entry so the map does not grow + // without bound as toolchains churn. + self.toolchain_prepare_locks.lock().unwrap().remove(&victim); } self.make_overlay_spec(tc, entry) @@ -992,4 +1043,31 @@ mod tests { ); drop(guard); } + + // The eviction path in prepare_overlay_dirs deletes a victim only after a + // try_lock on the victim's preparation lock succeeds. Constructing an + // OverlayBuilder requires root, so drive the try_lock invariant the eviction + // loop relies on directly: while a victim is being prepared (its lock held), + // try_lock must fail so the victim is skipped; once released, try_lock must + // succeed so the stale directory can be removed. + #[test] + fn test_eviction_skips_victim_while_being_prepared() { + let locks = Mutex::new(HashMap::new()); + let victim = tc("cccc"); + + let prepare_lock = get_prepare_lock(&locks, &victim); + let held = prepare_lock.lock().unwrap(); + + let eviction_view = get_prepare_lock(&locks, &victim); + assert!( + eviction_view.try_lock().is_err(), + "a victim under active preparation must not be evicted" + ); + + drop(held); + assert!( + eviction_view.try_lock().is_ok(), + "once preparation finishes the victim is free to evict" + ); + } } diff --git a/src/dist/cache.rs b/src/dist/cache.rs index 87057f8b9..0cc26cbb6 100644 --- a/src/dist/cache.rs +++ b/src/dist/cache.rs @@ -497,10 +497,12 @@ impl TcCache { self.inner .insert_file(make_lru_key_path(&tc.archive_id), path)?; let verified_archive_id = file_key(self.get(tc)?)?; - // TODO: remove created toolchain? if verified_archive_id == tc.archive_id { Ok(()) } else { + // The grafted file hashes to the wrong key; drop it so a poisoned + // entry cannot satisfy a later lookup under the expected key. + let _ = self.inner.remove(make_lru_key_path(&tc.archive_id)); Err(anyhow!("written file does not match expected hash key")) } } diff --git a/src/dist/http.rs b/src/dist/http.rs index f8e54a94e..dacc53ff9 100644 --- a/src/dist/http.rs +++ b/src/dist/http.rs @@ -288,8 +288,11 @@ mod server { pub fn bincode_req( req: reqwest::blocking::RequestBuilder, ) -> Result { - // Keep-alive is left enabled (no `Connection: close`) so the connection - // pool can reuse this socket for the next coordination call. + // A5: keep-alive is left enabled (no `Connection: close`), so a reused + // connection reaches tiny_http, which risks issue #151 (pipeline/body + // desync) surfacing as bincode decode errors under soak. Revert point: + // re-add `.header(reqwest::header::CONNECTION, "close")` on the request + // below. let mut res = req.send()?; let status = res.status(); let mut body = vec![]; @@ -1372,7 +1375,17 @@ mod client { let toolchain_file = tokio::fs::File::from_std(toolchain_file.into()); let toolchain_file_stream = tokio_util::io::ReaderStream::new(toolchain_file); let body = Body::wrap_stream(toolchain_file_stream); - let req = req.bearer_auth(job_alloc.auth).body(body); + // This path streams the toolchain as an HTTP chunked body (no + // Content-Length). tiny_http's chunked reader has no drop-drain, + // and the server's submit handler has early returns that skip + // reading the body (cache fast-path, JobNotFound), so keep-alive + // here can desync a pooled connection on the server's unread + // early-returns. Submits are rare and huge, so reuse is worth + // ~nothing; force a close. + let req = req + .bearer_auth(job_alloc.auth) + .header(reqwest::header::CONNECTION, "close") + .body(body); bincode_req_fut(req, self.deserialize_offload_threshold).await } Ok(None) => Err(anyhow!("couldn't find toolchain locally")), From 59811d6aa5ec13acfff98db9a12d8e0cf38a9d93 Mon Sep 17 00:00:00 2001 From: Javier Tia Date: Fri, 3 Jul 2026 18:26:54 -0600 Subject: [PATCH 43/43] dist/pkg: ship long input paths via GNU tar long-name The input packager built each tar entry with a ustar header and set the entry path with Header::set_path, which overruns the ustar name (100 B) and prefix (155 B) fields for deep build paths. gcc-runtime's libstdc++-v3 objects exceed both, so set_path returned "provided value is too long", the whole input tar failed, and every such compile fell back to local instead of distributing (observed as 50 gcc-runtime fallbacks on one cold core-image-minimal build). Build the entry headers with Header::new_gnu() and write them through a new pkg::append_tar_entry helper that routes to Builder::append_data, which emits a GNU @LongLink long-name entry when the path overflows the ustar fields and recomputes the checksum. Normal-length paths stay plain entries. The server untar side already reads @LongLink transparently via tar::Archive::unpack, so no server change is needed. Covers all three packagers (the two C input paths and the rust path). Signed-off-by: Javier Tia --- src/compiler/c.rs | 15 ++++---- src/compiler/rust.rs | 13 ++++--- src/dist/pkg.rs | 84 +++++++++++++++++++++++++++++++++++++------- 3 files changed, 89 insertions(+), 23 deletions(-) diff --git a/src/compiler/c.rs b/src/compiler/c.rs index 625ae927b..f3a4c229d 100644 --- a/src/compiler/c.rs +++ b/src/compiler/c.rs @@ -1351,10 +1351,14 @@ impl pkg::InputsPackager for CInputsPackager { format!("unable to transform input path {}", input_path.display()) })?; - let mut file_header = pkg::make_tar_header(&input_path, &dist_input_path)?; + let mut file_header = pkg::make_tar_header(&input_path)?; file_header.set_size(preprocessed_input.len() as u64); // The metadata is from non-preprocessed - file_header.set_cksum(); - builder.append(&file_header, preprocessed_input.as_slice())?; + pkg::append_tar_entry( + &mut builder, + &mut file_header, + &dist_input_path, + preprocessed_input.as_slice(), + )?; } for input_path in extra_hash_files.iter().chain(extra_dist_files.iter()) { @@ -1379,10 +1383,9 @@ impl pkg::InputsPackager for CInputsPackager { let mut output = vec![]; io::copy(&mut file, &mut output)?; - let mut file_header = pkg::make_tar_header(&input_path, &dist_input_path)?; + let mut file_header = pkg::make_tar_header(&input_path)?; file_header.set_size(output.len() as u64); - file_header.set_cksum(); - builder.append(&file_header, &*output)?; + pkg::append_tar_entry(&mut builder, &mut file_header, &dist_input_path, &*output)?; } // Finish archive diff --git a/src/compiler/rust.rs b/src/compiler/rust.rs index 877ec07ca..7d9493f9e 100644 --- a/src/compiler/rust.rs +++ b/src/compiler/rust.rs @@ -2321,7 +2321,7 @@ impl pkg::InputsPackager for RustInputsPackager { let mut builder = tar::Builder::new(wtr); for (input_path, dist_input_path) in all_tar_inputs.iter() { - let mut file_header = pkg::make_tar_header(input_path, dist_input_path)?; + let mut file_header = pkg::make_tar_header(input_path)?; let trimmed = if can_trim_rlibs && can_trim_this(input_path) { trimmed_rlib_metadata(input_path)? } else { @@ -2329,15 +2329,18 @@ impl pkg::InputsPackager for RustInputsPackager { }; if let Some(metadata_ar) = trimmed { file_header.set_size(metadata_ar.len() as u64); - file_header.set_cksum(); - builder.append(&file_header, metadata_ar.as_slice())?; + pkg::append_tar_entry( + &mut builder, + &mut file_header, + dist_input_path, + metadata_ar.as_slice(), + )?; } else { // Either not trimmable, or a split-metadata rlib with no // embedded metadata member -- ship the whole file so the remote // gets the real crate (e.g. an OE target sysroot's std/core). let file = fs::File::open(input_path)?; - file_header.set_cksum(); - builder.append(&file_header, file)?; + pkg::append_tar_entry(&mut builder, &mut file_header, dist_input_path, file)?; } } diff --git a/src/dist/pkg.rs b/src/dist/pkg.rs index 1a48b38a2..c52bdc567 100644 --- a/src/dist/pkg.rs +++ b/src/dist/pkg.rs @@ -611,10 +611,10 @@ mod toolchain_imp { } } -pub fn make_tar_header(src: &Path, dest: &str) -> io::Result { +pub fn make_tar_header(src: &Path) -> io::Result { let metadata_res = fs::metadata(src); - let mut file_header = tar::Header::new_ustar(); + let mut file_header = tar::Header::new_gnu(); // TODO: test this works if let Ok(metadata) = metadata_res { // TODO: if the source file is a symlink, I think this does bad things @@ -630,24 +630,38 @@ pub fn make_tar_header(src: &Path, dest: &str) -> io::Result { file_header.set_mtime(0); file_header .set_device_major(0) - .expect("expected a ustar header"); + .expect("expected a gnu header"); file_header .set_device_minor(0) - .expect("expected a ustar header"); + .expect("expected a gnu header"); file_header.set_entry_type(tar::EntryType::file()); } - // tar-rs imposes that `set_path` takes a relative path + Ok(file_header) +} + +/// Append a file entry to `builder` under the dist path `dest`. +/// +/// `dest` is an absolute dist path; tar-rs requires a relative path, so the +/// leading `/` is stripped. Routing through `Builder::append_data` (rather than +/// `Builder::append` with a pre-set path) makes tar-rs emit a GNU `@LongLink` +/// long-name entry when `dest` exceeds the ustar `name`/`prefix` limits. Deep +/// OE build paths (e.g. gcc-runtime's libstdc++-v3 tree) overrun those limits, +/// and a plain ustar header fails with "provided value is too long when setting +/// path". `Header::set_size` must already be correct on `header`. +pub fn append_tar_entry( + builder: &mut tar::Builder, + header: &mut tar::Header, + dest: &str, + data: R, +) -> io::Result<()> { + // tar-rs imposes that the entry path is relative. assert!(dest.starts_with('/')); let dest = dest.trim_start_matches('/'); assert!(!dest.starts_with('/')); - // `set_path` converts its argument to a Path and back to bytes on Windows, so this is - // a bit of an inefficient round-trip. Windows path separators will also be normalised - // to be like Unix, and the path is (now) relative so there should be no funny results - // due to Windows - // TODO: should really use a `set_path_str` or similar - file_header.set_path(dest)?; - Ok(file_header) + // `append_data` sets the path (emitting a GNU long-name entry if needed) and + // recomputes the checksum before writing the header and `data`. + builder.append_data(header, dest, data) } /// Simplify a path to one without any relative components, erroring if it looks @@ -704,3 +718,49 @@ impl SimplifyPath<'_> { Ok(final_path) } } + +#[cfg(test)] +mod pkg_tests { + use super::*; + use std::io::Read; + + #[test] + fn test_append_tar_entry_roundtrips_long_path() { + // A final path component longer than the ustar name field (100 bytes), + // which ustar cannot split across name/prefix, mirroring the deep + // gcc-runtime libstdc++-v3 build tree that triggers the failure. + let dest = format!( + "/build/gcc-runtime/13.4.0/libstdc++-v3/src/c++98/{}.o", + "b".repeat(120) + ); + let relative = dest.trim_start_matches('/'); + assert!(relative.len() > 100); + + // The old ustar-header path could not encode this: set_path errors with + // "provided value is too long", which is the bug this change fixes. + let mut ustar = tar::Header::new_ustar(); + assert!(ustar.set_path(relative).is_err()); + + let tmp = tempfile::tempdir().unwrap(); + let src = tmp.path().join("input.o"); + let contents = b"long-path-payload"; + fs::write(&src, &contents[..]).unwrap(); + + let mut buf = Vec::new(); + { + let mut builder = tar::Builder::new(&mut buf); + let mut header = make_tar_header(&src).unwrap(); + header.set_size(contents.len() as u64); + append_tar_entry(&mut builder, &mut header, &dest, &contents[..]).unwrap(); + builder.into_inner().unwrap(); + } + + let mut archive = tar::Archive::new(buf.as_slice()); + let mut entry = archive.entries().unwrap().next().unwrap().unwrap(); + let entry_path = entry.path().unwrap().to_string_lossy().into_owned(); + assert_eq!(entry_path, relative); + let mut read_back = Vec::new(); + entry.read_to_end(&mut read_back).unwrap(); + assert_eq!(read_back, contents); + } +}