Skip to content

Commit

Permalink
Apply eager fetch to local cache.
Browse files Browse the repository at this point in the history
[ci skip-build-wheels]
  • Loading branch information
stuhood committed Jun 17, 2022
1 parent 53309a9 commit e915c87
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 34 deletions.
40 changes: 23 additions & 17 deletions src/rust/engine/process_execution/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub struct CommandRunner {
inner: Arc<dyn crate::CommandRunner>,
cache: PersistentCache,
file_store: Store,
eager_fetch: bool,
metadata: ProcessMetadata,
}

Expand All @@ -41,12 +42,14 @@ impl CommandRunner {
inner: Arc<dyn crate::CommandRunner>,
cache: PersistentCache,
file_store: Store,
eager_fetch: bool,
metadata: ProcessMetadata,
) -> CommandRunner {
CommandRunner {
inner,
cache,
file_store,
eager_fetch,
metadata,
}
}
Expand Down Expand Up @@ -196,23 +199,26 @@ impl CommandRunner {
return Ok(None);
};

// Ensure that all digests in the result are loadable, erroring if any are not.
// TODO: Make conditional on `eager_fetch`, since backtracking makes this unnecessary as well.
let _ = future::try_join_all(vec![
self
.file_store
.ensure_local_has_file(result.stdout_digest)
.boxed(),
self
.file_store
.ensure_local_has_file(result.stderr_digest)
.boxed(),
self
.file_store
.ensure_local_has_recursive_directory(result.output_directory.clone())
.boxed(),
])
.await?;
// If eager_fetch is enabled, ensure that all digests in the result are loadable, erroring
// if any are not. If eager_fetch is disabled, a Digest which is discovered to be missing later
// on during execution will cause backtracking.
if self.eager_fetch {
let _ = future::try_join_all(vec![
self
.file_store
.ensure_local_has_file(result.stdout_digest)
.boxed(),
self
.file_store
.ensure_local_has_file(result.stderr_digest)
.boxed(),
self
.file_store
.ensure_local_has_recursive_directory(result.output_directory.clone())
.boxed(),
])
.await?;
}

Ok(Some(result))
}
Expand Down
1 change: 1 addition & 0 deletions src/rust/engine/process_execution/src/cache_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ fn create_cached_runner(
local.into(),
cache,
store,
true,
ProcessMetadata::default(),
));

Expand Down
32 changes: 15 additions & 17 deletions src/rust/engine/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,12 +266,14 @@ impl Core {
inner_runner: Arc<dyn CommandRunner>,
full_store: &Store,
local_cache: &PersistentCache,
eager_fetch: bool,
process_execution_metadata: &ProcessMetadata,
) -> Arc<dyn CommandRunner> {
Arc::new(process_execution::cache::CommandRunner::new(
inner_runner,
local_cache.clone(),
full_store.clone(),
eager_fetch,
process_execution_metadata.clone(),
))
}
Expand Down Expand Up @@ -363,23 +365,19 @@ impl Core {
None
};

// TODO: The local cache eagerly fetches outputs independent of the `eager_fetch` flag. Once
// `eager_fetch` backtracks via https://github.com/pantsbuild/pants/issues/11331, the local
// cache will be able to obey `eager_fetch` as well, and can efficiently be used with remote
// execution.
let maybe_local_cached_runner =
if exec_strategy_opts.local_cache && !remoting_opts.execution_enable {
Some(Self::make_local_cached_runner(
maybe_remote_cached_runner
.clone()
.unwrap_or_else(|| leaf_runner.clone()),
full_store,
local_cache,
process_execution_metadata,
))
} else {
None
};
let maybe_local_cached_runner = if exec_strategy_opts.local_cache {
Some(Self::make_local_cached_runner(
maybe_remote_cached_runner
.clone()
.unwrap_or_else(|| leaf_runner.clone()),
full_store,
local_cache,
remoting_opts.cache_eager_fetch,
process_execution_metadata,
))
} else {
None
};

Ok(
vec![
Expand Down

0 comments on commit e915c87

Please sign in to comment.