From cd1b4881189c286cf47d1d5876daf240dff8ff4f Mon Sep 17 00:00:00 2001 From: nicholaslyang Date: Tue, 28 May 2024 13:45:32 -0400 Subject: [PATCH 1/2] Feature flagged off file hashing --- crates/turborepo-lib/Cargo.toml | 2 +- crates/turborepo-lib/src/task_hash.rs | 126 ++++++++++++++------------ crates/turborepo/Cargo.toml | 1 - 3 files changed, 68 insertions(+), 61 deletions(-) diff --git a/crates/turborepo-lib/Cargo.toml b/crates/turborepo-lib/Cargo.toml index 27eb0ca243c0b..e2bbccadd4506 100644 --- a/crates/turborepo-lib/Cargo.toml +++ b/crates/turborepo-lib/Cargo.toml @@ -11,8 +11,8 @@ default = ["rustls-tls", "daemon-package-discovery"] native-tls = ["turborepo-api-client/native-tls", "turbo-updater/native-tls"] rustls-tls = ["turborepo-api-client/rustls-tls", "turbo-updater/rustls-tls"] -go-daemon = [] daemon-package-discovery = [] +daemon-file-hashing = [] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dev-dependencies] diff --git a/crates/turborepo-lib/src/task_hash.rs b/crates/turborepo-lib/src/task_hash.rs index 6fbe80f3d3644..2c9999b1b92cf 100644 --- a/crates/turborepo-lib/src/task_hash.rs +++ b/crates/turborepo-lib/src/task_hash.rs @@ -1,16 +1,13 @@ use std::{ collections::{HashMap, HashSet}, sync::{Arc, Mutex}, - time::Duration, }; use rayon::prelude::*; use serde::Serialize; use thiserror::Error; use tracing::{debug, Span}; -use turbopath::{ - AbsoluteSystemPath, AnchoredSystemPath, AnchoredSystemPathBuf, RelativeUnixPathBuf, -}; +use turbopath::{AbsoluteSystemPath, AnchoredSystemPath, AnchoredSystemPathBuf}; use turborepo_cache::CacheHitMetadata; use turborepo_env::{BySource, DetailedMap, EnvironmentVariableMap, ResolvedEnvMode}; use turborepo_repository::package_graph::{PackageInfo, PackageName}; @@ -83,7 +80,6 @@ impl PackageInputsHashes { tracing::trace!(scm_manual=%scm.is_manual(), "scm running in {} mode", if scm.is_manual() { "manual" } else { "git" }); let span = Span::current(); - let handle = tokio::runtime::Handle::current(); let (hashes, expanded_hashes): (HashMap<_, _>, HashMap<_, _>) = all_tasks .filter_map(|task| { let span = tracing::info_span!(parent: &span, "calculate_file_hash", ?task); @@ -92,10 +88,6 @@ impl PackageInputsHashes { return None; }; - let mut daemon = daemon - .as_ref() // Option::ref - .cloned(); - let task_definition = match task_definitions .get(task_id) .ok_or_else(|| Error::MissingPipelineEntry(task_id.clone())) @@ -126,58 +118,74 @@ impl PackageInputsHashes { let scm_telemetry = package_task_event.child(); // Try hashing with the daemon, if we have a connection. If we don't, or if we // timeout or get an error, fallback to local hashing - let hash_object = daemon - .as_mut() - .and_then(|daemon| { - let handle = handle.clone(); - // We need an async block here because the timeout must be created with an - // active tokio context. Constructing it directly in - // the rayon thread doesn't provide one and will crash at runtime. - handle - .block_on(async { - tokio::time::timeout( - Duration::from_millis(100), - daemon.get_file_hashes(package_path, &task_definition.inputs), - ) - .await - }) - .map_err(|e| { - tracing::debug!( - "daemon file hashing timed out for {}", - package_path - ); - e - }) - .ok() // If we timed out, we don't need to error, - // just return None so we can move on to local - }) - .and_then(|result| { - match result { - Ok(hashes_resp) => Some( - hashes_resp - .file_hashes - .into_iter() - .map(|(path, hash)| { - ( - RelativeUnixPathBuf::new(path) - .expect("daemon returns relative unix paths"), - hash, - ) - }) - .collect::>(), - ), - Err(e) => { - // Daemon could've failed for various reasons. We can still try - // local hashing. - tracing::debug!( - "daemon file hashing failed for {}: {}", - package_path, + #[cfg(feature = "daemon-file-hashing")] + let hash_object = { + let handle = tokio::runtime::Handle::current(); + let mut daemon = daemon + .as_ref() // Option::ref + .cloned(); + + daemon + .as_mut() + .and_then(|daemon| { + let handle = handle.clone(); + // We need an async block here because the timeout must be created with + // an active tokio context. Constructing it + // directly in the rayon thread doesn't + // provide one and will crash at runtime. + handle + .block_on(async { + tokio::time::timeout( + std::time::Duration::from_millis(100), + daemon + .get_file_hashes(package_path, &task_definition.inputs), + ) + .await + }) + .map_err(|e| { + tracing::debug!( + "daemon file hashing timed out for {}", + package_path + ); e - ); - None + }) + .ok() // If we timed out, we don't need to + // error, + // just return None so we can move on to + // local + }) + .and_then(|result| { + match result { + Ok(hashes_resp) => Some( + hashes_resp + .file_hashes + .into_iter() + .map(|(path, hash)| { + ( + turbopath::RelativeUnixPathBuf::new(path) + .expect("daemon returns relative unix paths"), + hash, + ) + }) + .collect::>(), + ), + Err(e) => { + // Daemon could've failed for various reasons. We can still try + // local hashing. + tracing::debug!( + "daemon file hashing failed for {}: {}", + package_path, + e + ); + None + } } - } - }); + }) + }; + + #[cfg(not(feature = "daemon-file-hashing"))] + let hash_object = None; + let mut hash_object = match hash_object { Some(hash_object) => hash_object, None => { diff --git a/crates/turborepo/Cargo.toml b/crates/turborepo/Cargo.toml index 4cbf580a3483c..a71cde9ffb183 100644 --- a/crates/turborepo/Cargo.toml +++ b/crates/turborepo/Cargo.toml @@ -12,7 +12,6 @@ license = "MPL-2.0" default = ["rustls-tls", "turborepo-lib/daemon-package-discovery"] native-tls = ["turborepo-lib/native-tls"] rustls-tls = ["turborepo-lib/rustls-tls"] -go-daemon = ["turborepo-lib/go-daemon"] pprof = ["turborepo-lib/pprof"] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html From a5031dcef0fbe54a9914736f48fd3d28b919d514 Mon Sep 17 00:00:00 2001 From: nicholaslyang Date: Tue, 28 May 2024 15:54:44 -0400 Subject: [PATCH 2/2] PR feedback --- crates/turborepo-lib/src/task_hash.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/crates/turborepo-lib/src/task_hash.rs b/crates/turborepo-lib/src/task_hash.rs index 2c9999b1b92cf..13e455e32491c 100644 --- a/crates/turborepo-lib/src/task_hash.rs +++ b/crates/turborepo-lib/src/task_hash.rs @@ -118,8 +118,7 @@ impl PackageInputsHashes { let scm_telemetry = package_task_event.child(); // Try hashing with the daemon, if we have a connection. If we don't, or if we // timeout or get an error, fallback to local hashing - #[cfg(feature = "daemon-file-hashing")] - let hash_object = { + let hash_object = if cfg!(feature = "daemon-file-hashing") { let handle = tokio::runtime::Handle::current(); let mut daemon = daemon .as_ref() // Option::ref @@ -181,11 +180,10 @@ impl PackageInputsHashes { } } }) + } else { + None }; - #[cfg(not(feature = "daemon-file-hashing"))] - let hash_object = None; - let mut hash_object = match hash_object { Some(hash_object) => hash_object, None => {