RemoteExecutionService: fix outputs not being uploaded (bazelbuild#15842

) 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 bazelbuild#15682 Thanks @adam-singer for doing the investigation 🙏 Closes bazelbuild#15823. PiperOrigin-RevId: 459519852 Change-Id: Ib004403d7893fe135adcc4b181b607d8cb33f3af Co-authored-by: Son Luong Ngoc <[email protected]>
databricks · Jul 14, 2022 · 40e485d · 40e485d
1 parent 06ca634
commit 40e485d
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 0 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteExecutionService.java
@@ -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,

diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh
@@ -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.