Skip to content

Commit

Permalink
RemoteExecutionService: fix outputs not being uploaded (#15842)
Browse files Browse the repository at this point in the history
In 4d900ce we introduced validation in
DiskAndRemoteCacheClient.uploadActionResult() where context's step must
be UPLOAD_OUTPUTS to trigger the upload.

However, this value was never set in RemoteExecutionService before hand
thus led to outputs not being uploaded and cause remote cache misses.

Fix #15682

Thanks @adam-singer for doing the investigation 🙏

Closes #15823.

PiperOrigin-RevId: 459519852
Change-Id: Ib004403d7893fe135adcc4b181b607d8cb33f3af

Co-authored-by: Son Luong Ngoc <sluongng@gmail.com>
  • Loading branch information
ckolli5 and sluongng authored Jul 14, 2022
1 parent 06ca634 commit 40e485d
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1240,6 +1240,8 @@ public void uploadOutputs(RemoteAction action, SpawnResult spawnResult)
SpawnResult.Status.SUCCESS.equals(spawnResult.status()) && spawnResult.exitCode() == 0,
"shouldn't upload outputs of failed local action");

action.getRemoteActionExecutionContext().setStep(Step.UPLOAD_OUTPUTS);

if (remoteOptions.remoteCacheAsync) {
Single.using(
remoteCache::retain,
Expand Down
27 changes: 27 additions & 0 deletions src/test/shell/bazel/remote/remote_execution_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2429,6 +2429,33 @@ EOF
rm -rf $cache
}

function test_combined_cache_upload() {
mkdir -p a
echo 'bar' > a/bar
cat > a/BUILD <<'EOF'
genrule(
name = "foo",
srcs = [":bar"],
outs = ["foo.txt"],
cmd = """
echo $(location :bar) > "$@"
""",
)
EOF

CACHEDIR=$(mktemp -d)

bazel build \
--disk_cache=$CACHEDIR \
--remote_cache=grpc://localhost:${worker_port} \
//a:foo >& $TEST_log || "Failed to build //a:foo"

remote_ac_files="$(count_remote_ac_files)"
[[ "$remote_ac_files" == 1 ]] || fail "Expected 1 remote action cache entries, not $remote_ac_files"
remote_cas_files="$(count_remote_cas_files)"
[[ "$remote_cas_files" == 3 ]] || fail "Expected 3 remote cas entries, not $remote_cas_files"
}

function test_combined_cache_with_no_remote_cache_tag_remote_cache() {
# Test that actions with no-remote-cache tag can hit disk cache of a combined cache but
# remote cache is disabled.
Expand Down

0 comments on commit 40e485d

Please sign in to comment.