Skip to content

Commit

Permalink
Only use remote cache when cache scope is Always or Successful (pants…
Browse files Browse the repository at this point in the history
…build#16920)

Processes with `ProcessCacheScope`s that prevent caching [already have unique `Digest`s](https://github.com/pantsbuild/pants/blob/1d007fb2a971d8518ab5311eadb0f8444c1f1fe8/src/rust/engine/process_execution/src/remote.rs#L894-L906). But in cases where we know that we should miss a remote cache, we can skip the lookup entirely.

[ci skip-build-wheels]
  • Loading branch information
somdoron authored Sep 21, 2022
1 parent 1d007fb commit 86295c1
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 7 deletions.
11 changes: 9 additions & 2 deletions src/rust/engine/process_execution/src/remote_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,10 @@ impl crate::CommandRunner for CommandRunner {
let (command_digest, action_digest) =
crate::remote::ensure_action_stored_locally(&self.store, &command, &action).await?;

let (result, hit_cache) = if self.cache_read {
let use_remote_cache = request.cache_scope == ProcessCacheScope::Always
|| request.cache_scope == ProcessCacheScope::Successful;

let (result, hit_cache) = if self.cache_read && use_remote_cache {
self
.speculate_read_action_cache(
context.clone(),
Expand All @@ -461,7 +464,11 @@ impl crate::CommandRunner for CommandRunner {
)
};

if !hit_cache && (result.exit_code == 0 || write_failures_to_cache) && self.cache_write {
if !hit_cache
&& (result.exit_code == 0 || write_failures_to_cache)
&& self.cache_write
&& use_remote_cache
{
let command_runner = self.clone();
let result = result.clone();
let _write_join = self.executor.spawn(in_workunit!(
Expand Down
4 changes: 4 additions & 0 deletions testprojects/src/jvm/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Copyright 2020 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

files(name="lib_directory", sources=["org/pantsbuild/example/lib/**/*"])
2 changes: 1 addition & 1 deletion tests/python/pants_test/integration/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ python_tests(name="prelude_integration", sources=["prelude_integration_test.py"]
python_tests(
name="remote_cache_integration",
sources=["remote_cache_integration_test.py"],
dependencies=["testprojects/src/python:hello_directory"],
dependencies=["testprojects/src/jvm:lib_directory"],
timeout=180,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,12 @@ def test_warns_on_remote_cache_errors() -> None:
def run(behavior: RemoteCacheWarningsBehavior) -> str:
pants_run = run_pants(
[
"--backend-packages=['pants.backend.python']",
"--backend-packages=['pants.backend.experimental.java']",
"--no-dynamic-ui",
"--no-local-cache",
*remote_cache_args(cas.address, behavior),
"package",
"testprojects/src/python/hello/main:main",
"check",
"testprojects/src/jvm/org/pantsbuild/example/lib/ExampleLib.java",
],
use_pantsd=False,
)
Expand All @@ -69,6 +70,18 @@ def write_err(i: int) -> str:
fourth_read_err = read_err(4)
fourth_write_err = write_err(4)

# Generate lock files first, as the test is java test.
pants_run = run_pants(
[
"--backend-packages=['pants.backend.experimental.java']",
"--no-dynamic-ui",
"--no-local-cache",
"generate-lockfiles",
],
use_pantsd=False,
)
pants_run.assert_success()

ignore_result = run(RemoteCacheWarningsBehavior.ignore)
for err in [
first_read_err,
Expand All @@ -88,7 +101,7 @@ def write_err(i: int) -> str:

backoff_result = run(RemoteCacheWarningsBehavior.backoff)
for err in [first_read_err, first_write_err, fourth_read_err, fourth_write_err]:
assert err in backoff_result
assert err in backoff_result, f"Not found in:\n{backoff_result}"
for err in [third_read_err, third_write_err]:
assert err not in backoff_result

Expand Down

0 comments on commit 86295c1

Please sign in to comment.