Skip to content

Commit

Permalink
Remove synchronous remote cache lookup from remote execution (#15854)
Browse files Browse the repository at this point in the history
To prepare for #11331, it's necessary to clarify the building of the "stacks" of command runners, in order to progressively remove caches for each attempt to backtrack.

Both for performance reasons (to allow an attempt to execute a process to proceed concurrently with checking the cache), and to allow us to more easily conditionally remove remote caching from remote execution, this change removes the synchronous cache lookup from the `remote::CommandRunner` by wrapping in `remote_cache::CommandRunner`.

Additionally, refactors the `CommandRunner` stacking, and produces stacks per backtrack attempt for #11331.
stuhood authored Jun 17, 2022

Verified

This commit was signed with the committer’s verified signature.
purajit purajit
1 parent 42bead6 commit 0616042
Showing 12 changed files with 483 additions and 537 deletions.
9 changes: 9 additions & 0 deletions src/rust/engine/process_execution/src/bounded.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::borrow::Cow;
use std::cmp::{max, min, Ordering, Reverse};
use std::collections::VecDeque;
use std::fmt::{self, Debug};
use std::future::Future;
use std::sync::{atomic, Arc};
use std::time::{Duration, Instant};
@@ -53,6 +54,14 @@ impl CommandRunner {
}
}

impl Debug for CommandRunner {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("bounded::CommandRunner")
.field("inner", &self.inner)
.finish_non_exhaustive()
}
}

#[async_trait]
impl crate::CommandRunner for CommandRunner {
async fn run(
17 changes: 13 additions & 4 deletions src/rust/engine/process_execution/src/cache.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::fmt::{self, Debug};
use std::sync::Arc;
use std::time::Instant;

@@ -29,28 +30,36 @@ struct PlatformAndResponseBytes {

#[derive(Clone)]
pub struct CommandRunner {
underlying: Arc<dyn crate::CommandRunner>,
inner: Arc<dyn crate::CommandRunner>,
cache: PersistentCache,
file_store: Store,
metadata: ProcessMetadata,
}

impl CommandRunner {
pub fn new(
underlying: Arc<dyn crate::CommandRunner>,
inner: Arc<dyn crate::CommandRunner>,
cache: PersistentCache,
file_store: Store,
metadata: ProcessMetadata,
) -> CommandRunner {
CommandRunner {
underlying,
inner,
cache,
file_store,
metadata,
}
}
}

impl Debug for CommandRunner {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("cache::CommandRunner")
.field("inner", &self.inner)
.finish_non_exhaustive()
}
}

#[async_trait]
impl crate::CommandRunner for CommandRunner {
async fn run(
@@ -125,7 +134,7 @@ impl crate::CommandRunner for CommandRunner {
return Ok(result);
}

let result = self.underlying.run(context.clone(), workunit, req).await?;
let result = self.inner.run(context.clone(), workunit, req).await?;
if result.exit_code == 0 || write_failures_to_cache {
let result = result.clone();
in_workunit!("local_cache_write", Level::Trace, |workunit| async move {
4 changes: 2 additions & 2 deletions src/rust/engine/process_execution/src/lib.rs
Original file line number Diff line number Diff line change
@@ -29,7 +29,7 @@ extern crate derivative;

use std::collections::{BTreeMap, BTreeSet};
use std::convert::{TryFrom, TryInto};
use std::fmt::{self, Display};
use std::fmt::{self, Debug, Display};
use std::path::PathBuf;

use async_trait::async_trait;
@@ -783,7 +783,7 @@ impl Context {
}

#[async_trait]
pub trait CommandRunner: Send + Sync {
pub trait CommandRunner: Send + Sync + Debug {
///
/// Submit a request for execution on the underlying runtime, and return
/// a future for it.
8 changes: 8 additions & 0 deletions src/rust/engine/process_execution/src/local.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::ffi::OsStr;
use std::fmt::{self, Debug};
use std::fs::create_dir_all;
use std::io::Write;
use std::ops::Neg;
@@ -137,6 +138,13 @@ impl CommandRunner {
}
}

impl Debug for CommandRunner {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("local::CommandRunner")
.finish_non_exhaustive()
}
}

pub struct HermeticCommand {
inner: Command,
}
9 changes: 9 additions & 0 deletions src/rust/engine/process_execution/src/nailgun/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::{BTreeMap, BTreeSet};
use std::fmt::{self, Debug};
use std::net::SocketAddr;
use std::path::{Path, PathBuf};

@@ -109,6 +110,14 @@ impl CommandRunner {
}
}

impl Debug for CommandRunner {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("nailgun::CommandRunner")
.field("inner", &self.inner)
.finish_non_exhaustive()
}
}

#[async_trait]
impl super::CommandRunner for CommandRunner {
async fn run(
Loading

0 comments on commit 0616042

Please sign in to comment.